Skip to content

fix(topic-memory): keep the healthy prefix before a failing or oversized look-ahead - #2530

Open
Fengzdadi wants to merge 2 commits into
oceanbase:masterfrom
Fengzdadi:fix/topic-window-healthy-prefix
Open

Fengzdadi wants to merge 2 commits into
oceanbase:masterfrom
Fengzdadi:fix/topic-window-healthy-prefix

Conversation

@Fengzdadi

Copy link
Copy Markdown
Contributor

Which issue or RFC does this PR close?

Implements the Window selection rules of RFC 1417 ("Window selection and context budgets", "Cumulative work and evidence ceilings"). No issue tracks this defect; it came out of a review of the Topic Memory processing path.

Rationale for this change

TopicMemoryWindowSelector.select() reads up to 100 Sources after the Cursor and projects all of them before it chooses a Window. If that projection raised TopicMemoryGenerationError, it returned a single-Source Window at the frontier. Two situations raise:

  • One Source fails on its own (source_complexity_limit: over 4 MiB characters, 65,536 nodes or depth 32; or unsupported_evidence). Every healthy Source before it is then processed alone. With 10 small healthy Sources followed by one Source over 4 MiB, the frontier walks [1], [2], …, [10], [11]. Without the bad Source, the same 10 share [1..10].
  • The look-ahead exceeds the 4 MiB Window character ceiling, with no bad Source at all. The ceiling belongs to the Window, but it was applied to the whole 100-Source look-ahead. With 60 healthy Sources of about 80k characters each and the default 125,000-token context window, the frontier walks 9 singleton Windows [1]…[9] before the 4-Source Windows the token budget allows ([10..13], [14..17], …). A backlog that keeps about 52 or more Sources of that size pending produces only singleton Windows.

Each Window runs the full pipeline: Probe, the global evolver or planner plus evolvers, the reconciler, embeddings and an atomic publish. Singleton Windows therefore multiply model calls and rewrite the same Topics once per Source. Every select() call also read and projected the whole look-ahead again.

What changes are included in this PR?

  • topic_memory_processing.py, TopicMemoryWindowSelector.select():
    • Projects eligible Sources in Journal order and ends the Window just before the first Source that fails projection on its own, would push the cumulative canonical characters past 4 MiB, or would push the Probe request past the input limit.
    • Returns a single-Source Window only when that Source directly follows the Cursor. The Worker then handles it exactly as before: source_complexity_limit is recorded as a durable rejection at the frontier, and the tail is never skipped.
    • A first eligible Source over the token budget still forms its own Window. Lineage-only Sources stay inside the contiguous Journal Window, and source_window_changed and the work-availability check are unchanged.
    • Sources after the stopping point are no longer projected.
    • The Worker's _project_window is unchanged. Every Window the selector returns stays within the same character ceiling, so the Worker never rejects a selected multi-Source Window for exceeding it.
  • Tests (tests/builtin/runtime/test_topic_memory_work_budget.py), through select() and TopicMemoryProcessor.process():
    • test_failing_source_ends_the_window_after_the_healthy_prefix: three healthy Sources and a lineage-only Source share one Window. The oversized Source after them is then selected alone at the frontier and durably rejected without provider requests, and it blocks later selection.
    • test_large_healthy_backlog_fills_each_window_to_its_budget: two backlogs larger than the Window character ceiling, each Window published by the processor.
      • In the first, the Probe budget binds first: 60 Sources, Windows of 3.
      • In the second, the character ceiling binds first: four Sources of half the ceiling, so each Window holds exactly 4,194,304 canonical characters. Changing either the selector's or the Worker's comparison to >= fails this case.
    • All three cases fail on upstream/master. In the first test, select() returns 1 instead of 4. The backlogs start with 8 and 2 singleton Windows.
  • Docs: RFC 1417 in both locales now lists the character ceiling and the projection-failure stop among the Window selection limits, and describes when the selector isolates a Source.

Are there any user-facing changes?

No API, configuration or persisted-format change. Topic processing of a backlog that contains a failing Source or exceeds 4 MiB now runs fewer, larger Windows, so the per-Window pipeline (model calls and Topic revisions) runs fewer times. Rejection of a failing Source at the frontier is unchanged.

How was this change tested?

  • Window sequences from a select() walk that mirrors the Scope loop (default 125,000-token context window, character token estimator):

    Scenario upstream/master This PR
    10 healthy + 1 Source over 4 MiB [1], [2], …, [10], [11] [1..10], [11]
    60 healthy Sources of ~80k characters [1]…[9], then [10..13], …, [58..60] (22 Windows) [1..4], [5..8], …, [57..60] (15 Windows)
  • uv run pytest on the Topic Memory selector, processor, scope, recovery, fragment-reduction, security and e2e processing tests.

  • Additional local checks, not committed:

    • lineage-only Sources before a failing Source form their own Window;
    • a first Source over the token budget keeps the lineage-only Source after it;
    • a two-Source Window of exactly 4 MiB is accepted by both the selector and the Worker, while 4 MiB + 1 splits;
    • a select() over ten 150k-character Sources reads only the three Sources up to the stopping point.
  • make check

  • make docs-test

  • make unit-test: 8,139 passed, 417 skipped, 6 failed, all unrelated to this change. tests/test_ci_installation_roles.py::test_batch_keeps_all_roles_isolated_and_reports_every_failure (4 cases) fails the same way on unmodified upstream/master in this environment. test_processing_scheduler.py::test_overflow_flush_cannot_advance_retry_and_cached_failures_cannot_extend_gate and test_handoff_report.py::test_handoff_report_statement_count_does_not_grow_with_scope_count failed only under the parallel run and pass on rerun.

Not validated: no run against a real model provider or a real Server workload, and no OceanBase or seekdb backend run. The change does not touch persistence. The model-call savings are inferred from the Window counts above, not measured.

Alternatives considered

  • Keep projecting the whole look-ahead and pick the prefix afterwards. Rejected: it still serializes up to 100 Sources on every select(), including Sources past the stopping point.
  • Share one projection helper between the selector and the Worker. Rejected: the two need different failure semantics. The selector stops before the failing Source, while the Worker must reject the Window it was given so that the rejection is recorded durably. Instead, the character-ceiling test fills Windows to exactly the ceiling and runs each one through the Worker, so moving either comparison fails it.
  • Skip the Probe estimate for the first Source, since it can never end the Window. Not done here: the token path keeps the prior estimation pattern unchanged.

AI usage statement

Claude Code, Claude Opus 5.5

…zed look-ahead

TopicMemoryWindowSelector projected the whole 100-Source look-ahead and fell
back to a single-Source Window whenever that projection failed. A Source that
failed on its own, or a look-ahead past the 4 MiB Window character ceiling,
therefore split every healthy Source before it into its own Window, each
running the full model pipeline.

Project Sources in Journal order and end the Window just before the first
Source that fails projection, would exceed the character ceiling, or would
exceed the Probe input budget. Only a failing Source at the frontier forms a
single-Source Window, so the Worker still records the durable rejection there.
Sources past the stopping point are no longer projected.
Size the character-ceiling backlog so two Sources fill a Window to exactly
4,194,304 canonical characters. Moving either the selector's or the Worker's
ceiling comparison now fails the test. Also assert that the frontier
rejection spends no provider requests.
Copilot AI balanced review requested due to automatic review settings November 7, 2026 04:59

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

@Fengzdadi

Copy link
Copy Markdown
Contributor Author

Both failing checks at 42bddbc come from CI timing, not from this change:

  • worker-observability (test_builtin_spawned_worker_exports_model_and_execution_spans_to_otlp[True-topic-memory], assert failed["last_error_worker_id"]) is the stats race that ci(oceanbase): bound startup and balance migration workers #2515 and feat(desktop): package and provision independent Ops resources #2503 already fix. The test reads /v1/stats as soon as the worker spans are exported, while for topic-memory the supervisor's _reap first awaits record_topic_memory_work_failure and only then records last_error_worker_id. Unmodified master (b8b1890) fails the same case once a 0.5 s delay is added to that hook.
  • harness-validation / Recall effort (OceanBase) hit the 600 s limit of "Maintain the registered Server chain on real OceanBase without skips" during its fifth migration test, test_mysql_managed_identity_initialization_requires_no_ddl[oceanbase]. The first four tests took 504 s here versus 456 s in a passing run of the same step earlier today (run 42329707512), which finished the whole step in 585 s. The same step also timed out on run 42254279764. These are OceanBase migration tests, which this PR does not touch.

Both jobs need a re-run once the workflow finishes.

@hidb4ai hidb4ai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed 42bddbc. I found no correctness issue in the changed selector and no new database tables or public interfaces.

Approval is pending a green CI result: worker-observability and harness-validation / Recall effort (OceanBase) are still failing. The logs show a missing worker-error stats field and a 600-second migration-test timeout; I have not established either as a regression from this patch. Please rerun or fix these jobs before merging.

@hidb4ai hidb4ai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@AlexStocks AlexStocks added the 1.6.0 v1.6.0 release tracking label Nov 9, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1.6.0 v1.6.0 release tracking

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants