docs(plans): correct S31's orphan shapes, counts, and AC numbers
What
Step 11's implementation contradicted the merged S31 plan in eight places. This corrects all of them in the plan, so a reviewer of Step 11's MR can tell an accepted trade-off from an oversight.
Corrections are stated where the plan makes the claim rather than substituted silently, per AGENTS.md's rule that a plan's research says so at that point. Every passage carrying a corrected count moves with it: these counts appear in three to five places each, and a single-site fix is what generated eight later blockers on the original plan MR.
The eight corrections
- Two binding-missing orphan shapes, not one. The hosted shape is required, not optional:
internal/format/npm/virtual_assembly.gohas to guardrow.NpmRepositoryIDbefore dereferencing it, so akind=0position whosenpm_repositoriesrow is absent either reaches a constructor of its own or reaches a nil dereference. So five position constructors, two audit reasons, twoReasonvalues, and twoloadAndFilterarms. - Eight foundation edits, not four, in Research Findings, Approach, the ceiling section, Step 11, and Dependencies. The "no signature moves" claim is narrowed to exported signatures and enum meanings, and the internal additions it glossed over (the orphan discriminator, the
Kind()arms, the zero-value predicate) are named. - Two spec amendments riding behind, not one, and both added to the list of divergences
## Open Questionssays are not open questions. - Step 11's sizing is measured, not forecast. 628 source and 1,833 test non-blank non-comment added lines at
edc6336dagainstorigin/main, against the~575/~1,290estimate, plus 452 lines of Markdown the Source and Test buckets never budgeted. The ceiling section now says which reading applies, because### Predecessor sizing, measuredbuckets everything non-generated and non-test as reviewable source, which would read Step 11 as 1,080. Raw--numstat: 23 files, +4,035/-189, of which Go production +977/-188, Go test +2,606/-0, Markdown +452/-1. virtual_memo.goships two per-request adapters, not three. The memoizingremote.Upstreamlanded in Steps 5 and 6 asNewVirtualHostedUpstreamandNewVirtualRemoteUpstream; Step 11 prefetches into them.- The NULL
last_health_statuspremise was false.structure.sqldeclares the columnsmallint DEFAULT 0 NOT NULLanddatastore.NpmVirtualUpstreamRemotetypes itint16, so no NULL can arrive and the explicit-0fill had no trigger. The presence-keyed map survives with the real reason, and the named test becomes the unknown-0case. - Resolution AC numbers drifted by one from position 10 on, when spec commit
0c30f2582inserted "A hosted winner's shed rebuild does not fall through to a remote". This affects Steps 4, 5, 6, 11, and 12 plus two Research Findings passages, not just Step 11. All now cite by name, which is the convention Steps 5 and 16 already use in-tree for this exact reason. - A sixth
npm_virtual_*counter ships from Step 11, outside the spec's five-metric table. Step 18's Scope,## Naming Conventions, and Step 18's catalog-ownership bullet now say "five spec-named families" and record thatmetrics.mdandobservability.mdare already partly written.
New open question
An npm virtual upstream list can name a repositories row of another format. A Maven or container repository has no npm format child by construction, so it arrives with an in-range kind and an absent child, which is byte-for-byte a binding-missing orphan. It is therefore excluded as a data error, and because a data error vetoes the not-found for the whole list, one such upstream turns every otherwise-404 read of that virtual repository into a 503, and a 500 when it is the only position.
Neither S31 nor S13 rules on it: both enumerate the data-error causes as a closed list naming a corrupt rule and an out-of-range kind. The recommended answer in the plan is rejection at the association write path, where the format is known, rather than a read-time carve-out. Paces nothing and blocks no step.
Overlap and ordering
- !2177 (merged) also touches this file, in the Status table only (row 4). This MR touches Research Findings, Approach, the ceiling section, Steps 4, 5, 6, 11, 12, 18, Naming Conventions, Testing Strategy, Dependencies, and Open Questions. The regions do not overlap, so the two merge in either order.
- Step 11's own MR is not open yet. This amendment should land first, so that MR is reviewed against a plan that matches it. Per Guardrail 4 this MR carries no step marker in its title.
- Step 11's Status-table row belongs in !2177 (merged) or its successor when that MR opens, not here.
Review round: four findings closed
A branch review of the first commit found four passages that still stated the
fact this MR is correcting, which is the failure mode the MR itself names.
02c5130c3 fixes each where the plan makes the claim:
- Two passages promised a renumbering note in
## Research Findingsthat did not exist, one of them from inside that section, so the pointer was circular. The note is now written and names spec commit0c30f2582, the criterion it inserted at position 10, and the rule that everything from position 10 on shifted by one. Both pointers link to it. Previously that derivation lived only in this description, which does not survive the merge. - Step 11's
virtual_assembly.gobullet still listed three position constructors. The assembly calls five (virtual_assembly.go:345, 356, 359, 366, 413atedc6336d), including both this step adds, so the one bullet a reviewer is sent to listed the shape the correction was about. - The health-memo counter was described as "registered from
virtual_memo.gointonpmCollectors" in two places, contradicting this plan's ownmetrics.gobullet. It is declared invirtual_memo.goand registered by the one-line addition tonpmCollectorsinmetrics.go. Step 18's "catalogued there" pointed at the wrong file too. This is what GitLab Duo's two threads flagged as ambiguity; against the code it was a contradiction. orphan_upstream_shapes.mdwas introduced as the single home for the orphan row shapes.internal/datastore/npm_virtual_resolution_contract.mdhas carried the row-level reading, and a cross-format integration subtest seeding a Maven row with no child, from Step 4 onward. The new sidecar is now scoped to the position layer and links to it rather than restating it, and the new open question says the row is not the discovery, only the ruling is.
Also in that commit: the foundation-touch summary moved back under resolve.go, where the four bullets it describes are, rather than under the sidecar bullet it had drifted into; virtual_assembly_refusal_test.go and virtual_remote_upstream.md join Step 11's Files list, both described from the files themselves rather than guessed; the virtual_memo.go bullet says why three adapters are listed under a count of two; and the numstat breakdown carries the Markdown -1 its own total already counted.
Diff size
Past the 500-LOC guardrail at 635 reviewable lines (+465/-170 against the merge base), and not splittable: it is one Markdown file, and the whole point of the change is that a corrected count has to move in every passage that carries it at once. docs/dev/development-model.md scopes the ceiling to reviewable code; this diff is prose, with no Go, schema, or configuration in it.
Testing
Documentation only, no code. markdownlint-cli2 (0 issues), vale (0 errors), and lychee --include-fragments (43 links OK, 0 errors) all pass. lychee is what confirms both the renamed ## Approach heading anchor and the new ## Research Findings subsection anchor resolve from their inbound links; a negative control that breaks the new anchor makes lychee exit 2, so the check genuinely runs.
Related to #892 (closed)