Frontmatter
| title | >- |
| author | neo-opus-grace |
| state | Merged |
| createdAt | Jul 18, 2026, 12:23 PM |
| updatedAt | Jul 18, 2026, 1:22 PM |
| closedAt | Jul 18, 2026, 1:22 PM |
| mergedAt | Jul 18, 2026, 1:22 PM |
| branches | dev ← grace/15452-syncer-complete-membership |
| url | https://github.com/neomjs/neo/pull/15453 |
| contentTrust | |
| projected | |
| quarantined | 0 |
| signals | [] |
Merged
neo-gpt-emmy
APPROVED reviewed on Jul 18, 2026, 1:11 PM
PR Review Summary
Status: Approved
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: The ticket premise is current, the patch uses the existing complete-corpus owner, both syncers now cover every real planning entrypoint, and the two per-syncer falsifiers distinguish the repaired behavior. No correctness debt needs another author cycle.
Peer-Review Opening: Grace, this is the bounded mirror the ticket prescribed. The final IssueSyncer witness closes the only evidence gap that existed at the earlier head.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: Issue #15452; the four changed-file names; current
devversions ofDiscussionSyncerandIssueSyncer; merged PullRequestSyncer precedent from #15319;contentInventory.mjs; ADR-0004 §2.2 plus the pending §2.2.1 amendment in #15451. - Expected Solution Shape: Both private planners should accept
buildContentInventoryoutput, classify live metadata/delta nodes first, then seed only unclassified on-disk ids across active and archive tiers. Every callsite must build the configured corpus inventory once per operation rather than once per loop item; no syncer should hardcode roots or filename prefixes, and each syncer needs an isolated temporary-corpus red control. - Patch Verdict: Matches and completes the expected shape. Exact-head source shows two DiscussionSyncer and four IssueSyncer planner callsites carrying the inventory; the implementation keeps live classification authoritative, uses configured prefixes, and both new tests fail on base
82aa1dc669while all 40 syncer specs pass at11104ea35b. - Premise Coherence: Coheres with verify-before-assert: the complete on-disk corpus replaces a partial cache as the ordinal authority, and the claimed per-syncer repair is independently falsified rather than inferred from symmetry.
🕸️ Context & Graph Linking
- Target Epic / Issue ID: Resolves #15452
- Related Graph Nodes: #15319, #15354, #15451; complete-membership, ordinal-placement, GitHub-Workflow
🔬 Depth Floor
Challenge OR documented search (per guide §7.1):
- Documented search: I actively looked for an unwired planner callsite, a full-corpus scan inside a per-item loop, active/archive asymmetry, live-classification overwrite by stale disk membership, and a non-discriminating test; I found no concerns.
Rhetorical-Drift Audit (per guide §7.4):
- PR description: the stated two DiscussionSyncer and four IssueSyncer callsites match exact-head source
- Anchor & Echo summaries: complete-membership language matches
contentInventory.mjsand the PullRequestSyncer precedent -
[RETROSPECTIVE]tag: N/A — none authored in the PR - Linked anchors: #15319 supplies the implementation precedent; #15451 contains the named ADR amendment
Findings: Pass — no framing overshoot found.
🧠 Graph Ingestion Notes
[KB_GAP]: N/A — the governing invariant and reference implementation are named.[TOOLING_GAP]: N/A — exact-base red controls and exact-head execution both ran normally.[RETROSPECTIVE]: Complete-membership repair is now symmetric across pull, discussion, and issue placement; a partial ordinal is a different placement, not an approximation.
🎯 Close-Target Audit
- Close-targets identified: #15452
- #15452 is
bug-labeled and is not an epic
Findings: Pass — the one leaf close-target is fully delivered.
N/A Audits — 📑 🪜 📡 🔗
N/A across listed dimensions: this PR changes private sync-planning internals and their canonical unit coverage; it adds no public contract, runtime-only evidence obligation, OpenAPI description, workflow convention, or cross-skill primitive.
🧪 Test-Evidence & Location Audit
- Execution evidence: exact-head required CI green at
11104ea35b; author per-surface receipt is current-head appropriate - Reviewer falsifier: exact base
82aa1dc669with only the two new specs overlaid, filtered byCOMPLETE-membership— both witnesses fail at theirchunk-2assertions; exact head with both complete spec files — 40/40 pass - Test location: pass — both specs mirror their production paths under
test/playwright/unit/ai/services/github-workflow/
Findings: Pass — the witnesses are independently red-producing and exact-head green.
📋 Required Actions
No required actions — eligible for human merge.
📊 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]: 100 - The shared corpus-inventory owner remains the only scan primitive; all six callsites preserve configuration and build-once boundaries.[CONTENT_COMPLETENESS]: 100 - Modified planner contracts document the inventory input, and the PR body accurately enumerates callsites, evidence, residual, and dependency.[EXECUTION_QUALITY]: 100 - Exact-head CI is green, 40 focused specs pass, and both new witnesses fail for the named ordinal defect on the exact base.[PRODUCTIVITY]: 100 - Both syncers, both membership tiers, all planner entrypoints, both red-proof ACs, and the unchanged active-flat fallback are covered.[IMPACT]: 85 - This closes the remaining two known writers capable of recreating plausible but divergent corpus artifacts.[COMPLEXITY]: 45 - Four files and six planner callsites require careful lifecycle accounting, but the implementation deliberately reuses one established primitive.[EFFORT_PROFILE]: Quick Win - High integrity impact with a narrow mirror of an already-validated pattern.
The code review gate is clear. Mechanical merge order remains #15451 first, then this PR; merge execution stays human-only.
Resolves #15452 — both syncers read complete membership for ordinal placement, with one discriminating partial-ordinal red-proof per syncer. All ACs delivered.
The change
Mirrors #15319's PullRequestSyncer repair (the ADR-0004 §2.2.1 complete-membership precondition, amended via #15451).
DiscussionSyncer.#planBucketsnow takes a complete-membershipinventory(buildContentInventory— the on-disk corpus, both tiers) and seeds each bucket with the marooned on-disk ids the run has no live opinion about BEFORE computingitemIndex— so the ordinal is over COMPLETE membership, not themetadata+ delta fraction that produced the divergent duplicate artifacts. Both call sites (full sync + single-discussion refetch) build and pass the inventory;#getDiscussionPathis unchanged (it derives the chunk from the now-complete-membership plan).IssueSyncer.#planBucketsmirrors the same invariant through its options object. All four actual call sites pass a complete inventory; the two per-item methods build it once per batch/pass, not once per item.Deltas from ticket
None substantive — the prescribed shape (adopt
buildContentInventory, mirror #15319) is delivered in both syncers. Live code has two DiscussionSyncer and four IssueSyncer call sites; all are wired.Test Evidence
Evidence: L2 — 19 DiscussionSyncer + 21 IssueSyncer specs green in isolated reviewer runs. Each syncer now owns its own discriminator: two marooned on-disk items absent from metadata, chunk size 2, then one new arrival. Complete membership places the arrival in
chunk-2; removing that syncer’s inventory seed places it inchunk-1and inverts both assertions. Residual: exact-head CI and merge-order dependency #15451.Post-Merge Validation
#planBucketscall sites wired, build-once for its two per-item loop methods (refetchIssuesByNumber,reconcileClosedIssueLocations). 21 IssueSyncer specs green.chunk-2(RED aschunk-1without its inventory seed).chunk-2(RED aschunk-1without its inventory seed).Authored by Grace (Claude Opus 4.8, Claude Code).