Repository navigation
Conversation
… to fit frozen file-size baseline The 3-line comment added by cheaper-inference#8426 pushed tokenHealthCheck.ts to 843 LOC, 2 over its frozen baseline of 841 (base-red on release/v3.8.49). Fold the devin-cli rationale into one line — same meaning, gate green again.
…leProviderCard cheaper-inference#8349 left the back-navigation highlight concern inline in providers/page.tsx (state + two callbacks + onCardClick/ref props repeated at 18 card call sites), pushing the page to 1990 LOC over its frozen baseline of 1927. Move the concern into a HighlightableProviderCard wrapper: each card instance keeps its own copy of the highlighted id and only the card whose provider id matches scrolls into view and highlights — same semantics as the single page-level state, since resolveHighlightedCard already gates on the id match. page.tsx drops to 1917 LOC (under the frozen baseline, no rebaseline needed).
Direct coverage for the wrapper extracted in the previous refactor: mount-time scroll+highlight only when history.state.providerId matches, no reaction on mismatch/null state, isolation across multiple card instances, and click-time navigation recording via history.replaceState. Note: the jsdom scrollIntoView shim is a plain no-op rather than a vi.fn() — vi.restoreAllMocks() would otherwise restore the shared mock and let the next test's spy inherit its accumulated call count.
Remove stale frozen entries that have shrunk to <=800 lines and lower frozen caps for files that are still large but have decreased since the last rebaseline. Keeps the file-size gate honest after ongoing cheaper-inference#3501 god-file decomposition work.
Drop three production frozen entries whose current LOC are already at or below the 800-line cap (loc == frozen, so --update could not remove them): - ProviderDetailPageClient.tsx - useProviderModels.ts - useProviderSettings.ts Also remove two test files from the production frozen block; they already live in testFrozen with correct values: - tests/unit/account-fallback-service.test.ts - tests/unit/provider-validation-specialty.test.ts
0bad8e8 to
87dec43
Compare
…-file-size-baseline
|
Thanks for the unusually careful writeup here — the "Blast radius" section is exactly the kind of thing that makes a change like this reviewable, and it turned out to be the most important part of the PR. What the review found. This matches your PR's own Fast Quality Gates run, so it isn't a local quirk. The cause is not a stale measurement on your side. I checked each of the three against the branch history, and all three were touched by commits that landed today, after you computed the baseline:
Your Which is precisely the point you raised. You warned that pinning 78 entries to their exact LOC leaves zero headroom, and that 22 open PRs touch those paths. That prediction came true inside 24 hours, before the PR even merged, from three unrelated commits. That is a stronger argument against landing the full ratchet mid-cycle than anything the review could have added. So we're taking the option you offered: holding this until the current queue drains, and leaving the wholesale ratchet to the release captain's rebaseline-at-release — consistent with what the Two notes so the work isn't lost:
Tagging this |
|
Agreed on holding this — and thanks for tracing the three violations back to their commits rather than just reporting the red. That table is what makes the decision easy. The 24-hour drift is the part I'd underweighted. I raised zero headroom as a future maintenance cost; it turned out to be an immediate one, from three commits that had nothing to do with this PR. A ratchet that pins 78 entries to their exact LOC is only stable in a tree that isn't moving, and this tree moves several times an hour. That's a property of the timing, not something a re-measurement fixes — so I'm not going to push new values, per your note. Two things from my side: On the shared commits. You're right that #8524 and this PR carry the same three ( On what this PR becomes. If the wholesale ratchet is release-captain work, the two commits here may be more useful as a note for that pass than as a PR — the removals in particular ( No action needed on my side; leaving it as-is under |
|
Heads-up before this merges: the baseline in this branch has gone stale relative to the release tip, and merging it as-is would introduce three new The branch already has the release tip merged in (
How I hit itBuilding a local integration branch of the five open base-red fixes on top of All five merge cleanly, and four gates go from red to green: So the remaining Suggested fixRe-run the ratchet on top of the current tip before merging: This is an inherent hazard of a shrink-only ratchet PR: the longer it sits in the queue, the more likely an unrelated merge grows a frozen file and turns the baseline into a source of red rather than a fix for it. Worth re-checking immediately before merge rather than at review time. Unrelated, but found in the same run
No changes pushed to this branch — it has active worktrees I did not create, so I am leaving it to whoever owns it. |
… the queue The ratchet in this PR was taken before cheaper-inference#8500, cheaper-inference#8507 and cheaper-inference#8508 landed. Merging "Update branch" brought their CODE in but not a fresh ratchet, so three frozen values now describe a tree that no longer exists and check:file-size fails on this branch: providerPageHelpers.ts 949 > frozen 929 (grew in cheaper-inference#8500) open-sse/services/combo.ts 3641 > frozen 3640 (grew in cheaper-inference#8508) tests/unit/translator-openai-responses-req.test.ts 1194 > frozen 1189 (grew in cheaper-inference#8507) Re-snapshotting them to the current dimensions is still a ratchet DOWN against the release tip, so nothing is loosened -- it is the same tightening this PR is for, taken at the right moment: file release tip this PR (was) now providerPageHelpers.ts 1054 929 949 open-sse/services/combo.ts 3642 3640 3641 translator-openai-responses-req.test.ts 1195 1189 1194 check-file-size --update cannot do this on its own: it only ratchets when there are zero violations, and a grown frozen file IS a violation, so the three had to be re-snapshotted by hand. check:file-size now passes on this branch.
f8ee280 to
c7afea1
Compare
|
Closing as mostly superseded by #8524 (merged in the 2026-07-26 batch). WhyThis PR mixed two things:
Hard-rebasing this PR would re-add files that already exist and fight #8524. No further work planned on this branch. |
What
Ratchets
config/quality/file-size-baseline.jsondown to current code dimensions. The gate only ever tightens — nothing here raises a cap.Measured against the base tip:
frozen(prod)testFrozen<= 800, so they leave the baseline and fall under the normal new-file cap.--updatehandles files that shrank since their last baseline. It cannot remove an entry whereloc == frozen(the removal path only runs for entries inimprovements, i.e.loc < frozen[file]), so the second commit drops three such entries by hand —ProviderDetailPageClient.tsx(798),useProviderModels.ts(155),useProviderSettings.ts(264) — plus two test files that were sitting in the production block while already correctly listed intestFrozen.Stacking
This branch is stacked on #8524 and shares its exact commits (
7e9d2d375,e54213964,b45b0bf72). Those shrink-to-fit changes are a prerequisite: without themproviders/page.tsxis 1990 on the base and the gate cannot go green after the ratchet. Merge #8524 first, then this rebases cleanly onto the new base and reduces to the two ratchet commits.Blast radius — please read before merging
Lowering 78 entries to exactly their current LOC leaves them with zero headroom, so any in-flight PR that adds a net line to one of those files goes red the moment this lands. Cross-referencing the 78 tightened paths against the 60 currently-open PRs: 22 of them touch at least one. Concentrations are
open-sse/services/combo.ts(#8482, #8476, #8494, #8508),open-sse/handlers/chatCore.ts(#8378, #8367),open-sse/handlers/imageGeneration.ts(#8477, #8510), and several dashboard pages (#8450, #8451, #8523, #8526).The baseline's own
_rebaseline_*notes repeatedly say the release captain's rebaseline-at-release supersedes ad-hoc entries, which suggests a wholesale ratchet may be the captain's call on timing rather than something to land mid-cycle. Raising it here rather than deciding unilaterally — happy to hold this until after the current queue drains, or to split it into a prod-only or test-only pass if that is easier to absorb.Test
npm run check:file-size— green on both gates (163 prod frozen, 56 test frozen).npm run check:file-size -- --update— reports no violations and no further entries to ratchet.