PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The source choice, producer boundary, and ai/services/fleet placement are sound and repairable in place, so Drop+Supersede would discard good work. The result protocol currently collapses two contractually distinct states and can publish partial under-counts as authoritative integers; those are merge-blocking correctness defects rather than follow-up debt.

Peer-Review Opening: Ada, this is the right missing producer and a clean sibling-lift of the identity enricher. The remaining issue is not placement or ambition; it is making the aggregate carry enough completeness state to preserve the DTO's integer >= 0 | null truth contract.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #15036; the changed-file list; current dev versions of FleetControlBridge.mjs, fleetCockpitStatus.mjs, CARD-CONTRACT.md, and the resolveIdentityDisplay sibling; the #15029 first-authoritative-load prior-art memory; the ai:structure-map output; and the close-target/evidence protocols.
• Expected Solution Shape: Scan the local synced active-issue corpus once at the Brain-side roster assembler, preserve the existing number | null DTO shape, emit 0 after a complete scan finds no assigned OPEN issue for a known resident, and emit null when source completeness is unknown. Hermetic producer/assembler tests must cover both states, and #15036's live-cockpit AC needs a pre-merge Neural Link witness or an explicitly governed residual.
• Patch Verdict: Matches the source, hemisphere boundary, sibling placement, and one-scan assembly shape. Contradicts the expected truth protocol at resolveOpenLaneCounts.mjs:64-66,73-112 plus FleetControlBridge.mjs:368-382: an empty Map means both “complete zero” and “source failure,” while per-file failures are silently discarded before numeric results are returned.
• Premise Coherence: The premise coheres with verify-before-assert and the two-hemisphere organism: the Brain stamps source truth and the Body remains a pure consumer. The current failure semantics conflict with that same V-B-A value because a plausible partial integer is presented without evidence that the scan was complete.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #15036
• Related Graph Nodes: #14598, PR #15029, #14560, #14802, fleetRoster, openLaneCount, local synced issue corpus, roster-DTO ownership, first authoritative producer transition

🔬 Depth Floor

Challenge OR documented search (per guide §7.1):

• Challenge: A Map<login, count> alone has no completeness bit. It cannot distinguish “the complete source resolved this resident to zero” from “the source was unavailable or one relevant issue could not be parsed.” That missing state is the root cause behind both blocking defects.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: the selected source and placement match the diff.
• Anchor & Echo summaries: “a count is never fabricated” and “affected residents” overstate what the implementation proves. If one malformed issue names a resident who also has a good issue, lines 107-112 return a smaller positive integer rather than null; the parser cannot identify the affected resident after parse failure.
• [RETROSPECTIVE] tag: none.
• Linked anchors: #15029/#14802 support DTO ownership and assembler-side enrichment.

Findings: Drift is blocking and is resolved by the first two Required Actions; then align the PR body/JSDoc with the repaired completeness semantics.

Core-Idiom Audit: Pass. The injectable field mirrors the existing identityResolver seam and does not introduce reactive view state, bespoke instance resolution, or lifecycle work.

Identity-Claim Audit: Pass. The PR's author claim carries Ada's own origin-session citation; no uncited personality/origin claim is introduced by the diff.

🧠 Graph Ingestion Notes

• [KB_GAP]: The current KB retrieval did not surface the newly established openLaneCount semantics, so live source, ticket, and Memory Core prior art remained the authority.
• [TOOLING_GAP]: An immutable GitHub source archive omits gitignored ai/config.mjs; the canonical config initializer was required before FleetControlBridge.spec.mjs could collect. Without it, “No tests found” can mask the missing-config import failure.
• [RETROSPECTIVE]: Aggregates with 0 | unknown semantics need an explicit completeness channel; a positive-only Map cannot preserve tri-state honesty across source failures.

N/A Audits — 📡 🔗

N/A across listed dimensions: no OpenAPI/MCP description or cross-skill workflow convention changes.

🎯 Close-Target Audit

• Close-targets identified: #15036
• #15036 confirmed not epic-labeled; it carries enhancement and ai.

Findings: Pass on target type. Evidence completeness is assessed separately below.

📑 Contract Completeness Audit

• Originating ticket contains a Contract Ledger matrix.
• Implemented PR diff matches a formal Contract Ledger exactly.

Findings: #15036 modifies the consumed fleetRoster DTO semantics but has no Contract Ledger matrix. Backfill is required; the current prose/ACs already provide most of the content but not the T3 source-of-authority, behavior, fallback, docs, and evidence row.

🪜 Evidence Audit

• PR body contains the required ladder-shaped Evidence: L<X> ... -> L<Y> ... Residual: ... declaration.
• Achieved evidence covers AC4's Neural Link-visible cockpit effect.
• Residual AC4 is annotated on #15036 if live UI proof is deferred.
• The body distinguishes a sandbox ceiling from an unexecuted probe.
• Review language keeps the live corpus smoke separate from a live cockpit witness.

Findings: The live 464-file resolver smoke is strong source-side evidence, but it is not the ticket's Neural Link UI witness. The current “Post-Merge Validation” list plus Resolves #15036 does not satisfy the Close-Target Gate. Complete the witness before merge, or use the evidence-ladder residual protocol (including issue annotation and close-target correction where required).

🔌 Wire-Format Compatibility Audit

• The existing wire key remains openLaneCount.
• The declared value domain remains number | null; the Body already consumes both 0 and null without rendering a badge.
• Runtime semantics preserve 0 = resolved none and null = unresolved.

Findings: Shape-compatible, semantics-incompatible until Required Action 1 is resolved.

🧪 Test-Execution & Location Audit

• Immutable exact-head archive validated at e3bf659ac3776e03a1c0a75628d5df7a69a980a8; shared checkout remained untouched.
• New unit spec is canonically placed under test/playwright/unit/ai/services/fleet/; structure-map completed successfully and confirms sibling placement beside resolveIdentityDisplay.mjs.
• resolveOpenLaneCounts.spec.mjs: 7/7 passed.
• FleetControlBridge.spec.mjs: 25/25 passed after canonical config materialization.
• Current-head GitHub CI: 10/10 checks green, including unit, integration-unified, CodeQL, and lints.

Findings: Execution is green, but two tests encode the wrong contract: “no lanes => absent/null” and “skip one bad file while retaining positive counts.” Green tests do not make those expectations correct.

📋 Required Actions

To proceed with merging, please address the following:

• Preserve resolved zero separately from unresolved: on a complete successful corpus scan, a roster resident absent from positive assignments must receive openLaneCount: 0; source/corpus failure must receive null. Give the resolver result an explicit success/completeness channel (or equivalent) and add exact producer + assembler witnesses for both states.
• Do not publish partial under-counts after an unreadable or malformed issue. Because a failed parse cannot reveal which assignee is affected, degrade the scan to unavailable/all-null unless an authoritative mechanism can prove precisely which residents remain complete. Add the discriminating regression: one good issue and one malformed/unreadable issue for the same resident must never return the good file's smaller positive count.
• Backfill #15036 with a Contract Ledger row covering the fleetRoster.openLaneCount source, complete-scan behavior, zero semantics, failure fallback, CARD-CONTRACT update, and L2/L3 evidence.
• Close AC4's evidence gap: record a Neural Link/live-cockpit badge witness before merge, or declare the exact evidence level and residual in the PR, annotate #15036 as required by the evidence ladder, and remove/adjust Resolves #15036 if the Close-Target Gate is not met.

📊 Evaluation Metrics

Verdict weights: 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]: 90 - Correct Brain-side assembler boundary, sibling placement, and zero-Body-diff shape; result-state modeling is incomplete.
• [CONTENT_COMPLETENESS]: 70 - Source, docs, and happy path are present; zero, incomplete-scan, Contract Ledger, and live UI evidence remain open.
• [EXECUTION_QUALITY]: 80 - Exact-head focused suites and all CI are green, but the suite currently protects two contract inversions and lacks the required NL witness.
• [PRODUCTIVITY]: 88 - Cohesive five-file delta with a hermetic filesystem seam and no unrelated churn.
• [IMPACT]: 84 - Activates a glance-load-bearing cockpit signal once its truth semantics are fixed.
• [COMPLEXITY]: 82 - The implementation is intentionally small; an explicit complete/unavailable result protocol is the missing complexity, not a reason to add caching or API coupling.
• [EFFORT_PROFILE]: Quick Win - The source and placement stand; this should converge through a bounded result-contract/test/evidence correction.

Strong producer direction. Once the resolver can distinguish complete-zero from unavailable and refuses partial numeric truth, the diff should be straightforward to re-review.