fix(819): derive the SPA page-size guard population from the git index #875
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: timothy/ersatztv#875
Reference in New Issue
Block a user
Closes the #806 residual that PR #818 explicitly deferred.
The defect
web/src/api/pageSizeCallSites.guard.test.tsasserts EXACT completeness (a multiset, bothdirections) over the
pageSizecall sites it discovers — but discovered the files withimport.meta.globoverweb/src. A directory walk is not an authoritative population, so anuntracked
.ts/.tsxentered it and failed as unregistered on that checkout, while CI — which onlyever checks out tracked files — stayed green. Developer-red / CI-green, the #778 shape #806 removed
everywhere else.
Witnessed before the fix: an untracked
Scratch819Untracked.tsxholdingpageSize: 25gaveUNREGISTERED (1) — screens/Scratch819Untracked.tsx:literal:25.The fix
The glob stays, as the source of file content. The population is the git index, read by
web/vite-plugins/trackedSourceFiles.tsin Vite's own Node context and handed to the app project asa virtual module — which is how the index is reached without admitting
@types/nodetotsconfig.app.json, the obstacle that deferred this.It fails rather than degrades (a failed or empty
git ls-filesthrows) and reads the indexlazily, so
vite buildnever shells out to git.Three mechanisms carry the proof, each added because the previous one was measured insufficient:
depth, with the predicate it checks — anything shared sits on both sides of the comparison and
cancels.
ls-files --others) cross-checks the population: everyin-scope walked path must appear in
tracked ∪ others. Narrowingtrackedcannot suppress that,because it adds nothing to
others.fake runner.
Why it took sixteen review rounds
Every early round found one mechanism: the proof shared something with its subject, so a
narrowing cancelled and went green. Each fix reproduced it one layer down — the scope, then an
example table (4 of 8 directories), then a delegated sub-predicate, then a
basenamehelper, thenthe population array itself. Two of those were blind: they hid a planted call site with the whole
suite green.
What terminated it was stating the criterion as a checkable property rather than an instruction,
and — where a restatement cannot police the population it reads — cross-checking against a genuinely
different query.
Verification
git added reddenscore.ignorecasespelling divergence each redden the shortfall testgit rm --cachedall stay green.gitabsent andgitoffPATH: rc=0, 117 files / 1226 testsnpx vitest run119 files / 1272 tests ·tsc -b --force·eslint .·npm run build·pytest scripts/tests1228 passed ·decisions_validateOK · catalog zero-diff.Residuals — six, each with its direction MEASURED
Stated rather than implied, because fail-direction is the argument for tolerating a residual:
(2), (4), (5) are fail-noisy; (1) (a three-site coordinated edit) and (3) (a
union-preserving mispartition) are blind; (6) (watch-mode staleness) is each in turn. The
obvious fix for (6) —
configureServerinvalidation — was implemented, measured, and rejected:git addfires no watcher event, so it blinds the create-then-stage sequence.Docs
docs/guard-inventory.md(audit row + scope item),docs/testing.md,docs/spa-conventions.md(the new
web/vite-plugins/seam), andtesting.guard-derives-population-from-source— whichgains a bounded exception and the closed-form criterion, with the catalog regenerated.
fixes #819
Round 3's cold review found the same defect class for the third round running, and this time in the fix for round 2's finding. That fix pinned the shared `isInScopeSourcePath` with a hand-written table of 8 example paths. The table names four of `src/`'s eight directories, so it proved only the one mutation the previous round happened to use. MEASURED: narrowing the predicate to skip `media|app|mediaSources|schedules` removed 23 real, tracked, on-disk files — 16% of the SPA — with all 35 tests green, and `/media/` alone was green too. That is precisely what the rule it was meant to satisfy calls out: "a hand-written literal list of members is the same defect in slower motion — a filter frozen at authoring time, correct on the day it was written and unable to report the day it stopped being." So the mechanism is deleted rather than extended. The scope is now proved by DECOMPOSITION over the whole tracked population: the predicate must equal its two documented components — `!includes('/generated/')` and `isScannableSourceFileName` — each already independently pinned by the 14-case filename table. Any new term, naming any directory or any filename shape, makes the two sets differ. MEASURED red against all four mutations above plus a filename-shaped one (`!endsWith('.d.ts')`) that a path table could never have seen. The table stays as worked examples and now says so instead of claiming to be the proof. Also from round 3: - The anti-vacuity floor's stated reason was falsified by the commit that wrote it: with the fixture test present, `expectedTrackedSources -> []` reddens even without the floor. Restated as what the floor uniquely covers — vacuity in the REAL plugin output, which injected fixtures structurally cannot reach — and the floor is now derived from the index size rather than a literal that was true once. The stale "1249-test suite" figure is gone with it. - `testing.guard-derives-population-from-source` states "assert existence rather than filtering on it" in its `rule:` field, so the catalog row a reader resolves by topic said the opposite of what this guard does. The record now carries the bounded exception — filtering on existence is admissible only where the OTHER direction reports the on-disk hole, so the absent member stays representable — and the catalog is regenerated. Without that pairing it remains the original defect. - Three cross-references the refactor invalidated: the `/generated/` exclusion moved to `isInScopeSourcePath`, and one comment said "above" of a predicate defined below. - guard-inventory: the docstring quote is attributed to `test_every_index_derived_module_is_registered` rather than the module, with the elided `Path.rglob` clause restored, and the over-long line rewrapped. Refs: #819 Decisions-Edit: yesRound 4's cold review found the recurring class had moved one level down rather than terminated. The decomposition restated the `/generated/` half inline but DELEGATED the filename half to `isScannableSourceFileName` — the same function `isInScopeSourcePath` calls — putting it on both sides of the comparison, where it cancels algebraically. Exactly the cancellation that hid the walk-vs-index narrowing in round 2 and the directory narrowing in round 3. MEASURED survivors on the previous head, whole suite, no filter: adding `&& !name.endsWith('.d.ts')` to that predicate dropped a real tracked file (`src/vite-env.d.ts`) and a term skipping lowercase-initial `.tsx` dropped eleven — 8% of the in-scope SPA — both with all 1259 tests green. Reproduced here before fixing. Both now redden `the shared scope predicate is EXACTLY its two documented components`. The fix is to restate the filename predicate inline too, so the decomposition delegates nothing. What that is, said plainly instead of overclaimed: a verbatim restatement of a FIXED-SIZE predicate checked against itself every run — the `MARKED_JOBS` shape the record already blesses — which does not decay as the SPA grows, unlike a list of members or example paths. Two residuals are now stated rather than implied: an author editing the predicate AND the restatement together passes deliberately, the two-site edit being the signal; and a term matching zero tracked paths survives until the day it first matches one. So the claim is "a term becomes visible the moment it affects a tracked path", not the previous "regardless of what it excludes", which round 4 measured false. Also from round 4: - `docs/guard-inventory.md` still named the table as the proof — the mechanism the previous commit retracted in code but did not sweep out of the docs. It now describes the decomposition, why both halves are restated, and the residual. The same retracted phrase in the walk-side code comment is gone too; grepping the subject ("table of cases") found both, which is the sweep that was missed. - The anti-vacuity floor was bounded by every tracked path under `src/` (260), a denominator that grows with test files while the numerator counts sources. It is now bounded by the IN-SCOPE count, so at most a tenth of those may be absent from disk before the plugin result reads as degenerate. - The Dockerfile comment named two gates on `Test SPA` and argued only one; the `docs_only` arm holds because `ci-detect-docs-only.sh` classifies any `web/**` path as not-docs-only, so a tree taking that skip has an SPA subtree already tested. - "~260 SPA sources" read as the guarded population in both the record and the guard comment; it is every tracked path under `web/src/`, of which ~141 are in scope. A worked example named a file that does not exist. Refs: #819 Decisions-Edit: yesRound 6's cold review found the class had moved from the scope predicate into the POPULATION SOURCE. Every comparison in the guard was between two things derived from `trackedSources.tracked` — the scope restatement, the shortfall direction, the floor's denominator — so a filter applied inside the plugin shrank both sides of all of them and cancelled. MEASURED on the previous head: adding `&& !path.startsWith('src/components/')` to the plugin's split dropped 8 real in-scope files, and a `pageSize: 100` planted in one of them went entirely unseen — 46/46 green. Reproduced here before fixing, and again on a second subtree (`/rules/`, 7 files). The fix is not another restatement. The plugin now runs a SECOND, independent query — `ls-files --others`, deliberately without `--exclude-standard` — and the guard asserts that every in-scope path the walk found is accounted for by one list or the other. `tracked ∪ others` is what git says is on disk, so narrowing `tracked` cannot suppress the signal: it adds nothing to `others`, and the file lands in neither. An untracked scratch file stays green, which is the #819 property itself. MEASURED red for both narrowings, including with the rogue site planted; MEASURED green for the untracked plant, an unstaged deletion, and a staged rename; MEASURED red still for a tracked plant and a tracked dotfile. The plugin also gains what it never had: tests that EXECUTE the real derivation against a real repository — a temp repo with one committed file and an identical untracked sibling, a working-tree deletion, and a non-repo — instead of only injecting a fake `GitRunner`. That is the point `test_guard_populations_derive_from_git.py` makes about its own primitive: it proves the mechanism by running it rather than by recognising its shape. Also from round 6: - `expectedTrackedSources` was still proved by a seven-path fixture table — the mechanism this PR deleted one function over. MEASURED: a `/media/` term inside it left the suite green. Now restated closed-form like the scope predicate. - The anti-vacuity floor had no clause proof and its failure direction is fail-OPEN, unlike `ABSENT_FROM_DISK`'s. Extracted as `populationIsDegenerate` so the threshold is a pinnable function, with the tight-threshold cost stated. - `basename` is deleted, not just labelled. It existed only as the trap the comment warned about, and the two-site "tidy" edit still survived. Inlined into its one caller, so there is no shared helper to reintroduce. - The closed-form criterion — the whole termination argument — lived only in the record's `signals:` line, which the catalog does not render. It is now in the `rule:` field and the record body, so a reader resolving the key by topic sees it. - A subsumed clause is mirrored deliberately and now says why; session chronology in durable comments is dated to #819 rather than "the previous head". Refs: #819 Decisions-Edit: yesRound 11 found the same defect class as round 10, one clause over: the previous head grouped residual (1) with the fail-noisy ones. It is blind. MEASURED here before fixing, planting `pageSize: 100` in the tracked file `src/media/HlsPlayer.tsx`: * plant alone -> UNREGISTERED (1), red * plant + TWO-site edit -> RED. `supplies content for every tracked source file on disk` catches it, because `expectedTrackedSources` carries a SECOND closed-form restatement the two-site edit does not touch. * plant + THREE-site edit -> 43/43 GREEN, call site hidden. So two claims were wrong, not one. The direction ("cannot hide a call site") and the extent ("the two-site edit being the signal") — the scope predicate has three restatement sites, at `isInScopeSourcePath`, this restatement, and the one guarding `expectedTrackedSources`, and four when a term also moves a worked example. Twice now I have written a fail-direction I had not measured, in a change whose whole subject is the difference between coverage and the appearance of it. So the directions are no longer stated as a group: each residual carries its own, each was measured by planting a real call site, and the block says so. (2), (4) and (5) are noisy; (1) and (3) are blind, tolerated for different and explicitly non-"harmless" reasons — (1) because a three-site policy edit is review-visible where a one-line slip is not, (3) because a mispartition preserves the union any consumer compares and is answerable only by testing the derivation. Also from round 11: - The residual paragraph had grown a verbatim duplicate of its own last two claims, eight lines apart — the second-copy-of-a-corrected-statement shape the decision record this branch amends warns about, and this branch has already lost a round to a claim drifting between copies. `docs/guard-inventory.md`'s row had the same duplication; both consolidated. - The `isInScopeSourcePath` filter on `walked` survives deletion. It only WIDENS the set and every extra path is tracked, so it cannot hide anything — recorded as a narrowing convenience so a later round does not re-litigate it. Refs: #819Round 12 found a BLOCKER, and it is the third instance of one habit of mine in this change: an affirmative coverage claim I had not measured. The guard said "Every mutation that could make it blind runs through `expectedTrackedSources`, which the fixture test pins." MEASURED FALSE, and reproduced here before fixing. Widening the plugin's own derivation by `|| path.startsWith('src/components/')` does not run through that function at all — the fixture test INJECTS `absentFromDisk`, so it structurally cannot police what the plugin derives. Those files are then subtracted from `expected`, which disarms the hole-detection direction: a tracked `src/components/.probe819.ts` holding `pageSize: 100` goes from red — named correctly by the shortfall test — to 43/43 green. The co-occurring condition is not exotic. It is exactly the case the shortfall test exists for and that this repo has measured as PERMANENT rather than transient: a dotfile, a dot-directory, or a disk/index spelling divergence under `core.ignorecase` (true on this machine) or NFD/NFC. CLOSED rather than merely stated, since one assertion does it: a path the WALK found is on disk by construction, so it can never legitimately be reported absent. MEASURED green on a clean tree, and red by name under the mutation. What that closes and what it does not, measured both ways and written down: it catches any fattening overlapping a walked file. A fattening restricted to paths the walk cannot see anyway stays inside residual (3) — which is widened from "the plugin's two lists partition faithfully" to "the plugin's THREE outputs describe the repository faithfully", since `absentFromDisk` is an output the pair-shaped wording did not cover. Also from round 12: - A pointer said "see residual (3)" as though it were the only blind one; (1) became blind last round. - "every extra path is tracked" is inexact on a developer checkout — an untracked out-of-scope file lands in `others`, so it is accounted for but not tracked. The conclusion holds; the reason is now stated exactly. Refs: #819Review-verdict: MERGEABLE @
bd69b4f16 cold review rounds; final two independent reviews returned MERGEABLE with nothing at any severity. Mechanism re-verified end to end: all six residuals measured by planting real call sites, Docker stage replicated with both .git absent and git off PATH, full mutation battery, all gates green.