Repository navigation
ScopeDescendants asserts the shape, not an exact emission count (#946) - #955
Merged
Merged
Conversation
Fixes #946. ScopeDescendants_NotifiesOnAllDescendantChanges waited for >= 3 distinct Added paths and then asserted HaveCount(3) EXACTLY. `scope:descendants` matches anything created under the root, so one extra descendant landing inside that window — a satellite the framework writes for the new nodes, which is timing-dependent under load — turned a correct system into a red build. That made it the top flake of 2026-08-08: four unrelated PRs (#927, #940, #942, #947), always shard 1, always passing locally. AGENTS.md already forbids exactly this ("Never assert 'exactly N change events' on a stream backed by a change feed that can race the initial-snapshot path — filter on the emission shape"). The test now asserts what it actually means: each of the three created descendants emitted an Added change. Extra descendants are explicitly allowed, because the scope legitimately matches them. Left alone deliberately: the sibling assertions in this class that count emissions to prove a NEGATIVE ("sibling create must not emit for ancestors scope") — a bound is the point there, and none of them has flaked. Changing passing tests without evidence is its own risk; noted here so the pattern is visible if one of them starts. ObservableQueryIntegrationTests 9/9. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the ScopeDescendants_NotifiesOnAllDescendantChanges integration test to assert the presence of the three expected descendant Added paths rather than asserting an exact emission/item count, eliminating a load-dependent CI flake while preserving the intended liveness contract of scope:descendants.
Changes:
- Replace exact-count assertions with set-based containment checks for the three explicitly created descendant paths.
- Make the wait condition in the test predicate depend on “all expected paths observed” instead of “>= N emissions then count == N”.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
This was referenced Aug 8, 2026
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
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.
Fixes #946.
The flake, and why it was the pipeline's top blocker today
ScopeDescendants_NotifiesOnAllDescendantChangeswaited for>= 3distinctAddedpaths, then assertedHaveCount(3)exactly.scope:descendantsmatches anything created under the root — so one extra descendant landing inside that window (a satellite the framework writes for the new nodes, timing-dependent under load) turns a correct system into a red build.It hit four unrelated PRs today — #927, #940, #942, #947 — always shard 1, always passing locally. Each occurrence blocked a green PR and burned a CI cycle.
Why this is the test's bug, not the product's
AGENTS.md already forbids this exact shape:
The test now asserts what it means: each of the three created descendants emitted an
Addedchange. Extra descendants are explicitly allowed, because the scope legitimately matches them.Deliberately not changed
Sibling assertions in the same class count emissions to prove a negative ("sibling create must not emit for ancestors scope") — there a bound is the assertion, and none of them has flaked. Changing passing tests without evidence is its own risk; the commit notes the pattern so it's visible if one of them starts.
ObservableQueryIntegrationTests9/9 locally. No What's New — test-only.🤖 Generated with Claude Code