Skip to content

fix(live): avoid reclassifying established append cohorts#2849

Merged
Sinity merged 2 commits into
masterfrom
feature/fix/bound-append-memory
Jul 13, 2026
Merged

fix(live): avoid reclassifying established append cohorts#2849
Sinity merged 2 commits into
masterfrom
feature/fix/bound-append-memory

Conversation

@Sinity

@Sinity Sinity commented Jul 13, 2026

Copy link
Copy Markdown
Owner

Summary

Replace repeated historical full-snapshot classification on the established
live-append path with the existing durable revision replay plan. Incomplete or
legacy/crash-interrupted cohorts still take the conservative classifier
fallback; if the current append remains outside the accepted chain, it is
deferred without advancing the cursor.

Problem

On 2026-07-13, a 46 MiB actively appended Codex JSONL selected by catch-up
every ~16 seconds took 37–38 seconds per 0.1–0.3 MiB append. During one
cycle, anonymous memory rose from roughly 0.4 GiB to 4.2 GiB; the daemon
recorded 6,655 memory.high events before it was intentionally stopped. The
production evidence harness identified repeated
classify_raw_revision_cohort reads of every retained historical full
snapshot as the dominant avoidable work.

Solution

polylogue/sources/live/append_ingest.py now first derives the accepted chain
from durable raw-revision metadata immediately after recording the
byte-contiguous append witness. It invokes historical full-snapshot
classification whenever that metadata omits the current append. If the
classifier still cannot prove the current raw, the append remains deferred, so
the watcher cannot advance its cursor over unindexed bytes.

The production-route harness distinguishes the normal metadata route from the
fallback. It proves that an established cohort makes zero classifier and
historical-full reads, while incomplete cohorts still invoke classification and
are deferred until their authority proof includes the current append.

Ref polylogue-cnaj

Verification

  • devtools test tests/integration/test_append_cohort_memory.py — 4 passed.
  • devtools verify --quick — all 15 static/generated checks passed.
  • A targeted legacy unit selection was also run; two tests that create an
    append without any predecessor baseline deferred before either old or new
    replay path and therefore did not exercise their injected index/crash
    failures. The production-route integration coverage above supplies the
    established and recovery cohort checks for this change.

Problem:
Each small live append reran historical full-snapshot classification despite
already having a byte-proven predecessor and baseline. That repeatedly loaded
all retained full blobs and caused the v35 catch-up memory and I/O spike.

What changed:
Use the durable raw revision replay metadata on the established append path.
Keep historical classification as a recovery fallback only when that metadata
has no accepted chain. The production-route evidence harness now proves both
branches and bounds normal-path blob reads to the selected baseline and tail.

Compatibility/migration:
No archive schema or cursor format changes. Existing incomplete cohorts retain
the conservative classifier path.

Ref polylogue-cnaj

Co-Authored-By: Claude <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Append ingestion now prefers durable replay metadata and falls back to cohort classification only when no raw IDs are accepted. Instrumentation and integration tests cover the primary route, established cohorts, and incomplete-cohort recovery.

Changes

Durable replay metadata routing

Layer / File(s) Summary
Replay-plan routing and primary coverage
polylogue/sources/live/append_ingest.py, tests/integration/test_append_cohort_memory.py
Append ingestion uses raw_revision_replay_plan before classification, and integration coverage verifies bounded replay without historical full reads. Test setup can vary full-snapshot authority.
Fallback recovery and call instrumentation
tests/infra/append_cohort_memory_counter.py, tests/integration/test_append_cohort_memory.py
Instrumentation records replay-plan calls and accepted IDs. Tests verify established cohorts skip classification and incomplete cohorts recover through historical classification.

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

Sequence Diagram(s)

sequenceDiagram
  participant WatcherAppend
  participant AppendIngest
  participant ArchiveStore
  participant HistoricalFullBlob
  WatcherAppend->>AppendIngest: process appended raw payload
  AppendIngest->>ArchiveStore: raw_revision_replay_plan(logical_source_key)
  ArchiveStore-->>AppendIngest: replay plan
  alt no accepted raw IDs
    AppendIngest->>ArchiveStore: classify_raw_revision_cohort(logical_source_key)
    ArchiveStore->>HistoricalFullBlob: read_all historical full blob
    HistoricalFullBlob-->>ArchiveStore: historical snapshot
    ArchiveStore-->>AppendIngest: recovery replay plan
  end
  AppendIngest-->>WatcherAppend: execute bounded replay
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Title check ✅ Passed The title accurately and concisely describes the main change: avoiding reclassification for established append cohorts.
Description check ✅ Passed The description is mostly complete with Summary, Problem, Solution, and Verification, though Changelog and Risks/Follow-ups are omitted.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/fix/bound-append-memory

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: eaa8537b46

ℹ️ 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".

Comment thread polylogue/sources/live/append_ingest.py Outdated
A non-empty metadata replay plan can describe an older accepted chain while a
newer full snapshot remains asserted. Reclassify whenever the current append
is absent and defer it if proof still cannot include it, so the watcher never
advances a cursor over unindexed bytes.

The append cohort harness covers both the empty and non-empty omitted-current
recovery states.

Ref polylogue-cnaj

Co-Authored-By: Claude <noreply@anthropic.com>
@Sinity

Sinity commented Jul 13, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@Sinity
Sinity merged commit 2b0221a into master Jul 13, 2026
2 checks passed
@Sinity
Sinity deleted the feature/fix/bound-append-memory branch July 13, 2026 16:38

@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: 01f4c44142

ℹ️ 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".

Comment on lines +176 to +177
if raw_id not in replay_plan.accepted_raw_ids:
replay_plan = archive.classify_raw_revision_cohort(logical_source_key)

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 Reclassify when quarantined append siblings remain

When a previous append for this same predecessor was deferred/quarantined, raw_revision_replay_plan() ignores it because only byte-proven children participate. With this new guard, a later append that has the same byte-contiguous parent can be accepted as soon as its own raw_id appears in the metadata plan, so the classifier is skipped; but classify_raw_revision_cohort() is the path that promotes quarantined contiguous appends via _promote_contiguous_append_evidence, after which sibling tails would be marked ambiguous instead of selecting one by arrival order. In a recovery/legacy cohort with a quarantined append tail, this can advance the cursor/index to the wrong branch.

Useful? React with 👍 / 👎.

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