Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #495 +/- ##
=========================================
Coverage 99.41% 99.41%
Complexity 1859 1859
=========================================
Files 136 136
Lines 4779 4782 +3
=========================================
+ Hits 4751 4754 +3
Misses 28 28 ☔ View full report in Codecov by Harness. |
Firehed
marked this pull request as ready for review
September 5, 2026 01:03
Firehed
added a commit
that referenced
this pull request
Sep 5, 2026
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). - **39** — `nodeAt` 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. - **44** — `Resolution\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. - **46** — `Index\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. - **52** — `AutoloadFilesLocator` 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Done
src/namesParserServiceexceptServer,KnowledgeStack,FilesystemBackend, andAutoloadFilesLocator— asserted by the newparser servicetransitional row inOneRoutePerFactTest.composer testsuite (parity tests included) is green.ParserServicewith holdersKnowledgeStack,FilesystemBackend, andAutoloadFilesLocator, rootServer, and nothing pending — see the new row inOneRoutePerFactTest.Stacks on
manifest/audit-fixes(targets that branch, notmain).