test(ai-edition): share one base document in the addZoom invariant - #441
Conversation
`createEmptyDocument` stamps `createdAt`/`updatedAt` from the wall clock, and
this test called `fixtureDocument()` twice — so the two runs it compares started
from two different documents, and the assertion held only while both calls
landed in the same millisecond. On a loaded CI runner that is a coin flip; it
just failed a PR that touches none of this code, with a one-digit diff:
"createdAt":"…T15:37:58.275Z" vs "createdAt":"…T15:37:58.276Z"
Handing both runs the SAME document is also the invariant the test means to
state: one zoom, written by a runtime that can read telemetry and one that
cannot, must come out identical. Comparing outputs of two different inputs was
never that claim. The pattern is already used elsewhere in this file.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 4 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe ChangesTelemetry consistency validation
Estimated code review effort: 1 (Trivial) | ~3 minutes Merge Risk: ⚪ Minimal · up to This localized test-only change makes the invariant deterministic by comparing executions against the same base document; no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
1 participant
Summary
mainhas a flaky test. It just failed CI on an unrelated PR (#440, which touches onlyworkbench/) with a one-digit diff:addZoom answers for the focus it was given > reports; it does not placecallsfixtureDocument()twice, andcreateEmptyDocumentstampscreatedAt/updatedAtfrom the wall clock. So the two runs it compares start from two different documents, andwithoutIdsnormalises zoom ids but not timestamps. The assertion holds only while both calls land in the same millisecond — a coin flip on a loaded runner.Handing both runs the same document is also the invariant the test means to state: one zoom, written by a runtime that can read telemetry and one that cannot, must come out identical. Comparing the outputs of two different inputs was never that claim.
executeAgentToolreturns a new document rather than mutating its argument, and the shared-base pattern is already used elsewhere in this file.Arrived with #431; not specific to anything in flight.
Related issue
Refs #431
Type of change
Release impact
Desktop impact
Testing
npx vitest --run electron/ai-edition/agent-tools.test.ts— 82 passed, run 8 times consecutively, green every time.npx tsc -p tsconfig.test.json --noEmit— clean.npx biome check— clean.The flake is timing-dependent, so a green run does not prove much on its own; the argument is the removed dependency on two wall-clock reads landing in the same millisecond, not the run count.
Summary by CodeRabbit