Skip to content

fix: carry inductor maxCurrentRating into source_component (#2837)#2838

Closed
DPS0340 wants to merge 1 commit into
tscircuit:mainfrom
DPS0340:fix/inductor-max-current-rating
Closed

fix: carry inductor maxCurrentRating into source_component (#2837)#2838
DPS0340 wants to merge 1 commit into
tscircuit:mainfrom
DPS0340:fix/inductor-max-current-rating

Conversation

@DPS0340

@DPS0340 DPS0340 commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Closes #2837.

maxCurrentRating is declared on InductorProps and max_current_rating exists on the simple_inductor schema, but doInitialSourceRender() never wrote it — the value was dropped with no warning.

                                  before      after
L1  (no rating)                   undefined   undefined
L2  maxCurrentRating="2A"         undefined   2
L3  maxCurrentRating={0.5}        undefined   0.5
L4  maxCurrentRating="500mA"      undefined   0.5

The part that isn't obvious

My first attempt used parseFloat, copying Fuse's approach, and the checker caught it: parseFloat("500mA") returns 500 — a 1000× error that would have written a plainly wrong value into circuit JSON rather than leaving it absent.

The reason Fuse gets away with parseFloat is that its ratings are conventionally written without SI prefixes. maxCurrentRating has no zod transform either, unlike capacitor's maxVoltageRating:

capacitor: maxVoltageRating: z.ZodOptional<z.ZodEffects<..., number, string | number>>  ← converted for you
inductor:  maxCurrentRating: z.ZodOptional<z.ZodUnion<[z.ZodString, z.ZodNumber]>>       ← raw string arrives

I then tried parseSiUnit, which returns NaN for "2A" — it doesn't expect a unit suffix. parseAndConvertSiUnit(...).value is the one that handles both, which I verified directly before using it:

"2A" → 2    "500mA" → 0.5    "1.5A" → 1.5    "2" → 2    "abc" → NaN

NaN/null/undefined all resolve to undefined so a malformed rating leaves the field absent rather than poisoning it.

Verification

The test bites. Reverting only Inductor.ts:

expect(ratingByName.L2).toBe(2)
Received: undefined
(fail) <inductor /> carries maxCurrentRating into source_component

It covers all four cases in one board — omitted, string with unit, plain number, and prefixed unit — so the 1000× bug specifically is pinned by expect(ratingByName.L4).toBe(0.5).

Suite: 1260 pass / 0 fail, no snapshots changed (this only adds a source field). biome format and tsc --noEmit clean.

Other props from the same sweep

I cross-referenced every *Props interface against the props each component reads. Several others appear unread — Capacitor.bypassFor/bypassTo/schSize, Resistor.schSize/tolerance, Board.topSolderMaskColor/bottomSolderMaskColor, Chip.internalCircuit/pinCompatibleVariants. I deliberately didn't touch those: unlike this one they have no obvious destination field, so "fixing" them would mean inventing schema semantics. Listed in #2837 in case they're worth triaging.

@vercel

vercel Bot commented Jul 25, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
tscircuit-core-benchmarks Ready Ready Preview, Comment Jul 25, 2026 5:11pm

Request Review

@DPS0340

DPS0340 commented Jul 25, 2026

Copy link
Copy Markdown
Contributor Author

Housekeeping — rebased onto current main (d99c99f), still 1260 pass / 0 fail. No behaviour change.

Since I have several PRs open here now, a note on how they interact so none of this lands on you:

Five of the seven are fully independent — they touch different files and merge in any order:

PR file
#2838 Inductor.ts
#2844 Hole.ts, PlatedHole.ts
#2846 Group/Group.ts (group insert sites)
#2840 Group/Group.ts (pcb_trace segment insert)
#2842 NormalComponent.ts, Group/Group.ts, Group_doInitialPcbComponentAnchorAlignment.ts

Two pairs conflict, and both are trivial:

  1. fix: omit the voltage half of the fuse label when voltageRating is absent (#2833) #2834fix: honour schShowRatings={false} on fuse (#2835) #2836 — both edit Fuse._getSchematicSymbolDisplayValue(). Whichever merges first, the other needs the two guards stacked:

    if (this._parsedProps.schShowRatings === false) return undefined   // #2836
    if (voltage === undefined || Number.isNaN(voltage)) return currentDisplay  // #2834
    return `${currentDisplay} / ${formatSiUnit(voltage)}V`

    They're orthogonal (one suppresses the whole label, the other drops just the voltage half), so the combined behaviour is well-defined. Both test files also touch fuse.test.tsx, appending separate tests.

  2. fix: carry net highlightColor into pcb_trace.highlight_color (#2839) #2840fix: write display_offset_x/y as display strings (#2841) #2842 — both add a field to nearby inserts in Group.ts; the resolution is to keep both lines.

Happy to do the merge myself: say the word and I'll combine any subset into a single PR, or rebase the losers as soon as the first one lands. I'd rather you not spend time on conflict resolution for my changes.

I verified the above rather than assuming it — checked all 21 branch pairs for conflicts and confirmed each rebases cleanly onto d99c99f.

@DPS0340

DPS0340 commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

Withdrawing this one, and eighteen others of mine in this repo, for a reason that has nothing to do with the change itself.

I opened 22 PRs here over two days. This repo currently has 29 open PRs, and 22 of them are mine — I am 75% of your review queue. In that same window you merged work from mohan-bee, Hero988, KrishnaX12, MustafaMulla29 and others, so the queue is moving; it is my share of it that is the problem. A pile that size is not a contribution, it is something a maintainer has to schedule around.

The deeper issue is where the work came from. Of those 22 PRs, 19 fix issues I filed myself. That is a closed loop: I found something, reported it, and fixed it, without anyone asking for any of it. Reviewing it costs you real attention regardless.

So I am closing the 19 that are self-originated and keeping the three that answer bug reports from other people:

The branches stay up. If any specific one of these is useful, say the number and I will reopen it — but I would rather you pick than have me guess again.

Sorry for the noise.

@DPS0340 DPS0340 closed this Jul 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

inductor drops maxCurrentRating: prop and schema field both exist, value never written

1 participant