Skip to content

fix(sync): include newly discovered threads in bounded acquisition - #292

Merged
vincentkoc merged 5 commits into
mainfrom
fix/automatic-private-report-cadence-20261017
Oct 17, 2026
Merged

vincentkoc merged 5 commits into
mainfrom
fix/automatic-private-report-cadence-20261017

Conversation

@vincentkoc

@vincentkoc vincentkoc commented Oct 17, 2026 •

Copy link
Copy Markdown
Member

What Problem This Solves

Bounded Discord acquisition can miss newly discovered text-channel threads because its second pass uses a fixed target list.

User Impact

The private publisher includes stored direct threads of selected text and announcement roots in its bounded scan. Ordinary channel selection stays unchanged. The publisher remains manual-only; automatic cadence is held until measured checkpoint and publication qualification succeeds.

Why This Change Was Made

A default-off native --include-stored-threads option expands only direct, same-guild thread children of the explicitly selected roots. It reuses native filtering, permissions, and history receipts without another collector or policy rewrite.

The workflow pins one reviewed policy pair for both admission and acquisition, removing routine policy inputs. Optional member refresh belongs only to the first guild-wide phase; the targeted phase always skips members. Both phases retain one 1,200-second budget. Prepared daily routing has no cron trigger in this change.

This composes with the checkpoint save and recovery repair, preserving its 840-second save deadline and publication guards. Runtime/workflow LOC is +71/-30 (net +41), justified by the opt-in native selector and scoped acquisition routing; tests are +279/-8.

Evidence

  • All 88 Python checkpoint/acquisition tests pass, including native argument passthrough and member-failure ordering.
  • Three Go workflow/publication fixtures pass, including manual-only admission, shared policy pins, and safe member-flag routing.
  • Focused native syncer and CLI tests pass for direct-thread expansion, default selection, exclusions, permission failures, and member flags.
  • Both independent composition reviews approved the exact head. Fresh P3 completed scoped-clean, and all 18 hosted checks passed, including CI run 39050283942. The checked merge tree exactly matches the reviewed composition.
  • Live acquisition with the new selector and a successful end-to-end checkpoint/publication timing remain required before automatic cadence activation.
  • ClawSweeper reviewed this exact head with no findings. Its optional Rank-up move for a redacted native CLI thread-receipt trace is deferred to that separately reviewed manual acquisition: it requires the merged entry point and remains an activation gate, rather than an additional pre-merge run.

@clawsweeper

clawsweeper Bot commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. labels Oct 17, 2026
@clawsweeper

clawsweeper Bot commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

What this changes

This PR adds opt-in stored-thread expansion to targeted sync and uses it in private bounded acquisition while preserving manual-only publication.

Example: The regression fixture discovers new-thread beneath the selected text channel root in guild g1.

  • Before: The subsequent bounded scan of root leaves new-thread without a history-window receipt.
  • After: With --include-stored-threads, the bounded scan includes new-thread and records its completed window.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) Useful, focused implementation with no concrete correctness finding; confidence is supported by source and harness coverage rather than demonstrated shipped-entry-point behavior.
Proof confidence 🦐 gold shrimp (3/6) This maintainer-authored PR is outside the ordinary contributor proof gate; supplied evidence covers SQLite-backed syncer fixtures and workflow argument routing, while live selector acquisition and operational timing remain expressly deferred to cadence qualification.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: Newly discovered text-channel threads can lack bounded coverage evidence even after the private publisher refreshes the guild.
Reason: The maintainer-authored direction supplies a narrow opt-in capability for the demonstrated acquisition gap without changing ordinary channel selection or activating schedules.

Merge readiness

✅ Ready for maintainer review

Keep this maintainer-authored draft open: it addresses a bounded thread-coverage gap still present on main, and source review found no actionable introduced defect.

Priority: P2
Reviewed head: 5888ce2986bf91c885e3c4dd86edcd98a1e11ed3

Before merge

None.

Findings

None.

Tests

  • Missing end-to-end proof: No supplied shipped-CLI run demonstrates discovery followed by bounded thread coverage; the fixture contains the before/after assertions, but a base-fail/head-pass execution was not supplied.
Agent review details

How this fits together

The private publisher supervises native Discord sync, which selects channels from live and stored metadata, writes messages and coverage receipts to SQLite, then saves a verified checkpoint before archive publication.

flowchart TD
  A[Reviewed policy] --> D[Native target selection]
  B[Guild latest refresh] --> C[Stored channel catalog]
  C --> D
  D --> E[Bounded history scan]
  E --> F[Verified private checkpoint]
  F --> G[Archive publication]
Loading

Technical review

Best possible solution:

Keep the focused native selector and shared acquisition owner, with automatic cadence disabled until its separately recorded operational qualification is complete.

Do we have a high-confidence way to reproduce the issue?

Current-main acquisition and message filtering show why newly stored text-channel children miss the second bounded pass, and the added fixture expresses that exact before/after case; no target code was run.

Is this the best way to solve the issue?

The opt-in native expansion addresses the selection mismatch at its existing owner while preserving default selection, exclusions, permissions, and history receipt handling.

AGENTS.md: found but not applied because it conflicted with ClawSweeper's review contract.

Codex review notes: model internal, reasoning medium; reviewed against 206aeb386ca1.

Provenance checked

Testing

Proof path: in-process harness.

Security

None.

Evidence

What I checked:

Likely related people:

  • Vincent Koc: Raw commit 64ad0cc adds scripts/private_discord_checkpoint.py:35 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 64ad0cc548d9; files: scripts/private_discord_checkpoint.py)
  • Peter Steinberger: Raw commit 0b3c311 adds internal/syncer/channel_catalog.go:28 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 0b3c31103e61; files: internal/syncer/channel_catalog.go)
  • Ayaan Zaidi: Raw commit 938b6b2 adds internal/syncer/channel_catalog.go:223 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: medium; commits: 938b6b279dfc; files: internal/syncer/channel_catalog.go)

Review metrics

Metric Value Why it matters
Production and test scope Runtime/workflow +71/-30; tests +279/-8 The small production expansion reuses native selection and traversal; the larger test change covers scope boundaries and acquisition routing.

Labels

Label changes:

  • add P2: The missing bounded receipts affect selected newly discovered threads in an advanced private acquisition workflow; ordinary channel selection remains intact.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Label justifications:

  • P2: The missing bounded receipts affect selected newly discovered threads in an advanced private acquisition workflow; ordinary channel selection remains intact.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • A redacted native CLI trace showing a newly discovered text thread receive its bounded-window receipt would strengthen behavioral confidence.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed October 17, 2026, 12:51 PM ET / 16:51 UTC.

@vincentkoc
vincentkoc marked this pull request as ready for review October 17, 2026 16:55
@vincentkoc
vincentkoc requested a review from a team as a code owner October 17, 2026 16:55
@vincentkoc
vincentkoc merged commit 95d1154 into main Oct 17, 2026
26 checks passed
@vincentkoc
vincentkoc deleted the fix/automatic-private-report-cadence-20261017 branch October 17, 2026 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant