ci(config): check the manifest field list against what the parser reads - #9980
Conversation
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-31 02:45:08 UTC
Review summary Nits — 6 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. Decision record
🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
loopover-ui | cef3a55 | Commit Preview URL Branch Preview URL |
Jul 31 2026, 02:31 AM |
Bundle ReportBundle size has no change ✅ |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9980 +/- ##
=======================================
Coverage 91.88% 91.88%
=======================================
Files 928 928
Lines 113722 113722
Branches 27436 27436
=======================================
Hits 104489 104489
Misses 7936 7936
Partials 1297 1297
Flags with carried forward coverage won't be shown. Click here to find out more. |
FOCUS_MANIFEST_TOP_LEVEL_FIELDS' own header says "Every top-level key parseFocusManifest below actually reads." That is a statement about code twenty lines further down, and nothing enforced it -- the same shape as the turbo.json inputs comment #9860 calls out as the tell: a snapshot written as a guarantee. The list is the single source of truth for unknown-field detection (#9065), so drift is silent in both directions and asymmetrically bad: DECLARED BUT NEVER READ -- the parser accepts the field, warns about nothing, and ignores it. An operator sets it in .loopover.yml, sees no error, and the setting does nothing. This is the worse direction: it looks configured. READ BUT NOT DECLARED -- the parser consumes the field while warning "unknown top-level field" about it, and config-lint's recognizedFieldsFor leaves it out of the recognized count. A working field reported as a mistake. Adding a field means editing two places twenty lines apart, and #9813 and #9821 each shipped a bug from missing one touchpoint of a multi-touchpoint field. Computed, not restated: the declared names come out of the array literal and the used names out of the function body, so this file holds no second list and cannot itself go stale. It is a regex over source rather than the TS AST, which is a real tradeoff. The access pattern is uniform and mechanical, and every extractor THROWS when the shape it depends on is missing -- a renamed constant, a moved signature, or a refactor to destructuring all fail loudly instead of matching nothing and reporting "no drift". A guard that passes vacuously is worse than none, because its presence reads as coverage. Holds today at 28 declared and 28 read, empty both ways, so this lands green and guards forward. Mutation-tested: declaring an unread field, reading an undeclared one, and removing a still-read declaration each fail with the field named. Closes #9979
598f2ab to
cef3a55
Compare
![loopover-orb[bot]](https://avatars.githubusercontent.com/in/4139483?s=60&v=4){kind=small}
1 participant
Item 4 of #9860, scoped to its one currently-unverified claim. Filed and shipped separately because #9860 explicitly asks for one independently-checkable change per PR.
The claim nothing was checking
That first sentence is a fact about code twenty lines further down. It is exactly the shape #9860 calls out as the tell — a snapshot written as though it were a guarantee.
Why drift is silent, and worse in one direction
This list is the single source of truth for unknown-field detection, so neither direction surfaces anywhere:
.loopover.yml, get no error, and it does nothing. It looks configured.config-lint'srecognizedFieldsForomits it from the recognized count. A working field reported as a mistake.Adding a top-level field means editing two places twenty lines apart — and #9813 and #9821 each shipped a bug from missing one touchpoint of a multi-touchpoint field.
Computed, not restated
There is deliberately no second list. Declared names are read out of the array literal; used names out of the function body. The checker holds nothing to keep in sync and cannot itself go stale — the bar #9853 set.
On using a regex rather than the AST
A real tradeoff, stated rather than glossed. The access pattern is uniform and mechanical (
record.<field>, inside one function). The risk with any regex guard is that it silently matches nothing and reports success, so every extractor throws instead:could not find the FOCUS_MANIFEST_TOP_LEVEL_FIELDS declarationcould not find export function parseFocusManifestconst { gate } = record) →the access pattern this checker relies on has changedThe body is also bounded at the next top-level
export, becausefocus-manifest.tsdeclares many sibling parsers that all destructure a local namedrecord; reading past that boundary would pull their field names in and report permanent phantom drift. Both are pinned by tests.It holds today
28 declared, 28 read, empty in both directions — so this lands green and guards forward, rather than arriving after the next drift.
Mutation testing
ghostField, never read itghostField, tells you to read or remove itrecord.orphanField, never declare itorphanField, tells you to add itpublicProofwhile the parser still reads itpublicProofBaseline restored:
28 top-level field(s) declared and read; no drift.Plus 15 unit tests, including the two "guard the guard" cases: that both extractors return a non-trivial number of fields, so the real-file invariant cannot pass on an empty set.
Verification
typecheck,checkers-wired:check,dead-source-files:check,dead-exports:check,import-specifiers:check,coverage-boltons:check,validate:no-hand-written-jsgreentest:ci;checkers-wired:checkconfirms it actually runsCloses #9979