Skip to content

Manifest audit: fix rows 35-52 and the ledger's layout check - #494

Merged
Firehed merged 7 commits into
mainfrom
manifest/audit-fixes
Sep 5, 2026
Merged

Manifest audit: fix rows 35-52 and the ledger's layout check#494
Firehed merged 7 commits into
mainfrom
manifest/audit-fixes

Conversation

@Firehed

@Firehed Firehed commented Sep 5, 2026

Copy link
Copy Markdown
Owner

Fixes from an audit of steps 35 to 53 against the ledger as shipped, the grid, and the code. The step-35 agent hit the first of these and did not stop; this closes the rest before the next one runs.

Ledger fix (test change)

OneRoutePerFactTest::layoutCheck compared each class's namespace to the interface's full name instead of the interface's namespace, so no family could ever pass layout, including one built correctly from the start. It now takes the interface's namespace as the family and requires that namespace's last segment to equal the interface's short name. Skip messages name the target namespace.

The SymbolBackend row's pending entry for DocumentSymbolSink moves from step-46 to step-43, which now owns the store interface.

A second read of the ledger, prompted by reviewing #495, found one hole and some slop:

  • A route class could name a sibling unreported. The scan exempted every implementation's whole file, so a composite constructing one of its members, or a backend naming another, would have passed. Now a file may name only its own class; naming any other route is a violation. Today's code has none, which confirms the two existing composites take their members through the interface.
  • Fact::transitional is now Fact::confined. Four of the five such rows outlive the rebuild (the vendor parser, name resolution, built-in origin, docblock types), so the shape confines a concrete class to named holders and is not transitional. Manifest, CLAUDE.md, and the review-slice question say "confinement row".
  • The test method is named for both shapes; derived implementations are no longer re-checked with class_exists; each source file is parsed once per run instead of once per row; classesImplementing documents that it reads implements only, which holds while every class in src/ is final.

Row fixes

  • 35 — a transitional row for ParserService, not a family row (one implementation, no composite). KnowledgeStack keeps ParserService because it constructs the two classes that still need parseFile().
  • 36, 51 — composites hold iterable, per the rule.
  • 37 — a test pins that every IncompleteCode/ fixture with a namespace keeps its namespace node through the composite, so first-non-empty cannot lose what the namespace-or-use check kept.
  • 38 — drops a delete target that does not exist (ClassInfoFactory has no text case; the text producer is DefaultTextSymbolExtractor, already listed).
  • 39nodeAt takes the tree. As written it would have reparsed behind the memo, doubled the parse count, and failed the count pin.
  • 40 — states the replacement for EnclosingClassResolver on a synthesized node: Scope::atOffset at the node's position, in ExpressionResolver, which drops the resolver here rather than at 42.
  • 42 — no longer deletes ScopeFinder's parent-chain walks, which Scope::forNode, ExpressionResolver, and MemberAccessDetector use.
  • 43 — replaced. It was throwaway (46 deleted its result). It now holds the DocumentSymbolStore interface, taken out of 46.
  • 44Resolution\NameContext keeps its import tables (SymbolCandidates and ReferenceResolver read them; php-parser's NameContext cannot list aliases) and delegates only the rules. "Before and after" becomes a golden recorded in the step's first commit.
  • 46Index\Symbol stays; every search answer is one.
  • 48 — rerouted for layers. TypeFactory (Domain) and the ClassInfo factories (Repository) cannot reach Resolution, so docblock class names resolve in the Parser tier: a DocblockTypeAnnotator visitor runs with NameResolver, uses the same PhpParser\NameContext, and stores fully qualified type strings as a node attribute. DocblockParser moves to Parser (Lateral). TypeFactory parses type syntax only.
  • 50 — adds a transitional ledger row; one implementation is not a family.
  • 51 — layout stays pending step-52; the row said nothing pending.
  • 52AutoloadFilesLocator implements two family interfaces and cannot satisfy both layouts, so it splits into a locator and a catalog over one shared AutoloadFilesIndex. States that no move needs a deptrac edge (Knowledge → Index and Index → Knowledge both exist; collectors are directory globs).

Policy

A new manifest rule and an enforcement-edits.md row: a file move carries its allowlist paths (Lateral) and adds no layer edge or path; a move that would need one is not made. Moving a file across layers is Lateral only when the ruleset already allows every dependency the class has.

Both are policy edits, so merging is the authorisation.

@codecov

codecov Bot commented Sep 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.41%. Comparing base (7d8678e) to head (9a98915).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff            @@
##               main     #494   +/-   ##
=========================================
  Coverage     99.41%   99.41%           
  Complexity     1859     1859           
=========================================
  Files           136      136           
  Lines          4779     4779           
=========================================
  Hits           4751     4751           
  Misses           28       28           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@Firehed
Firehed marked this pull request as ready for review September 5, 2026 02:40
@Firehed
Firehed merged commit 50beea3 into main Sep 5, 2026
9 checks passed
@Firehed
Firehed deleted the manifest/audit-fixes branch September 5, 2026 02:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant