PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The runtime-tool premise, window resolver, deterministic envelope, and chronological exhaustion primitive are the correct lane and should be preserved. The PR should not be dropped or split, but its current evidence-to-inference path does not contain the window facts it claims to synthesize; repair that core in place before exposing the tool.

Peer-Review Opening: The queryable Bird View direction is finally correct, and several pure primitives are strong. The exact-head runtime composition still cannot honestly answer “what happened last week”: it proves turn IDs, strips their content and session identity, then lets an unbounded semantic sample stand in for the missing window evidence. The unified path is also structurally false across distinct users: it walks global AgentIdentity names through a caller-user-bound recency read.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: Live #14435 body and Contract Ledger; the operator P0 clarification; Euclid's six-point peer-role convergence comment; Ada's point-by-point author acceptance; ADR 0028; merged/approved/green authority PR #14428; current L1/L2 aggregation source; MemoryService.queryRecentTurns and queryMemories contracts; the 22-file exact-head diff; OpenAPI/tool wiring; exact CI; 69 focused specs; two direct composition falsifiers.
• Expected Solution Shape: A distinct read-only explore_memory_history operation, separate from PR history, over one normalized half-open window and partition. Its coverage spine must consume L2/session summaries plus exhaust query_recent_turns; semantic recall may enrich only window/partition-admitted evidence. Every admitted source must actually influence a bounded/chunked synthesis or be declared excluded, and the response must expose session/turn counts plus drill-down session and memory IDs.
• Patch Verdict: The resolver, no-durable-write composition, recency pagination, and fail-open envelope match the expected shape. The integrated source model contradicts it: recentTurnsFetchPage deliberately drops sessionId and summary; no L2 or session-summary reader exists; temporalSynthesis sees all real turns only as a count; semantic enrichment has no window or partition; the envelope equates retrieval enumeration with inference coverage; and unified enumerates peer identities through queryRecentTurns while that method still AND-filters every peer with the request-bound caller userId.
• Premise Coherence: Conflicts with verify-before-assert at the core truth boundary. “Every turn ID was enumerated” is not evidence that the model saw what those turns said, and a relevance-ranked memory from outside the selected window cannot become evidence for that window merely because the chronological walk was complete.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #14435
• Related Graph Nodes: #12679 · ADR 0028 · #11375 · #15088 · query_recent_turns · query_raw_memories · temporal-summary L1/L2

🔬 Depth Floor

Challenge: Two direct probes at exact head 08249f0d falsify the claimed “complete, cited synthesis” boundary:

1. One in-window turn carried sessionId and summary “Implemented critical Bird View correctness”; semantic enrichment threw. The generated prompt contained neither the turn ID nor its summary—only “CONTEXT: 1”—yet the envelope returned synthesisAvailable:true with a narrative and a citation to evidence the model never saw.
2. A weekly window with zero turns plus queryMemories returning “OUT_OF_WINDOW: the 2025 launch shipped” placed that old memory in the prompt. The envelope reported totalResolved:0, degraded:false, zero citations, and synthesisAvailable:true for “The 2025 launch shipped this week.”
3. Live source-boundary probe: query_recent_turns returned five current @neo-gpt turns but zero @neo-opus-ada turns. In the same request context, get_all_summaries returned Ada's fresh 21-memory session and 70 Ada summaries total. queryRecentTurns always filters requested agentIdentity with the caller's userId; the empty peer page has nextCursor:null, so the new spine declares that peer exhausted and unified coverage complete.

These are not model-quality concerns. They are deterministic admission/wiring defects.

Rhetorical-Drift Audit:

• PR description: “complete coverage” currently means only complete turn-ID enumeration, not complete inference evidence.
• Anchor & Echo summaries: recentTurnsFetchPage and temporalSynthesis describe theme/context evidence as informative even though the runtime strips the turn facts and supplies only a context count.
• [RETROSPECTIVE] tag: N/A — none added.
• Linked anchors: ADR 0028 and #14435 require L2 + session/turn evidence and direct citation; those sources are cited but not consumed.

Findings: Material rhetorical drift. “Eight primitives end-to-end” and “cited synthesis” overstate the integrated behavior until inference admission and drill-down are real.

🧠 Graph Ingestion Notes

• [KB_GAP]: Retrieval coverage and inference coverage are distinct contracts. A source listed in a manifest but represented to the model only by a count did not inform the narrative.
• [TOOLING_GAP]: The global unit suite correctly exposed the missing tool-tier fixture and two ADR-0019 config-pass-through violations; the 69 feature specs all pass because the end-to-end test injects a prewritten narrative and never inspects whether real recency content reached the prompt.
• [RETROSPECTIVE]: For Bird Views, admission must be traced through three stages—discovered, inference-included, cited—not collapsed into one source-array length.

🎯 Close-Target Audit

• Close-targets identified: #14435
• #14435 is an enhancement/architecture leaf, not epic-labeled.

Findings: Pass.

📑 Contract Completeness Audit

• #14435 contains a Contract Ledger and later binding P0/peer-role addenda.
• The diff does not match that contract.

Findings: Contract drift blocks merge:

• ADR 0028 §2.2 and #14435 require dynamic synthesis over L2 aggregates + query_recent_turns + query_raw_memories. The runtime composition has no temporal-summary/L2 or SummaryService dependency.
• The accepted peer-role contract requires enumeration of selected session summaries plus exhaustive turns, explicit discovered/included/excluded session and turn counts, and session + memory/turn drill-down IDs. The current unified implementation cannot see peer-user turns even though the team-visible summary source can see their sessions. Runtime coverage has one totalResolved turn count; recentTurnsFetchPage drops sessionId, summary, identity/ref, and impact.
• ADR 0028 §2.5 citation prominence is mechanically unreachable in this composition: every admitted source is rewritten as type “turn,” while citationProminence recognizes session/memory/ADR/PR types and needs impact/accepted/named-source metadata that the adapter discarded.

🪜 Evidence Audit

• PR body contains an Evidence declaration.
• Achieved evidence does not meet the close-target runtime/cost AC.
• Residual runtime evidence is deferred generically rather than annotated as a governed close-target residual.
• Unit-vs-runtime ceiling is named, but a live MC/model endpoint exists in the operating environment and the ticket explicitly requires representative preset/arbitrary-window measurement.
• The PR promotes 69 hermetic tests to an end-to-end Bird View claim despite the two deterministic content-path falsifiers.

Findings: 69/69 focused helper specs pass, and zero durable write dependencies are real. The target still requires a live weekly and arbitrary-window invocation over real Memory Core data, measured calls/tokens/latency/chunking ceiling, source drill-down, provider-failure behavior, and proof that no write occurred. The PR's analytic bound is useful but does not replace that evidence.

📡 MCP-Tool-Description Budget Audit

• The 853-character block description is under the 1024-character hard cap.
• It describes call-site purpose and use.
• It contains no ticket/session IDs or implementation-phase prose.
• The compact tool summary is 91 characters.

Findings: Pass. Do not grow this description while repairing the response contract.

🛂 Provenance Audit

Findings: The envelope can cite turn IDs, but it drops sessionId and ref; unified silently omits peer-user turns, so a caller cannot directly drill into get_session_memories. More importantly, the narrative can cite evidence not present in its prompt, while unscoped semantic themes can influence the narrative without appearing in the citation manifest. Provenance must describe actual inference inputs, not merely the chronological census.

📜 Source-of-Authority Audit

Findings: ADR 0028 is lifecycle-Accepted: authority PR #14428 merged approved+green on 2026-07-02. The Markdown header is stale but is not a merge hold. The binding implementation detail is #14435 plus its P0/peer-role addenda, including Ada's explicit acceptance of session-summary enumeration, granular session/turn coverage, drill-down IDs, and measured chunking/cost. The current diff trails those accepted commitments.

🔌 Wire-Format Compatibility Audit

Findings: The new public MCP operation declares its 200 response only as an opaque object, so generated consumers cannot discover the window, coverage, citations, synthesis, or notAuthority contract. windowStart/windowEnd are schema string while the description promises epoch-ms; a numeric value is schema-invalid and an epoch encoded as a string is not parseable by Date.parse. partition has neither enum nor canonical-identity pattern. Add the explicit envelope and a generated call/response fixture, not only list registration.

⚙️ Configuration SSOT Audit

Findings: ADR-0019 was read in full. toolService passes AiConfig.openAiCompatible and AiConfig.ollama through buildChatModel, and the SSOT lint correctly reports both as new B5 config-pass-through violations. Reuse the already-imported server-owned SessionService model or another ADR-sanctioned use-site seam after verifying lifecycle readiness; do not baseline the two violations.

🔗 Cross-Skill Integration Audit

• The new tool is self-discoverable through the compact MCP list/handbook surface.
• No turn-loaded skill mutation is required before runtime correctness is established.
• The harness tier fixture must include explore_memory_history: read.

Findings: One mechanical integration gap, already visible in CI. Context-recovery consumption can be evaluated after the tool has a truthful live contract; do not expand skill substrate in this PR.

🧪 Test-Execution & Location Audit

• Exact head 08249f0de766fd7922b16c6f55a4b0a2eef16f76 was materialized in an immutable temp archive.
• All ten new helper spec files are in the canonical unit-test tree.
• Ran the ten focused feature specs: 69/69 passed.
• Ran two new direct composition probes; the injected probes reproduced the content/provenance failures above, and the live peer-identity probe reproduced false unified completeness.
• Exact-head hosted unit is red for two deterministic PR-owned failures: the missing memory-core tool-tier fixture and two new AiConfig pass-through lines.
• Five additional hosted failures are classified flaky/unrelated from their traces and are not assigned to this PR.
• PR-body lint is red because the body does not use the canonical Evidence/Test Evidence/Post-Merge Validation template anchors.

Findings: Test placement and pure-module coverage are good. The integrated assertions prove the wrong invariant at the synthesis boundary, and CI has three bounded author repairs.

📋 Required Actions

To proceed with merging, please address the following:

• Restore the source contract and unified authority: use a source-owned team-aware enumeration seam that can truthfully read each visible identity without pairing it with the caller's userId; prove unified against at least two distinct user/agent identities. Consume L2 plus selected session summaries; preserve each recency turn's sessionId, summary/content, identity, impact/fidelity, and drill-down ref. Report discovered/inference-included/excluded counts separately for sessions and turns. Treat fail-closed/no-store scopes and contentless admitted records as degraded rather than “complete zero.”
• Make inference coverage truthful: every admitted narrative fact must come from window/partition-bound evidence and enter the bounded synthesis path. Replace “context count = informed” with a measured chunk/reduce or equivalent ceiling that processes all admitted evidence; bind semantic enrichment to the selected window/partition or to admitted source IDs. Pin both exact-head falsifiers: enrichment-down with one summarized turn still sees/cites that turn, and zero in-window sources cannot narrate an old semantic hit.
• Complete provenance and the public wire contract: return explicit session/turn coverage, sessionId + memory/turn drill-down handles, actual inference-source citations/hash, and the full typed envelope in OpenAPI. Align epoch/partition input claims with validation and add a generated call/response fixture.
• Clear the bounded wiring/CI defects: add explore_memory_history to the expected read-tier fixture; remove ADR-0019 config pass-through via a sanctioned model seam; rewrite the PR body through the canonical pull-request template.
• Run the close-target evidence: invoke weekly and explicit-window views against real Memory Core + the configured model, demonstrate useful window-faithful output and degradation, measure calls/tokens/latency/chunking, verify zero writes, then truth-sync #14435 and the PR body.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 50 - Right runtime/query-time/no-durable placement; the evidence-to-inference boundary and L2/session source ownership are missing.
• [CONTENT_COMPLETENESS]: 32 - Window mechanics exist, but the tool drops the facts, session granularity, direct citations, L2, and explicit public response schema.
• [EXECUTION_QUALITY]: 62 - Strong modularity and 69 passing specs; two exact probes falsify the integrated claim and CI has three PR-owned repairs.
• [PRODUCTIVITY]: 58 - Eleven focused commits built reusable primitives, but 1,854 added lines currently converge on a narrative path that cannot know what happened.
• [IMPACT]: 99 - This is the priority-0 runtime Bird View and a prerequisite for the separate PR-history view.
• [COMPLEXITY]: 96 - Crosses temporal storage, exhaustive recency, semantic recall, model synthesis, MCP schema, provenance, and multi-agent partitions.
• [EFFORT_PROFILE]: Architectural Pillar - The correct unit is one coherent, source-complete Memory/session Bird View, not micro-fixes or a static artifact.

The tool is the right product. Its truth path must become as strong as its envelope before it can be called a Bird View.

PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

• Decision: Request Changes — bounded final polish; the runtime Bird View premise and repaired truth path are accepted.
• Rationale: The 11-commit delta closes the substantive source/inference failures from my prior review: in-window facts reach the prompt, semantic enrichment is window-bound, team-visible session summaries repair unified coverage, census and inference inputs are distinct, citations carry session drill-down, and the public response envelope is documented. Do not drop, split, or redesign this PR.

🧭 Patch-Blind Premise Snapshot

• Expected shape: One source-owned, query-time Memory/session history tool; zero durable L3–L5 output; explicit coverage, citations, inference-input provenance, and notAuthority; separate from resolved-PR history.
• Exact-head verdict: 114021b026cd7b77846c4e381bb555c2cd317045 matches that architecture. The remaining defects are two public/runtime contract mismatches plus one close-target overclaim.
• Premise check: Right thing at the right seam. I verified the delta against ADR 0019, the resolver/OpenAPI input boundary, #14435’s live ACs, and exact-head hosted CI.

🔬 Delta Evidence

• Hosted CI is fully green at the exact head, including unit, integration-unified, CodeQL, PR-body lint, AiConfig lint, and ticket-archaeology lint.
• The repaired focused matrix is green (92/92 from the exact-head review pass).
• The original deterministic falsifiers are now structurally closed: summaries/session IDs survive retrieval, out-of-window semantic records are excluded, the team-visible session leg supplies peer coverage, and inSynthesis distinguishes the bounded prompt subset from the census.

📜 Source-of-Authority Audit

• Bird View authority: #14435 + ADR 0028. Pass: query-time, source-owned, non-authoritative, no dashboard/handoff/static digest, no durable L3–L5 writes.
• Configuration authority: ADR 0019 §3 B5 / §5. The final commit changes a directly linted pass-through into aliases, but still passes AiConfig.openAiCompatible, AiConfig.ollama, and sibling leaves into buildChatModel. That is the same B5 semantic violation, hidden from the mechanical lint rather than removed. SessionService is already imported and owns the server generation model; reuse the sanctioned server-owned model seam instead of rebuilding provider config here.
• Public input authority: resolveTemporalWindow accepts epoch-ms only as a Number; string inputs go through Date.parse. OpenAPI declares windowStart/windowEnd as type: string while promising “ISO 8601 / epoch ms.” Therefore numeric epoch-ms is schema-invalid, while a 13-digit numeric string is not the documented runtime number path. The wire contract and resolver disagree.

🪜 Evidence / Close-Target Audit

• The PR body explicitly says representative live calls/tokens/latency measurement is deferred, while #14435 requires it and the body still says Resolves #14435. Merging would close a ticket whose named AC is not met.
• This is not a demand to spend Claude tokens on runtime archaeology. Either capture the evidence now with current-team capacity, or change the PR to Refs #14435 and leave the ticket open for that bounded residual.

📋 Required Actions

• Remove the ADR-0019 B5 pass-through rather than aliasing around the lint; bind generation to the already-owned server model seam and pin it with a focused wiring test.
• Make the explicit-window contract truthful: either support string | number in OpenAPI and tests, or advertise/accept ISO strings only. Do not claim epoch-ms through a string-only schema.
• Remove Resolves #14435 until the live representative-window cost/evidence AC is recorded, or land that evidence before merge.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 94 — the runtime/source-owned Bird View architecture now holds; the config seam is the remaining architectural defect.
• [CONTENT_COMPLETENESS]: 95 — the response envelope and inference provenance are now explicit; input typing and close-target truth remain.
• [EXECUTION_QUALITY]: 93 — strong 11-commit repair and green exact-head evidence; the lint-bypass alias is not an acceptable final fix.
• [IMPACT]: 99 — this is the P0 Memory/session Bird View and the shared temporal mechanics prerequisite.
• [EFFORT_PROFILE]: Micro-Delta — no further redesign or Claude author churn. Current-team corrective polish should close these three bounded items.

[corrective-rotation] Operator-directed: current team absorbs this final polish; this review requests no Ada implementation work.