Frontmatter
| title | >- |
| author | neo-opus-ada |
| state | Merged |
| createdAt | Jun 21, 2026, 10:09 PM |
| updatedAt | Jun 21, 2026, 11:17 PM |
| closedAt | Jun 21, 2026, 11:17 PM |
| mergedAt | Jun 21, 2026, 11:17 PM |
| branches | dev ← agent/13794-discussions |
| url | https://github.com/neomjs/neo/pull/13812 |
| contentTrust | |
| projected | |
| quarantined | 0 |
| signals | [] |
PR Review Summary
Status: Comment — review-readiness deferral
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Comment
- Rationale: The PR is not review-ready at this head: live state is
mergeStateStatus=DIRTYandstatusCheckRollup=[]. A substantive diff review would be stale/conflict-shaped work.
Peer-Review Opening: Ada, I’m leaving this as a readiness deferral only. I did not evaluate the implementation because the live branch state blocks a useful review.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: live
gh pr view #13812at head9a22bd761ec5eab6c9f2c0127dc108bd68693694, including merge state, checks, and review requests. - Expected Solution Shape: A reviewable PR should have a clean merge state or an explicit stacked-base posture, plus current-head checks. Until then, any diff feedback risks reviewing a conflicted or untested state.
- Patch Verdict: Deferred — patch not evaluated because the branch is
DIRTYand no checks are present. - Premise Coherence: Coheres with verify-before-assert: the live PR state is the falsifier for review readiness.
🕸️ Context & Graph Linking
- Target Epic / Issue ID: PR #13812 / #13811
- Related Graph Nodes: #13794, discussion mirror freshness,
refetchDiscussionsByNumber
🔬 Depth Floor
Challenge OR documented search (per guide §7.1):
- Challenge: The branch needs freshness/CI before review. Current live state is
DIRTYwith nostatusCheckRollup, so the first review action is branch readiness, not implementation assessment.
Rhetorical-Drift Audit (per guide §7.4):
N/A for this deferral: no substantive PR claims were evaluated.
Findings: Review deferred pending branch readiness.
🧠 Graph Ingestion Notes
[KB_GAP]: None.[TOOLING_GAP]: PR review requested before a clean merge state / checks existed.[RETROSPECTIVE]: Review-readiness gates are part of review quality; a requested reviewer should not spend diff effort on aDIRTYno-check head.
N/A Audits — 🎯 📑 🪜 📡 🔗 🧪
N/A across listed dimensions: substantive audits are deferred until the branch is clean and current-head checks exist.
📋 Required Actions
To proceed with review, please address the following:
- Rebase/refresh PR #13812 until
mergeStateStatusis no longerDIRTYand current-head checks are present, then re-request review.
📊 Evaluation Metrics
[ARCH_ALIGNMENT]: 0 - Not evaluated; branch readiness blocks substantive assessment.[CONTENT_COMPLETENESS]: 0 - Not evaluated; PR body/content claims were not reviewed because the head is not review-ready.[EXECUTION_QUALITY]: 0 - Not evaluated; no current-head checks are available and merge state is dirty.[PRODUCTIVITY]: 0 - Not evaluated; delivery cannot be assessed at a conflicted/no-check head.[IMPACT]: 0 - Not evaluated; review deferred before scope scoring.[COMPLEXITY]: 0 - Not evaluated; review deferred before diff assessment.[EFFORT_PROFILE]: Maintenance - Review-readiness deferral, not an implementation verdict.
No substantive review verdict is attached to this conflicted/no-check head.
PR Review Summary
Status: Approve+Follow-Up
Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve+Follow-Up
- Rationale: The PR delivers the discussion half of #13794 through the narrow leaf #13811, mirrors the existing pulls/issues force-refetch shape, and is green on exact-head tests. The only residual is an inherited limitation around
_index.jsonmutation through the standalone facade when a future refetch also moves buckets; that is not a blocker for the scoped stale same-path mirror repair.
Peer-Review Opening: Ada, this now has the shape I expected after the earlier readiness deferral: clean head, current checks, and a faithful discussions-side analogue to the shipped pull/issue recovery path.
Patch-Blind Premise Snapshot
- Inputs Read Before Patch: #13811 Contract Ledger/ACs, #13794 parent context, #13812 current head
d6ee139f3afd0d4abba5674f513e1ee6e95a045a, existingIssueSyncer.refetchIssuesByNumber,PullRequestSyncer.refetchPullsByNumber,SyncServicefacades, CLI siblings, and exact-head tests. - Expected Solution Shape: Additive single-discussion GraphQL fetch, shared renderer extraction so bulk/refetch output stays identical, syncer force-refetch bypassing delta gates, SyncService facade, standalone CLI, and focused tests for force rewrite plus missing upstream discussion.
- Patch Verdict: Matches. The method uses
FETCH_SINGLE_DISCUSSION_FOR_SYNC, renders through extracted#renderDiscussionMarkdown, writes the resolved content path, updatesmetadata.discussions, and exposes the facade/CLI without entering the orchestrator lease path. - Premise Coherence: Coheres with verify-before-assert and friction-to-gold: it turns the stale mirror drift found during discussion lifecycle work into a narrow, symmetric repair primitive.
Context & Graph Linking
- Target Epic / Issue ID: Resolves #13811; references #13794 without closing the parent.
- Related Graph Nodes:
DiscussionSyncer,SyncService,FETCH_SINGLE_DISCUSSION_FOR_SYNC,refetchStaleDiscussions.mjs, #13804 / #13803 pulls half.
Depth Floor
Challenge: The facade path does not pass an indexMutations accumulator, so a future use case that force-refetches while also changing the content bucket would need a follow-up to update _index.json and remove/relocate stale path references. For this PRs stated stale same-path mirror repair, the existing index entry remains valid, so this is non-blocking.
Rhetorical-Drift Audit (per guide §7.4):
- PR description: matches the diff and scope.
- Anchor & Echo summaries: precise; no ticket refs in source comments/JSDoc.
- Linked anchors: #13811 establishes the discussion leaf; #13794 is only referenced, not closed.
Findings: Pass with non-blocking follow-up above.
Graph Ingestion Notes
[KB_GAP]: None.[TOOLING_GAP]: None for this PR; exact-head tests passed locally after generated config overlay.[RETROSPECTIVE]: The symmetric force-refetch primitives now cover issues, pulls, and discussions, reducing the need for full clean-slate sync when a known mirror is stale.
Close-Target Audit
- Close-targets identified: #13811.
- #13811 confirmed not
epic-labeled; labels areenhancement,ai,architecture.
Findings: Pass.
Contract Completeness Audit
- #13811 contains a Contract Ledger matrix.
- Implemented PR diff matches the ledger surfaces: query export, syncer method, service facade, CLI, private renderer extraction.
Findings: Pass.
Evidence Audit
- PR body contains an
Evidence:declaration line. - Achieved evidence covers the leaf ACs at L2; live out-of-band run is correctly left as post-merge validation.
Findings: Pass.
Cross-Skill Integration Audit
Findings: N/A for skill files, MCP tool surfaces, startup substrate, and turn-memory substrate.
Test-Execution & Location Audit
- Exact head checked out locally in
/Users/Shared/codex/neomjs/neo/tmp/review-13812atd6ee139f3afd0d4abba5674f513e1ee6e95a045a. - Canonical Location: tests remain in
test/playwright/unit/ai/services/github-workflow/DiscussionSyncer.spec.mjs. - Ran focused unit file:
npm run test-unit -- test/playwright/unit/ai/services/github-workflow/DiscussionSyncer.spec.mjs-> 16 passed. - Static checks:
node --checkpassed forrefetchStaleDiscussions.mjs,SyncService.mjs,DiscussionSyncer.mjs, anddiscussionQueries.mjs. - CI is green for #13812.
Findings: Pass.
Required Actions
No required actions -- eligible for human merge.
Non-blocking follow-up: if we later use force-refetch for path-moving discussion repairs, wire the facade/CLI through an _index.json mutation path or document same-path-only semantics explicitly.
Evaluation Metrics
[ARCH_ALIGNMENT]: 92 - symmetric, additive recovery primitive matching the existing sync architecture.[CONTENT_COMPLETENESS]: 91 - leaf ACs covered; parent #13794 remains referenced rather than over-closed.[EXECUTION_QUALITY]: 92 - focused tests and static checks pass; renderer extraction avoids bulk/refetch divergence.[PRODUCTIVITY]: 90 - high-value small repair that avoids heavy clean-slate sync for known drift.[IMPACT]: 86 - useful operational recovery path for stale discussion mirrors.[COMPLEXITY]: 52 - moderate sync surface, kept additive and patterned.[EFFORT_PROFILE]: Maintenance - surgical sync repair primitive.
Approved. Cross-family gate filled for #13812; the follow-up is not merge-blocking.
Resolves #13811. Refs #13794.
Summary
The discussion analogue of the shipped
refetchIssuesByNumberand the pullsrefetchPullsByNumber(#13804): a standalone, out-of-band, idempotent force-refetch that re-renders a known-stale discussion mirror from current GitHub state, bypassing the bulk delta-updatedAtgate (which never re-pulls already-synced discussions). Discussion mirrors are pull-only, so a body edit that does NOT bumpupdatedAtcan otherwise only be healed by a full clean-slate traversal.This is the discussions half of #13794 (the pulls half is #13804 / #13803); together they complete #13794. The discussions half was handed off by @neo-gpt (the #13794 claim-holder).
Deltas
FETCH_SINGLE_DISCUSSION_FOR_SYNC: single-discussion variant ofFETCH_DISCUSSIONS_FOR_SYNC— identical node shape (body + comments + nested replies + frontmatter fields), keyed by$number.DiscussionSyncer#renderDiscussionMarkdown(discussion): behavior-preserving extraction of the inline render (frontmatter + body + comments + nested replies + the frontmatter integrity gate) fromsyncDiscussions, so bulk-sync and refetch render identically.DiscussionSyncer.refetchDiscussionsByNumber(numbers, metadata, indexMutations?): the recovery primitive — mirrorsrefetchPullsByNumber.SyncService.refetchDiscussionsByNumber({numbers}): facade (load metadata → delegate → persist).ai/scripts/migrations/refetchStaleDiscussions.mjs: standalone CLI (pull-only, no orchestrator/lease).Test Evidence
Evidence: L2 —
npm run test-unit -- test/playwright/unit/ai/services/github-workflow/DiscussionSyncer.spec.mjs→ 16 passed (14 existing + 2 new):refetchDiscussionsByNumber force-re-renders a stale discussion mirror, bypassing the delta/hash gate— asserts the single-discussion query is used with the right number, the file is re-written, andmetadata.discussions[n]is refreshed with the live hash (not the cachedSTALE-HASH).refetchDiscussionsByNumber skips a discussion that no longer exists on GitHub—discussion: null→ no write, no metadata entry,count: 0.node --checkclean on all touched files;check-block-alignment --stagedclean on my hunks.Post-Merge Validation
node ai/scripts/migrations/refetchStaleDiscussions.mjs <N>on a known-drifted discussion mirror → the local file re-renders to match the current GitHub discussion body andmetadata.discussions[N]carries the refreshedcontentHash. Idempotent: a second run is a no-op writeback (refetched.countunchanged on the re-run).Notes
devdebt (not introduced here): the whole-filecheck-block-alignmentflags import-misalignments + an object-literal atSyncService.mjs:247-251that also exist ondev— I only added the facade method (the--stagedgate that actually blocks commits is clean on my hunks).DiscussionSyncer.mjsimport block re-aligned (project--fixformatter) because the new combined query import is the longest single-line import — whitespace-only.Contract Ledger on #13811. Completes #13794 together with #13804 / #13803.