Repository navigation
Conversation
…g after its fixture aged out The saved-usage window test wrote an August 1, 2026 transcript but read it with the real clock. Since 2026-11-01 that fixture is older than the 92-day retention window, so deleted-file usage is correctly pruned and the test fails on every PR. Pin "now" near the fixture, matching upstream. (cherry picked from commit 1280ccd0d2a8525c840999fe633cff20ebdeafd5)
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 40 minutes. View limit details
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. |
Cross-model review blocked: authenticationImplementation (PR footer): Claude Opus 5.5 via Claude Code The review did not finish, so no verdict was produced. Reason: ReviewAuthenticationError: codex authentication was rejected during review on the review service. No review verdict was produced. Sign in to the reviewer on the review service host, then comment Automated cross-model review, updated 2026-11-02T18:25:02+00:00. |
dbalders
left a comment
There was a problem hiding this comment.
TritonAI Review
Model: api-glm-5.3 · Reasoning: high.
No actionable findings in the inspected changes.
Reviewed a57b752e1cd877acad2d595426cc50dcf1a5b1ff against 4429dcc73fb933cb1c76ecf70997e144f67ce396.
Reviewed 1 file(s) together as one PR.
Changes
PR: This PR is a minimal, test-only change that pins the clock in one UsageService test (reports saved usage of a removed directory only for windows it reaches). ATestClock.setTimeto 2026-08-15 is added aftersetup, and the test's pipe gainsEffect.provide(TestClock.layer())so the service's retention/pruning logic sees the pinned
CI snapshot
Check runs:
- Sync PR size label definitions: status=completed conclusion=skipped app=
- Label PR size: status=completed conclusion=success app=
- Windows Regression Tests: status=in_progress conclusion=none app=
- Macroscope - Effect Service Conventions: status=completed conclusion=skipped app=
- Macroscope - Correctness Check: status=completed conclusion=skipped app=
- CodeRabbit: status=completed conclusion=neutral app=
- Prepare PR size config: status=completed conclusion=success app=
- Test: status=queued conclusion=none app=
- Test Server 4: status=queued conclusion=none app=
- Test Server 3: status=queued conclusion=none app=
- Test Server 1: status=in_progress conclusion=none app=
- Test Server 2: status=in_progress conclusion=none app=
- Test Server 5: status=queued conclusion=none app=
- Preflight: status=queued conclusion=none app=
- Mobile Native Changes: status=queued conclusion=none app=
- Release Smoke: status=in_progress conclusion=none app=
- Rust: status=completed conclusion=success app=
- Native fingerprint diff: status=completed conclusion=success app=
- Windows Test Scope: status=completed conclusion=success app=
CI is reported separately. This code review does not claim to have run tests.
Comment /triton-review to request a fresh review. Findings are advisory; GitHub branch protection remains authoritative.
|
/cross-review |
Thread transfer impact✅ Thread transfer remains within every enforced ceiling.
Baseline: Scenario and decoded snapshot size10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed. |
|
Note 🤖 Claude Opus 5.5 responding on behalf of David Closing as superseded: #474 (squash 4429dcc) already put a clock fix for this test on main ( |
Summary
Pins the clock in one server test,
reports saved usage of a removed directory only for windows it reaches. This is the exact upstream fix (pingdotgg/t3code#28603,1280ccd0d2). Test-only; one file, 3 lines added and 1 changed.Why
The test writes a transcript dated 2026-08-01, deletes its folder, and checks that the saved usage still appears for a date range that reaches it. It read with the real clock. Saved usage for deleted files is kept for 92 days, so since 2026-11-01 10:00 UTC the fixture has been past retention. The service correctly drops it, and the test fails Test Server 1 on any PR based on current main (for example #473). Product behavior is unchanged; the test only ever passed because "now" happened to be close to its fixture date.
With "now" pinned to 2026-08-15, the test keeps checking both behaviors:
missing.Validation
vp check(format and lint),vp run typecheckviavp run check:changed: all passed, including knip and graphqlapps/server/src/usage/UsageService.test.ts40/40. Before this change, the same test failed ondd0d5acfd1withexpected [] to deeply equal [ { day: '2026-08-01', … } ]at line 2470.git diff --checkRisk and Recovery
None for users; only a test changes. Revert to restore the previous test.
Release and Deployment Impact
None.
Upstream Alignment
Byte-identical to upstream
1280ccd0d2(cherry-picked with-x), so the next upstream sync applies cleanly. Upstream's later8b4de61b8badds a redundant secondsetTimebeforesetup; the sync will bring that in.#474 currently carries its own version of this fix (
it.effectpinned to 2026-08-02). Once this lands, #474 can drop that edit and stay UI-only.Checklist
Implementation: Claude Opus 5.5 via Claude Code