[WRONG BRANCH] fix(scripts): name the size exemptions after the reason they exist (#4706) - #4757
Conversation
…4706) [skip ci] GENERATED_PATHS exempted twelve files from every size cap, and eleven of them were hand-maintained: nine i18n catalogues, a hand-curated benchmarks snapshot, and the model-metadata generator's INPUT. Its output, src/generated/model- metadata.ts, is 108 lines and was scanned normally, so the one file the list was named after was the one file it did not describe. Exempting catalogues and data snapshots is a reasonable policy. Calling them generated is what invites the next hand-written file onto the list, because a name is a claim nothing checks. The exemption is now three exact allowlists, each carrying its own reason. GENERATED_PATHS holds only agent_pb.ts, which opens with a protoc-gen-es banner. I18N_CATALOG_PATHS holds the nine locale catalogues, exempt because they grow by one line per UI string in nine locales at once, so a cap would block every new string in the GUI rather than any oversized module. DATA_SNAPSHOT_PATHS holds the two records whose size tracks how much was recorded. EXEMPT_PATHS is their union, and the verdict and baseline field are renamed from GENERATED to EXEMPT to match. loadBaseline still accepts the old key so a branch written before the rename loads instead of failing with a shape error that explains nothing. The classification is now checkable against the files themselves: every path in GENERATED_PATHS must carry a generator banner, and no path in the other two lists may. The positive control is the generator's real output, which carries the banner, is not exempt, and is scanned under a cap. The exemption relies on --update never turning an exemption into a cap, so that is asserted directly alongside the Math.min rule that only ever lowers one. Closes #4706
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Deterministic PR hygiene checks passed. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
리뷰 · 우선순위 76 / 80이 PR은 파일 크기 래칫( 베이스 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
…de splits (#4711) The architecture pages and structure/runtime.md still described the module ownership that existed before the facade splits of the last release train, so a reader following them landed in a file that no longer contains the code. Routing now lives in src/server/index/serve-options.ts, request preparation in src/server/responses/request-prepare.ts, and bridge conversion in src/bridge/ sse.ts and src/bridge/response-json.ts. src/bridge.ts is seven lines of re-exports. The pages named the pre-split files in every locale, so fixing only the English source would have left seven translations contradicting it. The facade paragraph also carried three counts -- seven, nine and five leaf modules -- that were already 26, 42 and 53. Counts that must be recounted on every split are a drift source rather than information, so they are gone; what replaces them is the distinction the counts were standing in for. A facade is the stable import path, not the implementation, and each step of the request flow now names the module that owns the code. structure/runtime.md gets the same correction on five ownership claims, and points at structure/transports/responses.md, which already carries the post-split owner inventory for the Responses surface. Scope is deliberately narrow: only claims about which module owns which responsibility change. No prose about behaviour is rewritten, and no locale receives a translation it did not already have. Closes #4711
docs(architecture): name the modules that own the code after the facade splits (#4711)
|
Cascading downward. The file-size ratchet's generated exemptions split into three exact allowlists, validated by the files themselves rather than by name: only the generated list may carry a generator banner, and the other two must not. Evidence at the verified tip baaf691 (tree
Chained-child stacks merge top-down, so this lands in the parent branch and cascades to Maintainer integration decision under MAINTAINERS.md / AGENTS.md: a maintainer with maintain or admin access may integrate into |
9af2b0c
into
codex/rg2-lab-sync-activation-guard
github-actions
Bot
changed the title
⏳ DRAFT
What to do
Its title has been prefixed with |
1 participant
Summary
GENERATED_PATHSexempted twelve files from every size cap, and eleven of them were hand-maintained: the nine dashboard locale catalogues (endescribes itself as the TKey source of truth), the hand-curatedfrontier-benchmarks.json, andscripts/model-metadata.source.json— which is the generator's input. Its output,src/generated/model-metadata.ts, is 108 lines and was scanned normally, so the one file the list was named after was the one file it did not describe.Exempting catalogues and data snapshots is a reasonable policy. Calling them generated is what invites the next hand-written file onto the list, because a name is a claim that nothing checks.
The exemption is now three exact allowlists, each carrying the reason it exists:
GENERATED_PATHSsrc/adapters/cursor/gen/agent_pb.tsI18N_CATALOG_PATHSDATA_SNAPSHOT_PATHSfrontier-benchmarks.json,model-metadata.source.jsonEXEMPT_PATHSis their union. The verdict and the baseline field are renamedGENERATED→EXEMPTto match;loadBaselinestill accepts the old key, so a branch written before the rename loads instead of failing with a shape error that explains nothing.The classification is now checkable against the files themselves rather than against its own name: every path in
GENERATED_PATHSmust carry a generator banner, and no path in the other two lists may. The positive control sits on the other side — the generator's real output carries the banner, is not exempt, and is scanned under a cap.Because the whole exemption rests on
--updatenever turning an exemption into a cap, that is now asserted directly, alongside theMath.minrule that only ever lowers one.No file's effective size policy changes: all twelve paths remain exempt, the repository scan is unchanged, and no interface file is modified by this PR.
Closes #4706
Verification
Local test suites, individual test files,
bun run typecheck,bun run test,bun run test:changed, the dashboard build and dependency installation were NOT RUN — the maintainer forbids local suite execution in this lane, since a local test run previously deleted real~/.opencodexdata.What was done instead:
file-size ratchet passed.scanRepo+evaluatereport zero offenders, and theEXEMPTrows equalEXEMPT_PATHSexactly. That scan includes the files this lane's earlier layers add./@generated|DO NOT EDIT|Do not edit/in its first twelve lines, andsrc/generated/model-metadata.tsmatches as the positive control.scripts/andtests/:structure/,docs-site/,.github/andAGENTS.mdname neither the constant nor the baseline field, so the rename has no documentation surface to drift from.[skip ci]under the maintainer-approved DEV-STACK-08 tip-only policy.Checklist