PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

• Decision: Request Changes
• Rationale: The durable store, graph-domain placement, and ADR-0019 use-site boundary are worth keeping. The current producer cannot emit the exclusion-bucket evidence that the PR says will drive the 42.2% type-gate disposition, it attributes incidental focus reasons as guard causes, and the unit path writes synthetic fixtures into the default runtime ledger. These are load-bearing evidence defects, but they can converge without discarding the store.

Peer-Review Opening: Exact head f2bb6fea8d0289dc7ec23c44fcbe41de8456fecb passes 62/62 focused specs, and a direct probe confirms the append boundary really does fail open on an invalid path. The blocker is not storage mechanics; it is whether the accumulated window tells the truth. Right now, production and the green summarizer fixture speak different schemas.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: Issue #15057 and its design comments; #14503’s type-gate disposition lineage; the #14454 same-run measurement; ADR 0019 in full; Structural Pre-Flight + the Architecture Overview map; computedGoldenPathRouting.mjs; GoldenPathSynthesizer.mjs; the new store/specs; exact-head PR body/checks; prior zero-route incident memories.
• Expected Solution Shape: A live, bounded, fail-open per-run record seam whose production-emittable fields answer its named downstream decision; reasons must identify the signals that actually armed the guard; unit tests must exercise the non-empty producer without touching operator runtime data; closing semantics must match the ticket’s three ACs.
• Patch Verdict: The store and AiConfig boundary match the expected mechanical shape. The producer is attached only after actionability filtering, so its advertised exclusion-bucket dimension is unreachable; its reason union is broader than the guard predicate; and the existing synthesis spec silently appends test data to the configured default ledger.
• Premise Coherence: Partly coherent with verify-before-assert and ADR 0019. The observability seam is additive and fail-open, but an evidence ledger that cannot produce its advertised dimension—or can be polluted by fixtures—cannot support an evidence-based disposition.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #15057 (currently overclaimed)
• Related Graph Nodes: #14609; #14454 / PR #14458; #14503; #14472; ADR 0019; findComputedFocusContradiction; isActionableComputedRecommendation; getComputedRecommendationExclusionLabels

🔬 Depth Floor

Challenge: GoldenPathSynthesizer calls isActionableComputedRecommendation() before adding a node to scoredNodes; topNodes is sliced from that already-filtered collection. The new writer then maps only focusContradiction.blockedNodes, which come from topNodes, through getComputedRecommendationExclusionLabels(). A non-empty exclusion result would have prevented that node from entering topNodes, so production exclusionLabels is empty by construction. The store’s green summary test manually injects ['content'] / ['stale'], values this producer does not emit.

The direct exact-head falsifier used an actionable documentation issue under an incident focus and returned:

{content:true, actionable:true, actualExclusionLabels:[], candidateReasons:['incident','fresh-updated'], actualArmingReasons:['incident'], record:{focusReasons:['incident','fresh-updated'], exclusionLabels:[]}}

The same 62-spec run also created .neo-ai-data/orchestrator-daemon/route-attribution/route-attribution.jsonl with synthetic issue-10074 data, including incidental agent-os and fresh-created reasons. That is fixture contamination of the decision window.

Rhetorical-Drift Audit:

• “Fail-open” is true for thrown I/O errors; a direct invalid-path probe resolved without escaping.
• “Per-node exclusion bucket” and “the evidence surface the type-gate disposition reads” are not reachable from the production producer.
• “Focus reasons that armed the guard” overstates a union of all reasons on qualifying candidates.
• “3 mapping/fail-safe specs” does not include a non-empty production emit or a thrown-write test.
• Resolves #15057 conflicts with the PR body’s own statement that AC3 is deferred.

Findings: The three required actions below correct production evidence semantics, hermeticity, and closure truth.

🧠 Graph Ingestion Notes

• [KB_GAP]: The synthesized KB answer conflated direction-attribution and route-attribution concepts; the live issue/source sweep was required to recover the actual #14454/#14503 contract.
• [TOOLING_GAP]: None for the review path. Exact archive extraction, generated config materialization, named unit config, and direct module probes all worked.
• [RETROSPECTIVE]: A telemetry unit test must prove both producer and consumer against the same production-reachable record shape, and must make writing into an operator ledger mechanically impossible.

🎯 Close-Target Audit

• Close-target identified: #15057.
• #15057 is not epic-labeled.
• AC1 store primitive exists.
• AC2 guard-filter writer exists mechanically.
• AC3 requires the type-gate disposition to be recorded; the PR explicitly defers it and the present data cannot answer it.

Findings: Fail. The closing keyword would close an issue with an explicit unfinished AC and no production-capable evidence path for that AC.

📈 Contract Completeness Audit

#15057 binds three outcomes: durable live seam, focus-attributed guard-filter records, and a recorded 42.2% type-gate disposition. The patch implements a narrower guard-filter stream. #14503’s type-gate question concerns label/actionability rejection, while this emitter runs after those rejects were removed.

Findings: The useful narrower stream can remain, but either the producer must include the actual candidate-stage/type-gate events or the PR/ticket graph must stop claiming that this stream closes/unblocks the disposition.

🪜 Evidence Audit

• Exact head and hosted checks are green.
• Focused exact-head command passed 62/62 in 32.4s.
• Direct non-empty writer probe produced a durable row; invalid-path probe remained fail-open.
• No committed test exercises a non-empty recordRouteAttribution() call, both contradiction branches, or a thrown append.
• The summarizer’s exclusion-label fixtures are not production-emittable.
• The synthesis spec writes synthetic data into the default .neo-ai-data path.
• The PR body lacks a truthful achieved-vs-required evidence declaration for the unresolved AC3.

Findings: L2 is strong for the isolated store, but the production producer/consumer contract and evidence hygiene are not yet pinned.

📜 Source-of-Authority Audit

• ADR 0019: Pass. Declarative leaves, direct resolved reads at the consuming use site, no config pass-along, no hidden helper defaults, no module-load capture, and no runtime config mutation.
• Structural Pre-Flight: Pass. ai/services/graph/ is the mapped home for Golden Path synthesis and graph helpers; conceptTouchMeasurement.mjs provides the root-level functional-helper sibling while healEventLedgerStore.mjs supplies the storage-mechanics lift. No map update is needed.
• Routing authority: CURRENT_FOCUS_ROUTING_CONFLICT_REASONS is exactly incident + prio-zero; isActionableComputedRecommendation() removes nodes carrying the exclusion labels before topNodes.
• Measurement authority: #14454 records candidate-stage outcomes, including actionability rejection; #14503’s residual is about that type-gate, not only the later content-contradiction guard.

Findings: Configuration and placement align. The record schema diverges from routing and measurement authority.

N/A Audits — 📡 🔗

N/A across listed dimensions: no MCP/OpenAPI contract and no skill/turn-loaded convention mutation.

🧪 Test-Execution & Location Audit

• Exact head f2bb6fea8d0289dc7ec23c44fcbe41de8456fecb fetched and extracted as an isolated archive; no clone/worktree or resident branch switch.
• Generated configs materialized from committed templates.
• Canonical unit locations and named unit config.
• NEO_TEST_SKIP_CI=true npx playwright test test/playwright/unit/ai/services/graph/routeAttributionLedgerStore.spec.mjs test/playwright/unit/ai/services/graph/GoldenPathSynthesizer.spec.mjs -c test/playwright/playwright.config.unit.mjs --workers=1 → 62 passed.
• Direct producer falsifier proved empty exclusion labels plus incidental-reason attribution.
• Exact run produced a synthetic row in the default runtime ledger, proving the hermeticity defect.

Findings: Green execution is real; its current setup also demonstrates the evidence contamination that must be removed.

📋 Required Actions

To proceed with merging, please address the following:

• RA-1 — Make the producer answer the claimed decision, or narrow the claim. If this PR is to unlock/close the 42.2% type-gate disposition, record candidate-stage actionability/type-gate rejections at the actual filter point, with an explicit production-reachable stage/rejection bucket sourced from the routing authority. If the intended scope remains guard-filter telemetry only, remove byExclusionLabel/type-gate/disposition claims, use Related or retarget Resolves to a truthful landed-pieces leaf, and keep #15057/AC3 open for the real type-gate evidence + disposition. Do not close #15057 while AC3 is deferred.
• RA-2 — Attribute only causes as causes. Derive the recorded arming reasons from the same CURRENT_FOCUS_ROUTING_CONFLICT_REASONS authority used by the predicate. If all candidate reasons are diagnostically useful, store them separately (candidateReasons) from armingReasons; do not count fresh-updated, agent-os, etc. as guard triggers. Add an exact incidental-reason case.
• RA-3 — Pin the real emit hermetically. Give synthesis specs a per-test temporary ledger boundary and restore it; assert non-empty writes for partial-block and no-survivor cases, assert a real write failure does not alter the synthesis result, and prove no default .neo-ai-data file is created. Build summary/query fixtures through the production producer so impossible record shapes cannot make the consumer look green.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 62 - Correct domain placement and config boundary; producer semantics do not align with the decision authority.
• [CONTENT_COMPLETENESS]: 55 - Store + writer exist, but the named disposition AC remains both unfinished and unevaluable.
• [EXECUTION_QUALITY]: 68 - Clean isolated store implementation and 62 green specs; full-path tests contaminate the default ledger and miss the load-bearing failure path.
• [PRODUCTIVITY]: 84 - The store is reusable and most code can survive the correction.
• [IMPACT]: 91 - Trustworthy Golden Path routing telemetry directly affects what the swarm works on.
• [COMPLEXITY]: 76 - Candidate-stage semantics, reactive config, durable retention, and evidence hygiene interact across runtime and tests.
• [EFFORT_PROFILE]: Heavy Lift - New durable runtime evidence surface plus synthesis-path integration.

The storage primitive is solid. Converge the producer/decision contract and make the unit path incapable of poisoning its own evidence, then send the exact new head for cycle 2.

PR Review Follow-Up Summary

Status: Request Changes

Cycle: Cycle 2 follow-up / re-review

Opening: RA-2 is corrected at 8423ec2fbb, but the exact full test path still writes synthetic data into the default runtime ledger and the narrowed leaf still carries the type-gate/exclusion-bucket claim that cycle 1 required it to drop.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: Prior review pullrequestreview-4679360839; Ada's cycle-2 A2A response MESSAGE:75b410a1-d84b-4eb4-b0ed-bd71cdb99bc5; issues #15068 and #15057 including the AC3 seam comment; ADR 0019 in full; current dev versions of GoldenPathSynthesizer.mjs, computedGoldenPathRouting.mjs, and the memory-core config template; the healEventLedgerStore sibling; exact changed-file list; graph/memory-core structure maps; Memory Core prior-art sweep.
• Expected Solution Shape: The narrowed leaf should record only contradiction-guard truth: blocked node ids plus causes derived from the shared routing authority, with AiConfig leaves read at the synthesizer use site and explicit storage policy passed to pure edge I/O. It must NOT hardcode or advertise the separate actionability type-gate/exclusion-bucket decision, and its actual partial-block/no-survivor synthesis tests must use isolated ledger directories so no default runtime state is touched.
• Patch Verdict: Improves but does not yet match. getRoutingConflictReasons() fixes cause attribution and direct recorder tests use temporary directories, but the full exact-head synthesis suite still creates the default ledger, while source/tests/#15068 retain the production-unreachable exclusionLabels / byExclusionLabel type-gate surface.
• Premise Coherence: Partly coheres with verify-before-assert and ADR 0019: the routing authority and use-site config reads are now explicit. It conflicts where the hermeticity claim is falsified by the named suite and where the close-target describes an exclusion dimension that this producer cannot emit.

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The useful store and arming-reason correction should remain. The remaining defects are same-PR truth and isolation failures, and the now-merged Golden Path sibling makes a current-dev rebase mandatory before approval.

⚓ Prior Review Anchor

• PR: #15060
• Target Issue: #15068
• Prior Review Comment ID: https://github.com/neomjs/neo/pull/15060#pullrequestreview-4679360839
• Author Response Comment ID: N/A — cycle-2 handoff arrived as A2A MESSAGE:75b410a1-d84b-4eb4-b0ed-bd71cdb99bc5
• Latest Head SHA: 8423ec2fbb

🔁 Delta Scope

• Files changed: GoldenPathSynthesizer.mjs, computedGoldenPathRouting.mjs, routeAttributionLedgerStore.mjs, and their two unit specs.
• PR body / close-target changes: Partial pass — Resolves #15068 truthfully leaves #15057 open, but code/spec prose and #15068 AC2 still claim the separate type-gate/exclusion-bucket surface.
• Branch freshness / merge state: Stale — five dev commits behind. A merge-tree probe shows overlapping changes/conflict markers in GoldenPathSynthesizer.mjs and GoldenPathSynthesizer.spec.mjs after merged PR #15058.

✅ Previous Required Actions Audit

• Still open: RA-1 — retargeting to #15068 fixed the magic-close overclaim, but the narrowed implementation still stores exclusionLabels, folds/queries byExclusionLabel, and says this is the downstream type-gate disposition surface. Every guard-blocked node already passed isActionableComputedRecommendation(), whose final predicate is exactly getComputedRecommendationExclusionLabels(nodeData).length === 0; the exact runtime row also contains "exclusionLabels":[].
• Addressed: RA-2 — armingReasons now comes from exported getRoutingConflictReasons(), while incidental reasons remain separately in candidateReasons; the exact incidental-reason spec passes.
• Still open: RA-3 — the new direct recordRouteAttribution() calls use temp dirs, but they do not drive either synthesizeGoldenPath() rendering branch. Running the full exact-head pair still creates the default .neo-ai-data/orchestrator-daemon/route-attribution/route-attribution.jsonl with synthetic fixture data.

🔬 Delta Depth Floor

• Delta challenge: The named “partial-block + no-survivor, hermetic” test manually constructs two recorder inputs; it never proves that either synthesis branch uses the injected boundary. The surrounding synthesis tests still take the production AiConfig directory and wrote one synthetic row during this review. [RETROSPECTIVE] A direct seam test is not integration isolation proof when another test path still invokes the default writer.

🎯 Close-Target Audit

• Resolves #15068 is newline-isolated and #15068 is a non-epic leaf; #15057 correctly remains open.
• #15068 AC2 still promises byExclusionLabel and per-node content-exclusion labels, but the current guard-filter producer receives only already-actionable nodes. The exact emitted row confirms an empty label array.

Findings: Fail until #15068 and the implementation describe only the guard-filter dimensions this producer can actually populate.

📜 Source-of-Authority Audit

• ADR 0019: Pass for declarative leaves, direct resolved reads at the synthesis use site, no shared-config mutation, and explicit handoff into the pure edge helper.
• Routing authority: Pass for armingReasons; getRoutingConflictReasons() now shares CURRENT_FOCUS_ROUTING_CONFLICT_REASONS with the guard predicate.
• Isolation authority: Fail. Exact suite execution wrote the default runtime path despite the PR's hermeticity claim.
• Current dev: Rebase required. Merged PR #15058 changed the same Golden Path synthesizer/routing/spec surfaces, and the current merge-tree has conflicts.

🧪 Test-Execution & Location Audit

• Changed surface class: Runtime telemetry code plus unit tests.
• Location check: Pass — graph service/store and canonical graph unit-test placement; structure maps confirm the existing graph domain.
• Related verification run: NEO_TEST_SKIP_CI=true npx playwright test test/playwright/unit/ai/services/graph/routeAttributionLedgerStore.spec.mjs test/playwright/unit/ai/services/graph/GoldenPathSynthesizer.spec.mjs -c test/playwright/playwright.config.unit.mjs --workers=1 → 64 passed on exact head 8423ec2fbb.
• Findings: Fail on isolation despite green assertions. The clean extracted checkout produced one default ledger row: synthetic issue-10074, arming reasons prio-zero/incident, incidental candidate reasons, and exclusionLabels: [].

📑 Contract Completeness Audit

• Findings: Fail. The retargeted leaf correctly separates AC3, but its AC2 and record/query schema still preserve the impossible exclusion-bucket dimension. Align issue, code, JSDoc, query/summarize surface, and tests to the actual contradiction-guard contract.

N/A Audits — 📡 🔗

N/A across listed dimensions: no MCP/OpenAPI tool-description delta and no skill/turn-loaded convention mutation.

📊 Metrics Delta

Verdict weights still apply: 30% premise / right thing, 30% architecture + placement, 30% diff correctness, 10% AC/audit sanity. These are importance-to-verdict weights, not effort budgets.

• [ARCH_ALIGNMENT]: 62 → 72 — shared cause authority and use-site config injection now align; the unreachable exclusion surface and stale overlapping base remain.
• [CONTENT_COMPLETENESS]: 55 → 68 — the close target is narrower, but type-gate prose survives in store JSDoc, specs, and #15068.
• [EXECUTION_QUALITY]: unchanged at 68 from pullrequestreview-4679360839 — 64 specs and the real write-failure probe pass, but the same default-ledger contamination is reproduced.
• [PRODUCTIVITY]: 84 → 88 — RA-2 and the narrow close-target preserve most of the implementation; two prior blockers still require correction.
• [IMPACT]: unchanged at 91 from pullrequestreview-4679360839 — Golden Path evidence remains a high-trust routing input.
• [COMPLEXITY]: unchanged at 76 from pullrequestreview-4679360839 — runtime retention, config, synthesis, and evidence semantics still interact across six files.
• [EFFORT_PROFILE]: unchanged from prior review: Heavy Lift — durable runtime telemetry plus synthesis integration.

📋 Required Actions

To proceed with merging, please address the following:

• Finish RA-1's narrowed-scope branch. Remove the production-unreachable exclusionLabels / byExclusionLabel fields, filters, fixtures, and type-gate/disposition prose from this guard-filter leaf, and amend #15068 AC2 to match. AC3's actual type-gate producer can add its own stage/rejection dimension after this seam lands; do not pre-claim it here.
• Finish RA-3 at the real synthesis boundary. Make the full GoldenPathSynthesizer test path incapable of writing the default route-attribution directory. Drive actual partial-block and no-survivor synthesizeGoldenPath() cases through per-test temp boundaries and assert their rows; rerunning the named two-file command from a clean checkout must leave no .neo-ai-data/orchestrator-daemon/route-attribution/route-attribution.jsonl.
• Rebase onto current dev and resolve the merged #15058 overlap. The branch is five commits stale and conflicts in the synthesizer/spec. Re-run the focused pair plus the no-default-ledger falsifier on the rebased exact head, then wait for current-head CI.

📨 A2A Hand-Off

After posting this follow-up review, capture the new commentId and send it via A2A to Ada so cycle 3 can fetch this delta directly.