Make enabling a reaction a go-live, not a replay of the whole record - #692
Merged
Conversation
A reaction's bookmark row was created at position zero, so the first boot after registering one walked it from the beginning of history. For a projection that is required: its table is a fold of the entire stream. For a reaction it is the opposite of what anyone intends, because a reaction has side effects in the world, and replaying history means re-performing it. On the 2-BM pilot record that is not theoretical. Turning the LLM subscribers on for the first time would have fired them at 674 already-finished Runs before either saw a new one, bought roughly 1300 model calls in a burst, and appended every verdict to a record that has no edit path. We had just finished measuring that three of four versions of the debrief guidance produced confident wrong verdicts, so the first act of the feature would have been to make a batch of them permanent. Reactions now seed at pg_snapshot_xmin(pg_current_snapshot()), the same watermark the record exporter takes, and not at max(position): positions come from a sequence that advances on rollback and does not order commits, so a cursor set past an in-flight transaction would skip its events forever once it committed. Seeding at xmin errs the other way and may deliver a few events that were mid-commit, which is safe because reactions are already at-least-once and idempotent by construction. Projections and Reactions are structurally identical Protocols, so the discriminator is the registered name: `proj_*` is a projection, already pinned by test_projection_table_match and test_projection_table_bc_prefix. Replaying on purpose is now `reactions_at_head=False` rather than the default. Six existing tests appended events and then seeded, which is exactly the ordering this change turns into a skip; they ask for the replay explicitly now instead of relying on it. That parameter is also the seam the operator backfill command will use when it lands, which is a separate slice: this one exists to stop an accidental replay, and shipping a deliberate one beside it would defeat the point. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A reaction's bookmark was created at position zero, so the first boot
after registering one walked it from the beginning of history. A
projection must do that: its table is a fold of the whole stream. A
reaction must not, because it has side effects in the world, and
replaying history means re-performing it.
On the 2-BM pilot record this was one restart away. Turning the LLM
subscribers on would have fired them at 674 already-finished Runs
before either saw a new one, bought roughly 1300 model calls at once,
and appended every verdict to a record with no edit path.
What changed
Reactions seed at
pg_snapshot_xmin(pg_current_snapshot()), not atmax(position). Positions come from a sequence that advances onrollback and does not order commits, so a cursor set past an in-flight
transaction would skip its events forever once it committed. Seeding at
xmin errs the other way and may deliver a few events that were
mid-commit, which is safe: reactions are already at-least-once and
idempotent by construction.
Projections and Reactions are structurally identical Protocols, so the
discriminator is the registered name (
proj_*), already pinned by twoarchitecture tests.
Deliberate replay is now
reactions_at_head=False. Six existing testsappended events and then seeded, the exact ordering this turns into a
skip; they now ask for the replay explicitly. That parameter is the seam
the operator backfill command will use, which is a separate slice.
Verification
The load-bearing test was checked against its own absence: with the fix
reverted,
test_new_reaction_bookmark_skips_events_that_predate_itfails and the other three still pass, so it is not green by
construction.
Gates: 4663 integration + contract passed, 31181 architecture passed.
🤖 Generated with Claude Code