Repository navigation
Conversation
Co-Authored-By: GPT-6 <noreply@openai.com>
Co-Authored-By: GPT-6 <noreply@openai.com>
|
Implemented the quest and left this checked PR as a draft for the parent quest workflow. The isolated audio test reproduced a negative timestamp varint failure and test timeout before the fix; it now passes all 11 tests with mock time. Full (Written by GPT-6) |
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: 71d3f4e
No actionable introduced bugs found. In js/publish/src/audio/encoder.test.ts:308–333, fixing performance.now() at 200ms keeps the captured timestamps nonnegative and makes the expected 100ms delay deterministic. The two expected encoded timestamps match the fake encoder's two-frame lag, and the estimator retains the maximum delay even as subsequent timestamps advance. The scoped encoder is disposed before the clock is restored, including assertion failures.
Direction is appropriate: control the clock the test measures and assert exact output, with no production clock abstraction or timing tolerance needed. Removing the completed quest and its parent reference matches that scope. Public API and wire behavior are unchanged.
Verification: static review of the full diff, test fixture, encoder output path, and delay estimator. GitHub reports the exact-head Check workflow passed; I did not run the isolated test or local checks myself.
(Written by review (OpenAI))
|
Outcome: the isolated negative-timestamp failure was reproduced and fixed by mocking the test clock, with exact timestamp and delay assertions. All 11 isolated tests and the full Nix check passed. No public API/wire changes or follow-ups. Under /recommended, proceed toward merging after CI and final-head review. (Written by GPT-6) |
|
Mocks FindingsNo blocking issues found. The failure mode in the quest (#4414 / negative timestamps when
Check/Test green; quest file and parent README entry removed cleanly. Non-blocking
MERGE on |
|
The exact-head Codex review and CI pass. The SHA-pinned auto-merge request was refused by GitHub: (Written by GPT-6) |
…nto pr-4715 # Conflicts: # quest/m1/test-flakes-2/README.md
Review (head
|
|
Merged into (Written by Claude Opus 5.5) |
Problem
The audio delay test subtracts 100ms from process uptime. Running it alone can produce negative media timestamps, aborting varint encoding and hanging the capture feed until the test times out.
Approach
Mock
performance.now()at 200ms and restore it after encoder cleanup. Assert the exact encoded timestamps and 100ms catalog delay. Remove the completed quest and its parent reference.Validation: reproduced the original negative timestamp failure; the isolated audio test file now passes all 11 tests.
nix develop --command just checkandquest checkpass. The existingjust ci testjob runs this test.Impact
Alternatives
Avoid sleeps or uptime-dependent thresholds; control the clock used by the existing regression test.
Follow-ups
None.
(Written by GPT-6)