Repository navigation
Conversation
A Topic window whose publication lost the head CAS to a concurrent replace or Dream returned HEAD_CONFLICT without recording a cause, yet still kept the attempt it had begun. Three such conflicts left attempts at 3 with no last error, and the next attempt blocked the Scope with window_attempt_limit. The Worker now records head_conflict as the attempt's last error. The attempt ceiling and the next attempt's counter treat that attempt as not spent, so the existing 0.1 s conflict deferral rereads the head and recomputes (RFC 1614 section 5.3). Provider reservations are never refunded: each recomputation spends from the frontier's 512-request and 64M-token ceilings, which stop a head that keeps moving with window_provider_budget_exceeded. The supervisor's attempt.completed log now names cursor_conflict or head_conflict in error_code. No schema, migration or row encoding changes; the persistence review is updated for the reinterpreted budget column.
There was a problem hiding this comment.
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.
Only the publisher's head CAS hands an attempt back. A RevisionConflictError from any other step keeps the previous mapping: HEAD_CONFLICT without a recorded cause, so that attempt still counts. The catch sits outside the commit tracing stage, so the stage still observes the conflict. A head that never settles normally crosses the provider ceiling inside an attempt, so the block shows window_provider_budget_exceeded as both reason and last error, with attempts=1. Test that case instead of one where the charges align exactly with the ceiling, and correct the Topic Memory docs.
There was a problem hiding this comment.
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.
hidb4ai
left a comment
hidb4ai
left a comment
There was a problem hiding this comment.
Reviewed commit 9626a4c. No blocking correctness findings. Returning an attempt after a rolled-back publication head conflict matches RFC 1614: the next attempt rereads the head, real failures still consume attempts, and provider reservations remain charged. No new database tables or interfaces; the existing last_error_code field can now contain head_conflict.
Validation covered SQLite, HTTP replacements racing a spawned Worker, and reopening the database after a conflict on the last available attempt. A child-startup timeout in the broader suite passed on both the base and this head using the same explicit source-path invocation. Native database backends and real model providers were not exercised.
Before merging, resolve the release/upgrade-support.json conflict with current master and clear the macOS Intel Desktop test timeout. That CI failure is in unchanged frontend code; I found no evidence connecting it to this patch. This approval applies to the reviewed commit, not to an unreviewed conflict resolution.
Resolve the release/upgrade-support.json conflict: keep master's schema review rationale, append this branch's head-conflict sentence, and recompute persistence_sha256 with scripts/upgrade_release_gate.py.
There was a problem hiding this comment.
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.
|
@hidb4ai Thanks for the review. I merged current master into this branch (merge commit 3b2dd04, no rebase). The only conflict was the persistence digest in Checks run locally after the merge, all on SQLite:
The earlier red check was desktop-validation on macos-15-intel: |
Which issue or RFC does this PR close?
No tracking issue. This implements the Worker side of RFC 1614 §5.3: if a manual replacement wins the head CAS, the stale Worker commit is rejected and "its existing reread/recompute behavior" recovers it.
Rationale for this change
When a concurrent replace or Dream moves a Topic head between a Topic window's read and its publication, the publisher raises
RevisionConflictErrorand the Worker returnshead_conflict. The supervisor defers the retry for 0.1 s and does not count it as a failure. However,TopicMemoryWorkBudget.begin()had already charged the attempt, and no cause was recorded. Reproduced on923e3d4cethroughTopicMemoryProcessor.processwith a concurrent revision published during the global stage:After three concurrent edits, the Scope stays blocked until an operator retry, and nothing records why. The new e2e reproduces the same block with real HTTP
PUTreplacements and a spawned Topic Worker.What changes are included in this PR?
topic_memory_processing.py): when the publisher's head CAS fails, the attempt recordshead_conflictas itslast_error_codeon its own budget row, then returnsHEAD_CONFLICT. The publish transaction has already rolled back, so nothing is written. Only the publish call is covered. ARevisionConflictErrorfrom any other step keeps the previous mapping:HEAD_CONFLICTwithout a recorded cause, so that attempt still counts.topic_memory_budget.py): an attempt whose last error ishead_conflictis not spent.exhausted_reasonand the nextbegin()both use the spent count, so the existing conflict deferral starts a fresh attempt that rereads the head and recomputes.exhausted_reasonis also used by block observation, explicit retry, the selector guard and the status reader in feat(processing): add read-only artifact processing status inspection (part of #2130) #2568, so they all stay consistent.window_provider_budget_exceeded. The ceiling is normally crossed partway through an attempt, so that code is also the block'slast_error_code. A lowattemptscount, typically 1, shows that recomputations after head conflicts used the budget rather than failed attempts. Each conflict also appears in the supervisor log, described below. I chose this over a dedicated conflict counter because the frontier's model cost stays capped by the ceiling it already had, and a counter would need a migration on every backend. The trade-off: before this change, conflicts could cost at most three attempts' worth of model calls. Now they can use up the full frontier ceiling, at the rate other writers move the head.artifact_processing.attempt.completedfor a control conflict now setserror_codetocursor_conflictorhead_conflict. Before, both appeared only asoutcome=conflict. This uses an existing field, so no allowlist change.window_attempt_limitdescription, the attempt-completed event, the Topic Memory workflow's processing section, and the processing-failure matrix.release/upgrade-support.jsonhas the new digest and a rationale sentence. No table, column, constraint, migration resource, checksum or row encoding changes. Existing rows never containhead_conflict, so their counters and blocks keep their meaning.Self-review (code-review skill at high effort, plus
REVIEW.md) and an independent review found no blocking defects. Changes that came out of them:Notes I left unchanged:
attemptsshow the raw column, so a handed-back attempt is still counted there.head_conflictbut before returning is treated as a conflict by the budget and as a failure (with backoff) by the in-memory supervisor state.error_codemakes the logging filter deriveerror_category=conflicton those events. The shipped dashboards count onlyartifact_processing.failed, so no panel changes.head_conflictitself fails, for example on a transient database error, the conflict becomes an ordinary counted Worker failure with backoff. This is conservative and still bounded.Are there any user-facing changes?
Yes, in behavior:
last_error_codecan now behead_conflict.error_code.No API, schema or persisted-format changes. A binary without this change would count an already recorded head-conflict attempt as spent, which can only stop that frontier sooner.
How was this change tested?
tests/builtin/runtime/test_topic_memory_head_conflict.py:attempts=1andlast_error_code=head_conflict, and the window then publishes on the moved head with the right lineage.window_provider_budget_exceededwithattempts=1.head_conflict.tests/e2e/test_topic_memory_head_conflict.py:PUTwhile answering the global stage, four times.head_conflictattempts, then the Worker revision is published, with no block.923e3d4ceand pass with this change.make check: pass.make unit-test(on the first commit): 8733 passed. 12 failed, all already failing on unmodified master: 8 intest_jev_rerank_comparison, 4 intest_ci_installation_roles.AI usage statement
Investigation, implementation, tests and this description were produced with Claude Code (Claude Opus 5.5), then reviewed and run locally by the author.