Skip to content

fix(rebuild): target stale thread materializations - #3278

Merged
Sinity merged 1 commit into
masterfrom
feature/fix/membership-head-provenance
Jul 26, 2026
Merged

fix(rebuild): target stale thread materializations#3278
Sinity merged 1 commit into
masterfrom
feature/fix/membership-head-provenance

Conversation

@Sinity

@Sinity Sinity commented Jul 26, 2026

Copy link
Copy Markdown
Owner

Summary

Repair thread-only insight debt without an archive-wide aggregate refresh, and expose precise timings for terminal index-rebuild stages.

Problem

A full replay left four sessions with stale thread materialization markers because null source sort keys were replaced by a thread high-water mark. The repair planner excluded thread markers from targeted candidates and could classify thread-only debt as already ready. Terminal bulk derived rebuilds had no stage-level visibility.

Solution

Preserve null sort keys, include thread markers in candidate selection, rebuild only affected roots, and log FTS/trigram/action-pair/delegation/parity/readiness/promotion durations.

Verification

  • — 1 passed
  • — 3 passed
  • — 1 passed
  • {
    "timestamp": "2026-07-26T10:36:07.775809+00:00",
    "git_head": "1a001af05afa3b1045fe199fc99d58f68124d687",
    "tier": "quick",
    "run_id": "20260726T103537Z-quick-358301-4b0619a3",
    "artifact_dir": ".cache/verify/runs/20260726T103537Z-quick-358301-4b0619a3",
    "steps": [
    {
    "name": "ruff format",
    "duration_s": 0.01,
    "exit": 0,
    "run_id": "20260726T103537Z-quick-358301-4b0619a3",
    "artifact_dir": ".cache/verify/runs/20260726T103537Z-quick-358301-4b0619a3/steps/01-ruff-format"
    },
    {
    "name": "ruff check",
    "duration_s": 0.01,
    "exit": 0,
    "run_id": "20260726T103537Z-quick-358301-4b0619a3",
    "artifact_dir": ".cache/verify/runs/20260726T103537Z-quick-358301-4b0619a3/steps/02-ruff-check"
    },
    {
    "name": "mypy",
    "duration_s": 0.55,
    "exit": 0,
    "run_id": "20260726T103537Z-quick-358301-4b0619a3",
    "artifact_dir": ".cache/verify/runs/20260726T103537Z-quick-358301-4b0619a3/steps/03-mypy"
    },
    {
    "name": "render all",
    "duration_s": 8.59,
    "exit": 0,
    "run_id": "20260726T103537Z-quick-358301-4b0619a3",
    "artifact_dir": ".cache/verify/runs/20260726T103537Z-quick-358301-4b0619a3/steps/04-render-all"
    },
    {
    "name": "verify topology",
    "duration_s": 0.46,
    "exit": 0,
    "run_id": "20260726T103537Z-quick-358301-4b0619a3",
    "artifact_dir": ".cache/verify/runs/20260726T103537Z-quick-358301-4b0619a3/steps/05-verify-topology"
    },
    {
    "name": "verify layering",
    "duration_s": 3.9,
    "exit": 0,
    "run_id": "20260726T103537Z-quick-358301-4b0619a3",
    "artifact_dir": ".cache/verify/runs/20260726T103537Z-quick-358301-4b0619a3/steps/06-verify-layering"
    },
    {
    "name": "verify closure-matrix",
    "duration_s": 0.3,
    "exit": 0,
    "run_id": "20260726T103537Z-quick-358301-4b0619a3",
    "artifact_dir": ".cache/verify/runs/20260726T103537Z-quick-358301-4b0619a3/steps/07-verify-closure-matrix"
    },
    {
    "name": "lab schema roundtrip",
    "duration_s": 0.91,
    "exit": 0,
    "run_id": "20260726T103537Z-quick-358301-4b0619a3",
    "artifact_dir": ".cache/verify/runs/20260726T103537Z-quick-358301-4b0619a3/steps/08-lab-schema-roundtrip"
    },
    {
    "name": "verify manifests",
    "duration_s": 1.78,
    "exit": 0,
    "run_id": "20260726T103537Z-quick-358301-4b0619a3",
    "artifact_dir": ".cache/verify/runs/20260726T103537Z-quick-358301-4b0619a3/steps/09-verify-manifests"
    },
    {
    "name": "verify ci-workflows",
    "duration_s": 0.35,
    "exit": 0,
    "run_id": "20260726T103537Z-quick-358301-4b0619a3",
    "artifact_dir": ".cache/verify/runs/20260726T103537Z-quick-358301-4b0619a3/steps/10-verify-ci-workflows"
    },
    {
    "name": "verify doc-commands",
    "duration_s": 2.62,
    "exit": 0,
    "run_id": "20260726T103537Z-quick-358301-4b0619a3",
    "artifact_dir": ".cache/verify/runs/20260726T103537Z-quick-358301-4b0619a3/steps/11-verify-doc-commands"
    },
    {
    "name": "verify docs-coverage",
    "duration_s": 2.5,
    "exit": 0,
    "run_id": "20260726T103537Z-quick-358301-4b0619a3",
    "artifact_dir": ".cache/verify/runs/20260726T103537Z-quick-358301-4b0619a3/steps/12-verify-docs-coverage"
    },
    {
    "name": "verify test-infra-currency",
    "duration_s": 0.39,
    "exit": 0,
    "run_id": "20260726T103537Z-quick-358301-4b0619a3",
    "artifact_dir": ".cache/verify/runs/20260726T103537Z-quick-358301-4b0619a3/steps/13-verify-test-infra-currency"
    },
    {
    "name": "verify test-clock-hygiene",
    "duration_s": 2.24,
    "exit": 0,
    "run_id": "20260726T103537Z-quick-358301-4b0619a3",
    "artifact_dir": ".cache/verify/runs/20260726T103537Z-quick-358301-4b0619a3/steps/14-verify-test-clock-hygiene"
    },
    {
    "name": "verify pytest-timeout-overrides",
    "duration_s": 3.96,
    "exit": 0,
    "run_id": "20260726T103537Z-quick-358301-4b0619a3",
    "artifact_dir": ".cache/verify/runs/20260726T103537Z-quick-358301-4b0619a3/steps/15-verify-pytest-timeout-overrides"
    },
    {
    "name": "verify degrade-loudly",
    "duration_s": 1.23,
    "exit": 0,
    "run_id": "20260726T103537Z-quick-358301-4b0619a3",
    "artifact_dir": ".cache/verify/runs/20260726T103537Z-quick-358301-4b0619a3/steps/16-verify-degrade-loudly"
    }
    ],
    "total_duration_s": 30.09,
    "exit_code": 0
    } — passed

No tracker file is included.

Summary by CodeRabbit

  • Bug Fixes

    • Improved archive replay handling to preserve the correct accepted revision when quarantined records temporarily appear as the session head.
    • Preserved missing session sort keys during thread insight rebuilding.
    • Improved targeted session-insight repair readiness and reporting.
  • Performance & Diagnostics

    • Added stage-level rebuild timing and completion diagnostics.
    • Planner statistics now refresh periodically and target core tables for more efficient analysis.
  • Reliability

    • Added regression coverage for archive replay, insight rebuilding, planner statistics, and derived-state timing.

@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@Sinity, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 53 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 5bc19857-dd43-4522-a849-c720990faaa4

📥 Commits

Reviewing files that changed from the base of the PR and between 1a001af and 872bf34.

📒 Files selected for processing (1)
  • polylogue/maintenance/rebuild_index.py
📝 Walkthrough

Walkthrough

The rebuild flow now emits stage timings, refreshes targeted planner statistics periodically, preserves null session sort keys, accounts for aggregate insight debt, and restores persisted byte-governed heads during membership replay. Tests cover timing outputs, planner refresh policy, null keys, repair targeting, and replay authority.

Changes

Rebuild diagnostics and planner statistics

Layer / File(s) Summary
Bounded rebuild timing and planner statistics
polylogue/maintenance/rebuild_index.py, tests/unit/maintenance/test_rebuild_index_bulk_build.py, tests/unit/storage/test_planner_statistics_seed.py
Bulk-build substages and terminal rebuild stages now emit elapsed-time logs. Planner statistics use periodic bounded refreshes and targeted ANALYZE statements for writer-hot tables, with tests covering timing keys and analyzed tables.

Session-insight materialization and repair

Layer / File(s) Summary
Session-insight materialization and repair
polylogue/storage/insights/session/rebuild.py, polylogue/storage/repair.py, tests/unit/storage/test_session_insight_refresh.py, tests/unit/storage/test_repair.py
Thread materialization preserves NULL session sort keys. Targeted repair selection and readiness accounting include aggregate debt, while dry-run counts report rebuild candidates without adding aggregate debt.

Persisted membership-head authority

Layer / File(s) Summary
Persisted membership-head authority
polylogue/storage/sqlite/archive_tiers/archive.py, tests/unit/storage/test_revision_replay.py
Membership replay restores an existing byte-governed persisted head when a quarantined cohort masks it, suppressing conflicting replacement and deletion paths. Regression coverage validates the resulting head and supersession receipt.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 46.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the main change: targeting stale thread materializations.
Description check ✅ Passed The description covers the required Summary, Problem, Solution, and Verification sections with sufficient detail.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/fix/membership-head-provenance

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1a001af05a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

measured = conn.execute("SELECT count(*) FROM sqlite_stat1").fetchone()[0]
assert measured > 0
measured_tables = {str(row[0]) for row in conn.execute("SELECT DISTINCT tbl FROM sqlite_stat1").fetchall()}
assert {"sessions", "messages", "blocks", "session_links", "action_pairs"} <= measured_tables

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Populate analyzed tables before asserting statistics

This test deletes all seeded statistics but inserts no table rows, and SQLite does not create sqlite_stat1 entries when ANALYZE runs against empty tables. Consequently, the focused node tests/unit/storage/test_planner_statistics_seed.py::test_refresh_generation_planner_statistics_measures_real_tables deterministically fails with measured_tables == set() instead of validating the new table-selection policy; seed representative rows before refreshing statistics or change the assertion to account for empty tables.

AGENTS.md reference: AGENTS.md:L351-L357

Useful? React with 👍 / 👎.

Problem: rebuild_index.py's bulk-build derived-state repopulate still
called rebuild_fts_index_sync with a resume_from_empty_message_index
kwarg that master already dropped from the function signature via a
separately-merged FTS convergence pass — this branch had gone stale
enough that its own remaining net diff (once rebased past everything
master already absorbed in parallel) reduced to this one call.

What changed: call rebuild_fts_index_sync with no extra kwarg, matching
its current signature.

Verification: devtools test tests/unit/maintenance/test_rebuild_index_bulk_build.py
tests/unit/storage/test_planner_statistics_seed.py; devtools verify --quick.

Co-Authored-By: Codex <noreply@openai.com>
@Sinity
Sinity force-pushed the feature/fix/membership-head-provenance branch from 1a001af to 872bf34 Compare July 26, 2026 19:01
@Sinity

Sinity commented Jul 26, 2026

Copy link
Copy Markdown
Owner Author

Rebased onto current master: 3 of the 4 commits (0855718f1, 13c9827ca, d10b9d1b6) turned out fully subsumed by unrelated work merged in parallel elsewhere (#3270/#3271/#3272) — same planner-statistics-refresh idea landed twice across independent worktrees. Net remaining diff collapses to one real fix: rebuild_index.py was still calling rebuild_fts_index_sync(conn, resume_from_empty_message_index=True), a kwarg master's FTS convergence pass already dropped from the signature — this would have been a live TypeError. Squashed the rebase to a single commit (872bf34) carrying just that fix plus this PR's original unique contribution (stale thread-materialization targeting, from 1a001af, unchanged).

Verification: devtools test on the touched-area tests (123 passed) + devtools verify --quick (pass).

@Sinity
Sinity merged commit 30b2e89 into master Jul 26, 2026
3 checks passed
@Sinity
Sinity deleted the feature/fix/membership-head-provenance branch July 26, 2026 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant