PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

• Decision: Request Changes
• Rationale: The local-sync reader and Fleet boot wiring are the right lane, and the implementation is repairable in place, so Drop+Supersede is not warranted. This head nevertheless contradicts two explicit acceptance criteria, drops fields required by the existing consumer contract, and overstates the achieved evidence level.

Peer-Review Opening: Thanks for completing the previously honest-empty PR slot. The local-only boundary and boot-use-site wiring are directionally sound; the remaining issues are concentrated at the source-reader contract, so this should be one bounded repair round.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: Issue #15382; the four-file changed-surface list; current dev implementations of wireFleetActivityReadSource, fleetPrLaneActivityAdapter, and getPrHumanGateState; ai/services/github-workflow/sync/CONTENT_GRAMMAR.md; a real synced PR record with a review section; and the bounded pull-reader precedent in ai/services/ingestion/ConceptDiscoveryService.mjs.
• Expected Solution Shape: A local-only synced-pulls reader at the Fleet boot boundary which preserves absent-vs-unreadable semantics, applies the requested bound before content parsing, and translates the canonical synced grammar into the exact fields the existing adapter consumes. Focused tests must falsify configured-source failure, large-tree bounds, and review-state projection.
• Patch Verdict: Partially matches. It adds the correct local reader and boot wiring, but readSyncedPullRecords() turns a configured read failure into [], parses every file before the composer limit, and forwards raw frontmatter/body without translating canonical review data.
• Premise Coherence: The local-sync/no-live-GitHub boundary coheres with verify-before-assert. Conflating unavailable with empty, and unknown review/draft state with negative state, conflicts with the same honesty requirement.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #15382
• Related Graph Nodes: Related: #15339, #13015; canonical sync grammar; Fleet PR/lane activity adapter.

🔬 Depth Floor

Challenge: The ticket assumes the synced record already has the GitHub-Workflow-shaped review/draft fields. The source of authority disproves that assumption: CONTENT_GRAMMAR.md lists only lifecycle frontmatter and serializes reviews under ## Reviews; a real synced record follows that grammar. Meanwhile createPrActivityEvents() reads reviewDecision, isDraft, and structured reviews via getPrHumanGateState(). Spreading frontmatter plus body therefore cannot emit truthful review/human-gate facts.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: the “opens, reviews, and merges” framing is not substantiated for reviews.
• Anchor & Echo summaries: terminology is clear, but the reader summary repeats the incorrect fail-soft/consumer-shape claims.
• Linked anchors: the ticket requires unreadable-source degradation and a bounded read; this head implements neither.

Findings: Drift requires mechanical repair: the PR calls configured failure “honest-empty,” claims the adapter-consumed record shape without producing its review/draft inputs, and declares no residual while deferring the live close-target receipt.

🧠 Graph Ingestion Notes

• [KB_GAP]: Downstream readers need to treat CONTENT_GRAMMAR.md as the projection contract; pull frontmatter is not a full live GitHub PR payload.
• [TOOLING_GAP]: The new tests exercise only happy-path parsing, absent-directory empty behavior, and a stray file; they do not cover the ticket’s degradation, bound, or review-state contracts.
• [RETROSPECTIVE]: Synced projections must preserve three distinct states—valid empty, unavailable, and unknown—and apply resource bounds before parsing, not after DTO composition.

🎯 Close-Target Audit

• Close-targets identified: #15382
• #15382 confirmed not epic-labeled.

Findings: The target type passes, but the close claim is premature while AC1, AC3, and AC4 remain unproven or contradicted.

📑 Contract Completeness Audit

• The originating ticket states a concrete internal contract: local-only reads, configured-source degradation, pre-read bounding, and the consumer fields reviewDecision / isDraft.
• The diff matches that contract.

Findings: Contract drift. issueFocusSections.mjs:695-725 returns [] when collection fails and reads every discovered file; wireFleetActivityReadSource.mjs therefore reaches the builder’s success path. The raw record also lacks the review/draft projection consumed by fleetPrLaneActivityAdapter.mjs:94-125.

🪜 Evidence Audit

• PR body contains an Evidence: declaration.
• Achieved evidence meets the close target, or the residual is declared consistently.
• The close-target marks the live residual as deferred.
• The two-ceiling declaration matches the ticket’s required evidence.
• No external receipt is being used as exact-head proof.

Findings: Evidence-AC mismatch. The issue explicitly requires a live devFleetServer + fleetActivity receipt, while the PR says “L2 → L2 required,” “Residual: none,” and then defers that same receipt to post-merge. Either supply the reachable exact-head live receipt or retain the residual/close target honestly under the Evidence Ladder.

N/A Audits — 📡 🔗

N/A across listed dimensions: this PR changes no MCP OpenAPI description, skill substrate, startup convention, or predecessor workflow contract.

🧪 Test-Evidence & Location Audit

• Execution evidence: all required CI is green at exact head 1d5202e020ddc95b85edab8e4d4aaf11f284efb3; author unit receipt is present, but the required live receipt is absent.
• Reviewer falsifiers:
  • configured unreadable source: readSyncedPullRecords() catches collection failure and returns []; the wiring then invokes the builder without error, producing capability.state = "wired", not "degraded";
  • bound: the current corpus has 334 pull markdown files, and the function loops over and parses all discovered files before createFleetPrLaneActivitySnapshot() applies .slice(0, limit);
  • projection: CONTENT_GRAMMAR.md and a real ## Reviews record show review state lives in body headers, while the consumer reads structured reviews / reviewDecision; the new fixture contains neither.
• Test location: the new unit spec is in the canonical Playwright unit tree.

Findings: Exact-head CI is green, but the named source/corpus falsifiers fail three delivered behaviors that the current tests do not exercise.

📋 Required Actions

To proceed with merging, please address the following:

• Preserve absent-vs-unreadable semantics. An intentionally omitted pullsDir may remain honest-empty, but a configured directory that cannot be collected/read must reach makeReadPrLaneSnapshot()’s catch and the builder’s error path. Add a wiring-level witness asserting a named degraded PR/lane capability and source-degraded event.
• Apply the bound before content parsing. Thread the requested limit into the reader, choose a deterministic bounded candidate set before fs.readFileSync / gray-matter (numeric PR-descending is an existing repo precedent), and add a large-tree witness proving files beyond the bound are not parsed.
• Translate the canonical synced-pull grammar into the consumer’s truthful contract. Parse ## Reviews into structured review facts / decision state, provide a real source for draft state (or preserve it explicitly as unknown rather than coercing absence to false), and add fixtures proving approved/changes-requested human-gate output. The “reviews” claim must be mechanically observable.
• Close the evidence gap: either add the exact-head live devFleetServer + fleetActivity receipt required by AC1, or change the close relationship/residual declarations so #15382 remains open until that receipt exists. “Residual: none” cannot coexist with required post-merge validation.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 62 - Correct local-sync and boot boundary, but unavailable/empty semantics and projection-boundary translation are wrong.
• [CONTENT_COMPLETENESS]: 58 - JSDoc and PR narrative are substantial, yet both overstate the consumer shape and evidence closure.
• [EXECUTION_QUALITY]: 54 - Green exact-head CI and clean targeted code, but explicit failure and bound contracts are not implemented.
• [PRODUCTIVITY]: 48 - Open/merge records can now reach the feed, but review facts, degradation, and bounded-resource behavior remain incomplete.
• [IMPACT]: 72 - A truthful PR activity feed materially improves Fleet coordination visibility.
• [COMPLEXITY]: 57 - The remaining work spans source projection, error semantics, bounding, and focused witnesses without requiring a redesign.
• [EFFORT_PROFILE]: Maintenance - A bounded completion of an existing reader/wiring lane.

The integration direction is worth keeping. This is the single bounded changes-requested round; at a repaired exact head, the next pass should converge on approval unless a genuine behavioral blocker remains.

[review-budget-managed]

• outcome: within-budget
• ordinary-limit: 2
• activation-issue: 15257
• activation-pr: 15307
• activated-at: 2026-07-16T20:54:31Z