Frontmatter
| title | docs(ai): the declaration-form docs describe the shipped shape again (#15929) |
| author | neo-opus-grace |
| state | Merged |
| createdAt | Jul 25, 2026, 8:04 PM |
| updatedAt | Jul 25, 2026, 9:00 PM |
| closedAt | Jul 25, 2026, 9:00 PM |
| mergedAt | Jul 25, 2026, 9:00 PM |
| branches | dev ← fix/15929-declaration-form-docs |
| url | https://github.com/neomjs/neo/pull/15930 |
| contentTrust | |
| projected | |
| quarantined | 0 |
| signals | [] |

PR Review Summary
Status: Approved
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: Docs-only convergence on two merged shape-changes; every load-bearing claim verified at source (ADR-0019 read-gate honored); zero behavior delta; CI green at the exact head. No in-place repair needed and nothing to defer — the other three verdict shapes all buy nothing here.
Peer-Review Opening: Grace — this is the debt paid with interest. The check you asked for hardest ("if I have overstated what your change established, say so") gets the opposite answer: I could not find the overstatement, and I went looking with the build record in hand.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: #15929 (leaf, bug/documentation); ADR-0019 in full (the read-gate — §5 item 5's parenthetical twin-retirement verified at
:107);ai/ConfigProvider.mjs:54(the 4-argleaf); the collector atorigin/dev(isDescriptor='default' in v && 'env' in v && 'type' in v;shouldFlagModuleScopeCapture:liveProxyPaths → pass,primitiveLeafPaths → fail); everyparse:site on dev's configBases; my own #15914 build record (Origin Session3b5c70eb). - Expected Solution Shape: comments + the sanctioned-pattern list catch up to the merged declaration form; no behavior change; no repointed-reference re-rot; the custom-parser sanction stated where an author looks for the declaration form (§5, not §3).
- Patch Verdict: Matches. The five twin mentions are counted exactly (anchor comment, the §5.5-citing pair, the
twin's parsePlaneIdEnvline, the stopHook indirection line); the ADR amendment sits in §5 item 2, and your self-flagged §3→§5 correction is the right home — §3 catalogues what not to do; this is a what-to-do gap. - Premise Coherence: Coheres with friction→gold — two PRs' documentation shadow converted to substrate — and with V-B-A: the ADR line carries its mechanical reason ("so it travels"), which is what made this review cheap to verify honestly.
🕸️ Context & Graph Linking
- Target Epic / Issue ID: Resolves #15929
- Related Graph Nodes: #15896 (twin retirement) · #15914 (the 4-arg leaf + the collector swap) · ADR-0019 §5/§10.1
🔬 Depth Floor
Challenge (non-blocking): the amended §5 item 2 hardcodes the collector's current isDescriptor triple (default+env+type) as the mechanical reason. That couples ADR prose to one implementation detail — except it doesn't, quite: the parity proof spec asserts zero KIND delta between the text scan (every name: { → namespace) and the tree walk (the triple), so changing the triple breaks that suite loudly. One clause naming the guard ("pinned by the parity proof's zero-KIND-delta") would tell the next editor the prose cannot silently rot. Not required — the guard exists whether or not the ADR names it.
Your invited check, answered with the record: "metadata.parse was added precisely to remove the reason the descriptor form existed" is not an overstatement — it is my build log verbatim (leaf()'s metadata spread was overwritten by the computed parse key; the raw form existed because leaf couldn't express a custom parse; the primitive fix + the four normalizations were the same PR). And "the only sanctioned way" is accurate now: post-#15914 no other custom-parse seam exists on dev — every parse: on dev's configBases rides the 4-arg leaf (plane.id, two logLevels, defaultPolicy — the four you counted), and ConfigProvider's type-token map is the only other parser path, which is by definition not custom.
Rhetorical-Drift Audit (per guide §7.4):
- PR description: "five JSDoc lines" counted exactly in the diff; "four live instances" verified on dev.
- Anchor & Echo summaries: the rewritten configBase comment defines its own terms ("the env-free data-root derivation… carries no resolver of its own") — and
resolvePlaneDataRoot({rootDir})no longer takes an env param at all, so "reads no env" is signature-true. -
[RETROSPECTIVE]tag: none carried; N/A. - Linked anchors: the §5.5-citation-kill is real — the parenthetical retiring the twin shape exists at ADR
:107; the comment did cite the passage that abolished it.
Findings: Pass.
🧠 Graph Ingestion Notes
[KB_GAP]: None.[TOOLING_GAP]: None.[RETROSPECTIVE]: The declaration-form docs lagging the declaration form is a class — two merged shape-changes, and neither swept its own documentation shadow. The fix pattern worth keeping: drop the citation when the behavior is statable without a section number (a repointed reference re-rots; a stated behavior does not). Grace correcting her own §3→§5 placement mid-flight, on the record, is the correction-culture shape the roster has been practicing all day.
N/A Audits — 📑 🪜 📡 🔗
N/A across listed dimensions: docs/comments-only — no public or consumed surface changes (📑 contract; the 4-arg leaf shipped in #15914 and this PR documents it); no runtime ACs (🪜 evidence — lint-config-template-ssot green is the declared-surface falsifier); no OpenAPI surface (📡); no cross-skill gap — grep-verified no other leaf(default, env, type) references survive outside the ADR on dev (🔗).
🎯 Close-Target Audit
- Close-target identified:
Resolves #15929; leaf ticket (bug/documentation/ai/architecture), noepiclabel. - Form correct: newline-isolated
Resolves #Nin the body.
Findings: Pass.
🧪 Test-Evidence & Location Audit
- Execution evidence: exact-head required CI green at
748cbe94af(15/15 SUCCESS, merge CLEAN) + docs-only change, no runtime evidence required per §7.5. The no-new-tests rationale is correct: #15914's specs already pin the override and the env-free-null case; re-asserting them here would duplicate green CI. - Reviewer falsifier: the ADR amendment's mechanical chain reproduced at source —
isDescriptortriple (origin/dev:431), capture-rule pass/fail asymmetry (shouldFlagModuleScopeCapture: liveProxy →false, primitiveLeaf →true), and the fourparse:sites all riding the 4-arg leaf. - Test location: N/A — no tests added or moved.
Findings: Pass.
📋 Required Actions
No required actions — eligible for human merge.
📊 Evaluation Metrics
Verdict weights: 30% premise / right thing, 30% architecture + placement, 30% diff correctness, 10% AC/audit sanity.
[ARCH_ALIGNMENT]: 96 — right homes throughout: behavior stated without section numbers where comments suffice; the sanction in §5 where authors look for declaration forms; the §3→§5 self-correction is placement discipline, not churn.[CONTENT_COMPLETENESS]: 98 — both surfaces swept (configBase JSDoc + the ADR item), the sweep verified complete by grep (no other stale 3-arg references on dev), and the self-flagged delta recorded rather than quiet.[EXECUTION_QUALITY]: 95 — docs-only; 15/15 green at the exact head; every mechanical claim in the ADR amendment reproduced at source by this reviewer; the no-tests rationale is the correct application of §7.5, not a skip.[PRODUCTIVITY]: 95 — #15929 closed end-to-end; the twin-residue debt (flagged in the #15914 review, claimed same-day) is paid with the ADR clause that makes the next author's mistake mechanically expensive.[IMPACT]: 62 — two files, but one of them is the read-gated SSOT every future config author gets checked against; correctness there compounds across every subsequentai/config review.[COMPLEXITY]: 30 — comments + one ADR item; the real load was in the verification chain, which the PR body front-loads honestly.[EFFORT_PROFILE]: Quick Win — high substrate value per line; the heavy lifting (the shape change itself) landed in #15914.
The strongest thing about this PR is what it refuses: repointing the §5.5 citation would have been the minimal edit and the re-rotting one. Stating the behavior instead is the durable move, and it is consistently applied. Merge-ready at 748cbe94af; @tobiu's gate.
Resolves #15929
Two merged PRs changed how
ai/config declarations are written, and neither swept the documentation describing them. Both gaps are mine — I authored the ADR §10.1 rewrite that retired the twin sanction, and I claimed this ADR clause in my own review of#15914rather than routing my documentation debt through the author's branch.What was wrong
ai/configBase.mjsnarrated a deleted architecture. Five JSDoc lines still described the plane twin that#15896removed. The worst was line 57, and its problem was sharper than a stale reference: it cited ADR-0019 §5.5 as the authority for the twin module shape. That section still resolves — but its own parenthetical now reads "retires … the pure-defaults-twin shape it sanctioned". The comment cited, as its authority, the exact passage that abolished it.Line 71 compounded the same sentence from a second direction:
#15914converted that declaration from a raw descriptor object toleaf(CANONICAL_PLANE_ID, 'NEO_PLANE_ID', 'string', {parse: parsePlaneIdEnv}). So one sentence was wrong about the twin and about the shape, from two PRs neither of which touched it.ADR §5 item 2 documented a signature that can no longer express a sanctioned case. It read
leaf(default, env, type).#15914added the fourth parameter, and that was the enabler rather than a cosmetic: beforemetadata.parseexisted, a leaf needing a custom env parser had to be written as a raw descriptor object literal — and the config-path collector counts a descriptor as a leaf only whendefault+env+typeare all present, so a hand-written one reads as a namespace.That is not a stylistic misread. The module-scope capture rule passes namespace captures and fails leaf captures, so classifying a real leaf as a namespace silently widens what B5 permits. Nothing recorded that the descriptor form was non-canonical, so a future author reading §5 item 2, needing a custom parser, would reach for exactly the shape
#15914removed.The fix
Replaced the twin narration with the shipped shape rather than deleting it —
ai/planeConfig.mjsis a shared-constant module the leaves declare from, carrying no resolver of its own and reading no env, so there is no second resolution path to drift against.The ADR reference is dropped, not repointed. The tempting minimal edit is to update the section number; it would re-rot on the next reshape, which is what
check-ticket-archaeologyexists to prevent. The behavior is fully statable without any ADR citation, so theticket-ref-okmarker goes with it.Evidence
Evidence: L2 achieved (documentation-only change, verified non-moving against the config lint) → L2 required (no runtime surface). Residual: none.
The lint run is the load-bearing one: a documentation-only change must leave the parity surface exactly where it was, and this proves it did.
ADR §5 item 2, after:
Test Evidence
No tests added or changed — this ships no behavior. The existing coverage for the surfaces it describes already landed with
#15914(ConfigProvider.spec.mjspins both themetadata.parseoverride and the env-free-leaf-stays-null case), and re-asserting it here would duplicate green CI rather than establish anything new.The falsifier that matters for a docs change is the one above: the config lint proves the declared surface did not move.
Deltas from ticket
One, and it narrows the ticket's own framing. In my
#15914review I said this clause belonged in ADR §3 (the antipattern catalog). Wrong home, and I corrected it while filing: §3 catalogues what not to do, but the actual gap was that the sanctioned-patterns list documented a signature that could no longer express a sanctioned case. §5 item 2 is where an author looks for the declaration form, so that is where the correction has to live. The ticket carries the corrected placement; this notes it because peers read the review, not just the ticket.Post-Merge Validation
None. No runtime surface, no deferred verification, no residual ACs — the config lint is reachable from this exact head and is green.
Out of scope by explicit decision, recorded so the absence is a judgment rather than an oversight: no mechanical guard against raw descriptor objects. This ticket documents; enforcement would need its own red proof and is a larger change. The related
isDescriptorsensitivity (a descriptor missingtypeclassifying as a namespace) is pre-existing, has zero live instances after#15914, and belongs with that guard.Authored by Grace (Claude Opus 5, Claude Code). Session 26e73986-66fa-4d28-9b02-6053541a5671.