Repository navigation
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
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 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f45d5bd02
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ( | ||
| cassette.wireApi === "responses" && | ||
| cassette.rounds.some( |
There was a problem hiding this comment.
When a Chat Completions provider returns a final 429/500 after the repetition retries are exhausted, the recorder forwards that status but omits it from the cassette because byte/status evidence is stored only for Responses. The run is classified as TRANSPORT, yet this validation checks failures only when wireApi === "responses"; neither recorded checks nor later verification rejects the report's transport failure. Consequently an approved Chat measurement can encode a provider outage as model-quality failures and feed those failures into a bound baseline. Preserve and validate Chat status evidence, or reject candidates containing infrastructure failure classes.
Useful? React with 👍 / 👎.
| invocationTimeoutMs: options.timeoutMs, | ||
| }, | ||
| }); | ||
| const liveBudget = createInvocationBudget(liveTransport.limits); |
There was a problem hiding this comment.
For a run containing multiple repetitions or scenarios whose aggregate duration exceeds --timeout (300 seconds by default), this budget is created once before the scenario loop and reused by every recorder. Since createInvocationBudget measures remainingMs() from this single creation time, later otherwise-valid turns are rejected as soon as the whole command crosses the timeout; previously --timeout guarded each turn, and the documented default pack is expected to take longer than five minutes. Keep the global request counter if required, but do not reuse its wall-clock deadline across independent turns.
Useful? React with 👍 / 👎.
| behaviour: newcombeDelta( | ||
| { | ||
| k: left.manifest.results.axes.behaviour.passed, | ||
| n: left.manifest.results.axes.behaviour.decided, | ||
| }, |
There was a problem hiding this comment.
When scenarios contain multiple weighted checks, these inputs treat every decided check as an independent, equally weighted Bernoulli observation. Checks from the same model turn are correlated, so this inflates n from the number of repetitions to checks × repetitions, and it also lets several low-weight checks determine a direction opposite to the weighted axisScores displayed immediately above. The resulting interval and significant/direction fields can therefore claim a regression or improvement that the workbench's actual weighted metric does not show; derive the comparison from independent repetition-level outcomes while preserving the configured weights.
Useful? React with 👍 / 👎.
| if ( | ||
| report.fingerprint.systemSha256 !== manifest.fingerprints.systemSha256 || | ||
| report.fingerprint.toolsSha256 !== manifest.fingerprints.toolsSha256 || | ||
| report.fingerprint.model !== reportModel || | ||
| report.fingerprint.reps !== manifest.results.repetitions || | ||
| report.fingerprint.effectiveSourceSha256 !== manifest.source.effectiveSha256 |
There was a problem hiding this comment.
If an adopted manifest's promptSha256, wireSha256, or rubricSha256 is accidentally edited, verify still succeeds: manifest validation checks only that they look like SHA256 values, and this cross-check binds only system/tools/model/repetition/source fields. Those unchecked values are later trusted by assertComparable, so editing them can make genuinely different measurements appear compatible (or compatible measurements appear incompatible) without changing any hashed artifact. Recompute these identities from the scenario/cassettes where possible, or bind them to a verified artifact rather than accepting bare manifest claims.
Useful? React with 👍 / 👎.
| const requestHash = | ||
| wireApi === "responses" ? hashResponsesRequest(effectiveBody) : hashRequest(effectiveBody); | ||
| if ( | ||
| requestHash !== stored.requestHash || | ||
| (stored.inboundPath && stored.inboundPath !== inboundPath) |
There was a problem hiding this comment.
For a new transport-bound Chat cassette, changing only a tool description or parameter schema still produces the same hashRequest, because that legacy hash includes only tool names. This branch then considers the request fresh, and the new replay command can emit a receipt with fresh: true while serving responses recorded against a different tool contract. Keep the historical algorithm for legacy cassettes if compatibility is required, but version new Chat cassettes with a full normalized request hash that includes descriptions and parameters.
Useful? React with 👍 / 👎.
| const recorded = readJson(resolveArtifact(root, checksRefs[0].path)) as RecordedChecks; | ||
| if (!recorded.complete) fail("CANDIDATE_INCOMPLETE", "recorded checks are not complete"); | ||
| const recomputed = recomputeMeasurementCounts(recorded); |
There was a problem hiding this comment.
When a package contains a recorded-checks.json copied from another scenario, recomputeMeasurementCounts accepts any nonempty scenarioId, and verification never compares that value with manifest.scenario.id. If the accompanying manifest and report carry matching counts, verify and the baseline command accept those foreign outcomes as this scenario's evidence; only a separately requested replay would expose the mismatch. Require recorded.scenarioId === manifest.scenario.id and validate the recorded check roster against the current scenario before producing a bound baseline.
Useful? React with 👍 / 👎.
| const requestHash = | ||
| wireApi === "responses" ? hashResponsesRequest(effectiveBody) : hashRequest(effectiveBody); | ||
| if ( | ||
| requestHash !== stored.requestHash || | ||
| (stored.inboundPath && stored.inboundPath !== inboundPath) |
There was a problem hiding this comment.
When a Chat cassette is replayed after changing another request-level field such as temperature, model options, or tool schemas, this branch still uses the historical hash that covers message content and tool names only. The request can therefore differ materially while staleRounds remains empty, causing assertFresh() and the replay receipt to certify a response recorded under a different contract. Use a versioned full normalized-body hash for newly recorded transport-bound Chat cassettes while retaining the legacy hash only for old discriminator-less files.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 948f4529a9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| skip(); | ||
| return; | ||
| } | ||
| expect(() => captureSourceIdentity(root)).toThrow(SourceIdentityError); |
There was a problem hiding this comment.
On Linux and macOS, this assertion always fails: git ls-files --others --exclude-standard reports the directory symlink as src/linked, and isRelevantSourcePath discards that path because it has no supported source suffix, so captureSourceIdentity neither traverses it nor throws. The targeted provenance suite consequently fails with “expected function to throw an error, but it didn't”; construct a symlink at a path the capture logic actually considers relevant, or change the expectation to match the non-traversal behavior.
AGENTS.md reference: AGENTS.md:L78-L79
Useful? React with 👍 / 👎.
| toolsSha256: unknown(first?.toolsSha256), | ||
| wireSha256: sha256Canonical( | ||
| results.map((result) => ({ | ||
| rounds: result.run.wire.rounds, |
There was a problem hiding this comment.
When two otherwise comparable measurements take different numbers of model turns—for example, one model uses an additional read-tool round—this observed rounds value changes wireSha256. Because assertComparable requires equal wire fingerprints, wb:measurement compare then reports INCOMPATIBLE_IDENTITIES instead of comparing the quality results; round count is model output being measured, not an invariant request contract, so it should not participate in the comparability identity.
Useful? React with 👍 / 👎.
| import { createInvocationBudget, publicHeaderProfile, transportIdentity } from "../lib/transport"; | ||
| import { getScenario } from "../scenarios/registry"; | ||
|
|
||
| const ROOT = "workbench/.agent-evidence/responses-validation/vitest"; |
There was a problem hiding this comment.
Running the new Responses tests writes deterministic cassette files below this repository-relative directory, but no teardown removes it and .gitignore does not exclude it. For example, the passing responses-judge.wb.ts test leaves ?? workbench/.agent-evidence/ in git status, so routine targeted or full workbench runs dirty every developer's checkout and risk staging generated evidence; use a temporary directory with cleanup or explicitly ignore this output tree.
Useful? React with 👍 / 👎.
| if (roleRefs(manifest, "judge-cassette").length === 0) { | ||
| fail("CANDIDATE_INCOMPLETE", "judged measurement is missing its judge cassette"); |
There was a problem hiding this comment.
For a completed judged package containing two judge-cassette references, this check accepts both, assertCassettes validates both, and usage is recomputed across both, but assertTransportFingerprint binds only the first cassette. Thus verify and baseline creation can accept evidence that replayMeasurement later rejects because it requires exactly one judge cassette, while the extra cassette is not covered by the claimed transport identity; enforce cardinality here (and reject judge cassettes for unjudged measurements).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 735ed3dab0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "path": "workbench/lib/cassette.ts", | ||
| "kind": "text", | ||
| "sha256": "78e596227a288f222a3e8b63d6fec1835484544c0e7dd2cdf51d6c5188a15955" |
There was a problem hiding this comment.
In a clean checkout of this exact commit, the documented npm run wb:measurement -- replay --id local-pr15-scripted-control-20260913-v1 command exits with SOURCE_IDENTITY_MISMATCH. For example, this manifest records workbench/lib/cassette.ts as 78e596…, while the committed file hashes to 50437b…; several other newly changed workbench files differ too. Because replayMeasurement compares the aggregate effective source identity before starting replay, the committed artifact cannot serve as the advertised offline workflow control until its source manifest is regenerated from the final tree.
Useful? React with 👍 / 👎.
| "rounds": [ | ||
| { | ||
| "round": 0, | ||
| "requestHash": "a9f5459b3417be21", |
There was a problem hiding this comment.
Fresh evidence after the earlier Chat freshness issue is the newly committed control cassette itself: it declares a transport identity, so startReplay computes the new 64-character hashChatRequest, but all three stored requestHash values are still 16-character legacy hashes. Running the standalone replay CLI against this cassette marks rounds 0, 1, and 2 stale and exits nonzero, independently of the measurement's source-identity mismatch. Regenerate the cassette with the transport-bound hashing algorithm so the advertised offline replay can succeed.
Useful? React with 👍 / 👎.
| !reported || | ||
| reported.weight !== count.weight || | ||
| reported.passed !== count.passed || | ||
| reported.total !== count.total || | ||
| reported.indeterminate !== count.indeterminate |
There was a problem hiding this comment.
When a report's checks[].wilson, behaviour/dsl interval fields, gateScoreMean, or passRate is corrupted while its basic passed/total/indeterminate counts and axis rates remain unchanged, verifyMeasurementDirectory still accepts it because this comparison never validates those derived values. These are the confidence intervals and gate statistics readers use to interpret an experiment, so a verified package can display materially false conclusions; recompute and compare the derived report statistics from the recorded repetition outcomes rather than checking only the underlying counters.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d5101a1fb3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| results.map((result) => ({ | ||
| systemSha256: result.run.wire.systemSha256, | ||
| toolsSha256: result.run.wire.toolsSha256, | ||
| toolNames: result.run.wire.toolNames, | ||
| })), |
There was a problem hiding this comment.
When two otherwise identical measurements use different sample sizes, this array contains a different number of duplicate entries, so wireSha256 changes and assertComparable rejects the comparison as INCOMPATIBLE_IDENTITIES. Fresh evidence beyond the earlier round-count finding is the remaining results.map(...) wrapper: observed model-round counts were removed from each entry, but the requested repetition count is still encoded by the array length. Hash the request-contract fields once (after confirming they are consistent across repetitions) so runs such as --reps 3 and --reps 10 remain comparable.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b1466895e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export function measurementTransportSha256( | ||
| agent: TransportIdentity[], | ||
| judge: TransportIdentity | "no-judge", | ||
| ): string { | ||
| return sha256Bytes(JSON.stringify({ schema: 1, agent, judge })); |
There was a problem hiding this comment.
When otherwise identical transport-bound runs use different sample sizes, agent contains one identical transport profile per main cassette, so this hash changes solely because the array has 3 versus 10 entries; assertComparable then still rejects the measurements as INCOMPATIBLE_IDENTITIES. Fresh evidence after removing repetition count from the wire fingerprint is that the transport fingerprint independently retains the same sample-size dependency. Validate that the profiles agree, then hash the request contract once.
Useful? React with 👍 / 👎.
| for (;;) { | ||
| const endpoint = options.live | ||
| ? await startLiveEndpoint({ | ||
| scenario: options.scenario.id, | ||
| cassetteFile: options.live.record?.(rep), | ||
| transport, | ||
| budget, |
There was a problem hiding this comment.
When a live Chat repetition ends in TIMEOUT or TRANSPORT and this loop retries it, each attempt creates a fresh recorder but reuses the same main-cassette-rep-N.json path. Because every recorder starts with empty round/attempt arrays and later writes replace that path and its attempt receipt, a successful retry erases the preceding provider attempt; the adopted package can therefore undercount attempts and usage while losing the infrastructure evidence entirely. Keep one recorder history across retries or merge the prior evidence before replacing the cassette.
Useful? React with 👍 / 👎.
| if (declaredWireApi === "chat-completions") { | ||
| if (hasResponsesEvidence || hasResponsesSseEvents) { | ||
| throw new Error("Chat cassette contains Responses-only round evidence"); | ||
| } | ||
| return; |
There was a problem hiding this comment.
When a Chat provider alias resolves different rounds to different concrete models, the recorder retains only the first response's resolvedModel, and this early return skips checking the model reported by every later round. Verification consequently attributes a hybrid run entirely to the first model and may compare it as single-model evidence. Validate each Chat round's SSE model against the stored resolved model, as the Responses branch already does.
Useful? React with 👍 / 👎.
| assertComparable(baseline.identity, verified.manifest); | ||
| if (canonicalJson(baseline.counts) !== canonicalJson(verified.manifest.results)) { | ||
| fail("CHECK_COUNT_MISMATCH", "baseline k/n does not match its bound measurement"); |
There was a problem hiding this comment.
When a bound baseline's expectedFailures is edited, verifyBoundBaseline still succeeds because it checks only identity and counts, even though boundBaselineFromMeasurement derives that list from the verified check outcomes. The altered list can subsequently excuse arbitrary failing checks when consumed by the baseline ratchet, while appearing to remain bound to the measurement. Recompute and compare expectedFailures (and the other manifest-derived metadata) during verification.
Useful? React with 👍 / 👎.
| transport, | ||
| publicHeaders: env.publicHeaders, | ||
| budget: createInvocationBudget(transport.limits), |
There was a problem hiding this comment.
When wb:judge processes multiple judged scenarios, this budget is created inside the scenario loop, so --max-requests N resets to N for every scenario and the command can forward up to N times the number of selected scenarios. That defeats the request cap as a bound on a paid invocation, unlike the main run path's shared counter. Create one shared request budget for the judge command while refreshing only the per-turn deadline where appropriate.
Useful? React with 👍 / 👎.
78cc1eb to
e651390
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e651390dde
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (roster === null) roster = keys; | ||
| else if (canonicalJson(roster) !== canonicalJson(keys)) { | ||
| fail("CHECK_COUNT_MISMATCH", `repetition ${repetition.rep} changed the check denominator`); | ||
| } |
There was a problem hiding this comment.
When every repetition consistently omits a failing check or substitutes another ID/weight, this comparison only proves that the repetitions agree with each other; it never compares the roster with getScenario(...) plus the structural checks. Fresh evidence after adding the scenarioId equality check is workbench/l0/measurements.wb.ts:70-95, whose target-right-clip fixture is accepted with only the unrelated beh.remove and dsl.remove checks. Consequently verify and baseline creation can certify inflated results without replaying them; require the recorded IDs, axes, and weights to exactly match the registered scenario roster.
Useful? React with 👍 / 👎.
| for (let rep = 0; rep < options.reps; rep += 1) { | ||
| budget?.refreshDeadline(); | ||
| const canonicalFile = options.live?.record?.(rep); | ||
| const attemptLists: CassetteAttempt[][] = []; | ||
| let attempt = 0; | ||
| for (;;) { |
There was a problem hiding this comment.
When a Chat attempt reaches --timeout and is classified as TIMEOUT, the retry loop starts a full new turn but reuses the deadline refreshed only before the outer repetition loop. Fresh evidence after adding refreshDeadline() is its placement outside the retry loop: the retried turn's first proxied request immediately encounters the expired invocation deadline and returns 429, so timeout retries cannot recover. Refresh the wall-clock deadline for each attempt while preserving the shared request counter.
Useful? React with 👍 / 👎.
| "path": "package-lock.json", | ||
| "kind": "text", | ||
| "sha256": "84fb90e8ba756f85c391e875c0aa026d58a66cf44ff84355e00004f9e3393baa" |
There was a problem hiding this comment.
The advertised offline control still cannot replay from a clean checkout of this commit. Fresh evidence after the regeneration is that the reviewed package-lock.json canonicalizes to ff8c0448…, while this effective-source entry records 84fb90e8…; therefore npm run wb:measurement -- replay --id local-pr15-scripted-control-20260913-v1 exits with SOURCE_IDENTITY_MISMATCH before serving the cassette. Regenerate the package after restoring the committed lockfile bytes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 46cb59cd9a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| manifest.input.fixtureSha256 !== refs[0].sha256 || | ||
| manifest.input.documentSha256 !== fixture.documentSha256 || | ||
| manifest.input.telemetrySha256 !== fixture.telemetrySha256 |
There was a problem hiding this comment.
assertFixture only compares manifest.input with the hash strings in the equally mutable fixture descriptor; it never compares them with sha256Canonical(getScenario(...).document()) or the scenario's cursor telemetry. If the descriptor's document/telemetry digests and the matching manifest fields are edited together, verify accepts arbitrary values, allowing incompatible measurements to be presented as comparable or compatible ones to be rejected. Recompute these digests from the registered public-synthetic scenario during verification.
Useful? React with 👍 / 👎.
| if (cassette.wireApi === "responses" && round.response) { | ||
| const decoded = Buffer.from(round.response.bodyBase64, "base64").toString("utf8"); | ||
| for (const line of decoded.split("\n")) { | ||
| if (line.startsWith("data:")) scan(line.slice(5).trim()); | ||
| } |
There was a problem hiding this comment.
When a Chat response contains JSON text or tool arguments such as {"apiKey":"totally-secret-value"}, the cassette's JSON serialization escapes those quotes, so the file-level field regex does not detect them; this decoded scan also skips the response because it is restricted to Responses cassettes. Such a cassette passes adoption and can commit credentials or forbidden document/transcript fields. Parse and scan round.sse payloads for Chat as well as Responses.
AGENTS.md reference: AGENTS.md:L158-L160
Useful? React with 👍 / 👎.
| candidate.cassetteFiles | ||
| .map((file) => readCassette(file).resolvedModel) | ||
| .filter((model): model is string => typeof model === "string" && model.length > 0), |
There was a problem hiding this comment.
With --record, if a repetition exhausts its retries through an admission or network failure before any complete round, startRecorder writes only the .attempts.json receipt and the runner never creates the expected canonical cassette. This unconditional readCassette then throws ENOENT after the report is written, aborting candidate preparation for every later successful scenario instead of recording the failed scenario as ineligible. Check cassette availability or handle candidate preparation per scenario.
Useful? React with 👍 / 👎.
…eakers The preview raises a take by its loudness gain (up to +12 dB) and the output trim, with no limiter, so a loud take's peaks went past full scale and the audio device clipped them. The export holds them at -1.5 dBFS. Measured on a real take: a -7.1 dBFS peak raised 8.9 dB, 92 samples over. The mix now reaches the speakers through a WaveShaperNode: the identity up to the export's ceiling, then a tanh knee towards full scale. A curve has no state, so the preview still plays everything under the ceiling exactly as the export writes it. Checked in Chromium with an OfflineAudioContext: -120 dB error under the ceiling, and a +1.77 dBFS sine goes from 18800 samples over full scale to none. Refs getopenscreen#911
…K ceiling The renderer asked ScreenCaptureKit's writer for computeBitrate of the app's 4K ceiling with the high-frame-rate boost: 76.5 Mbit/s for every take, a 1080p one included, 4.25x the Windows budget for the same capture. Linux had the same bug and fixed it by sending no bitrate and deriving it from the negotiated size. macOS now does the same: the request carries no bitrate, and the helper uses Linux's rule on the size captureOutputSize settled (0.1 bit per pixel per frame, 2 to 60 Mbit/s): 12.4 Mbit/s at 1080p60, 49.8 at 4K60. Fixes getopenscreen#924
The compositor decodes every recording as BT.709 studio range. The PipeWire helper converted with swscale's default matrix, BT.601, and tagged nothing, so every Linux take shifted hue and saturation: pure red came out at Y 81, Cb 90 where BT.709 is 63 and 102. - swscale converts with BT.709 coefficients, full-range RGB in, studio range out. - The dmabuf path's scale_vaapi is asked for the same (out_color_matrix, out_range, primaries, transfer) instead of the driver's default. - The codec context carries range, matrix, primaries and transfer. software_path_converts_and_tags_bt709 stages a red frame through the software encoder and reads the YUV back: 63/102/240, and all four tags. Run under WSL (nix develop .#compositor-view, ffmpeg n8.1.2): it passes, the crate's 93 tests pass, and with the BT.709 call removed it fails on Y 81, Cb 90. Fixes getopenscreen#926
…s voice The mixer emitted at real time while the capture threads poll WASAPI and hand packets over late (15.6 ms at the default timer resolution, more under load). Every chunk whose packet had not arrived yet was zero-filled, leaving holes of up to 10 ms inside words: the crackle of getopenscreen#911. Real takes show ten or more such drops to digital silence in 25 s. The mixer now writes 100 ms behind real time. Timestamps still come from the frame count, so nothing moves on the timeline. A pause or a stop writes what the cushion holds up to that instant, and a source that ran dry (loopback after silence) resumes a cushion ahead so it lands when it played. Fixes getopenscreen#911
…nchor its clock exactly Review of getopenscreen#916 (CodeRabbit), plus two placement errors the same tests exposed on a loaded machine. - A resume right behind a pause cleared the queues before mixLoop had written the cushion: the last 80 to 90 ms of voice before the pause were lost (measured). The resume now waits for the pause flush. - A stop that lands on a pause mixLoop has not handled yet flushes to the pause, not to now: the paused span is out of the video. - A source coming back after running dry is placed by the mixer's real lag, not by the cushion alone; under load the difference reached 50 ms early. - The clock is anchored in beginTimeline and at the resume instant, not when mixLoop next wakes, which placed the whole track early by the wake-up delay. mixLoop reads the shared clock under the lock. New test: an instant pause/resume keeps the voice before it (400 of 400 ms, was 310 to 320) and resumes within one chunk of the pause.
…video frames The GOP was left to VideoToolbox; it is now one second by frame count and by duration, like Windows. Frames the writer input was not ready for or refused were dropped silently; recording-stopped now carries droppedVideoFrames.
Every wait in wgc-capture and cursor-sampler landed on the default 15.625 ms timer tick. Measured on a real take: webcam frames spaced in whole ticks (25.9 unique fps for 30), the cursor sampled at 22.7 Hz for 30, and WASAPI packets handed over late enough to leave holes in the voice (getopenscreen#911). Both helpers now hold a 1 ms timer resolution for their lifetime and opt out of power throttling, which Windows 11 otherwise uses to ignore that request. The video writer, WASAPI capture, audio mixer and webcam threads register with MMCSS. Every call failing is logged and survived. A 5 ms sleep now lasts 5.06 ms (median) instead of 15.45 ms. Fixes getopenscreen#921
The MediaRecorder error handler reset the renderer's state only, so the tray and the display-sleep blocker stayed on after the recorder failed. Review of getopenscreen#939.
The PipeWire helper's loop advanced the capture clock, which holds the last picture forward and drains the audio rings, only on a new frame or when recv_timeout expired. Every other message restarts that timeout, and cursor messages arrive at the compositor's rate while the mouse moves: on a static screen with a moving mouse nothing advanced, and the 2 s audio rings dropped their oldest samples. The heartbeat is now checked at the top of every pass, whatever woke the loop, and is the one place a tick advances the clock. Fixes getopenscreen#936
…hanged copyLatestFrame now takes the caller's last seen sequence and returns false without copying when nothing new arrived. Both capture backends share the rule through snapshotWebcamFrame, covered by webcam_snapshot_test.
Every Windows take carried 0.44 to 0.58 s of audio past its video (measured on three takes of 13 to 113 s, a constant gap, so a tail). The stop sequence quiesces WGC and stops the microphone, loopback, device watcher, keep-alive and webcam before it stops the mixer, and the mixer is clock-driven: it kept writing chunks through that half second. The mixer is now paused on the stop request, the same request the video writer stops on. The pause writes what the cushion holds up to that instant and nothing after; a pause already in place is left as it is. New test: a pause followed 300 ms later by a stop ends the track at the pause (410 ms for a pause at 418, the chunk grid). Fixes getopenscreen#942
…ects Every Windows recording was written in BT.601 and untagged. The helper fed the encoder RGB32, and the colour converter Media Foundation puts in front of it produced BT.601 whatever the media types said: solid red came out at Y 82, Cb 90 from the software and the hardware encoder alike, where BT.709 is 63 and 102. The compositor decodes every recording as BT.709, so colours shifted. The encoders also defaulted to Constrained Baseline. - The helper now converts BGRA to NV12 BT.709 studio range itself (SSE2) and feeds NV12 on every path. A 1080p frame costs 1.6 ms to hand over, against about 2 ms for the old copy plus converter. - Range, matrix, primaries and transfer are tagged BT.709 on both tracks. - H.264 High profile, with B-frames off through the encoding parameters (the software encoder added them in High). - The cpuInputIsNv12 option is gone: every system-memory path is NV12. mf_encoder_color_test drives the real encoder, software and hardware, and reads the file back: High, no B-frames, bt709 tags, red/green/blue exact to the code value, and the converter within one code value of BT.709 on noise. It runs from npm run build:native:win. Fixes getopenscreen#922 Fixes getopenscreen#923
…s the MMCSS mixer
whisper.cpp's t_dtw marks where a token ends, so taking a word's first-token t_dtw as its start put every word one token late (+175 ms median). The helper now starts a word at the previous token's t_dtw, ends it at its own last token's, and sends the first-token time as `anchor` to tell which stretch of speech owns it. The RMS lookback snap goes; phrase edges are anchored on the VAD in both directions. Adds the word-timing harness used to measure it (issue getopenscreen#948).
Cursor-only sessions never create a Capture, so recording_started stayed false and any stream error, even mid-recording, ended the helper.
Cursor-only sessions have no Capture, so 'started' means the stream reached streaming there. Errors after that stay warnings.
Measurement packages bind a baseline, record usage and replay Chat Completions and Responses transports offline so a result can be checked without repeating the live call.
…nest Chat outages and shared-budget timeouts can no longer land in an adopted measurement.
…ntity Keep failed retry attempts, recompute prompt/wire/rubric and expectedFailures, and fail closed if a Chat measurement changes concrete model.
Replay now hashes the full Chat request, so the checked-in control had to be re-recorded against the current source identity.
A transport failure before the first complete round never wrote a cassette, so the runner dropped that provider attempt. Adopt attempts from the live endpoint instead.
runner.ts is a measurement source anchor, so the checked-in control had to be re-recorded after the pre-round attempt fix.
A failed retry could leave a partial tape that later became canonical while results kept the last attempt. Replay Chat httpStatus, pin the symlink tests, and isolate the Responses fixtures.
…ials correctly Three related correctness bugs in the measurement arithmetic that the normal compare/baseline workflow hits without any artifact editing: - weightedAxisTrials rounded the weighted mean into whole successes, so a 0.625 mean over two repetitions entered the Newcombe comparison as 0.5. Trials stay fractional now; the Wilson arithmetic holds for non-integer k. - A wholly indeterminate repetition-axis scored a placeholder 1 (the score runChecks reports when nothing was decided), which averaged into axisScores and bound baselines as a nominal pass. Such repetitions now leave the mean, and an axis with nothing decidable scores 0. - The report summary averaged the same placeholder into its axis rates, so a judge-abstained scenario's freshly generated report disagreed with its manifest and finalize/export failed its own AXIS_SCORE_MISMATCH check. Report and manifest now share one implementation (undecidedAwareAxisMean), and MeasurementCounts carries measuredRepetitions so comparisons count only repetitions that decided weighted checks — an unknown repetition is not an implicit success, and comparing a wholly unmeasured axis is refused with UNMEASURED_AXIS instead of reading zero trials as a rate.
Four attribution bugs in the judge pass, each reachable through the
documented record/judge/replay workflow:
- The judge transport identity stamped the agent SDK's default retry
policy (default/2) while askJudge issues one raw fetch per verdict:
adopted evidence claimed retries that never ran. The judge recorder
now stamps {0,0}, and the chat identity reconstruction uses the
RECORDED policy so those cassettes survive replay validation.
- Offline judge replay sent the literal model "cassette"; the
transport-bound request hash covers the model, so every round came
back stale and assertFresh() failed the replay against itself. The
replay now sends the model the judge cassette was recorded with.
- persistRepetition truncated turn.answer at MAX_FIELD_CHARS, so the
judge scored cut text while replay regenerates the full answer — the
verdict evaluated different text than the measured turn and the
strict replay hash diverged. The answer is retained in full.
- The README's bounded Responses example ran the CLI without loading
.env.workbench, aborting at requireLiveEnv() with no credentials
even when the documented setup was followed exactly.
The adoption privacy gate scans artifact bytes, but a Responses cassette stores the full request base64-encoded — the user prompt, tool inputs and file paths are invisible to the file-level scan. A normal recording whose request body carries an OS absolute path or credential could therefore be adopted and committed while the same content in plaintext would have been refused. Decode each round's request and apply the existing privacy rules to the parsed content (keys and string values unescaped: the serialized form's own escape sequences read as drive paths to the private-path rule). No new rule set, no framework — the same privacyFailure that every other artifact goes through.
…tree One regeneration after the scope rollback, per the owner's instruction: the control binds its source manifest to the frozen clean tree (the original twelve commits plus the three retained fixes). The public synthetic fixture (input-fixture.json) and the recorded check verdicts (recorded-checks.json) are byte-identical to the original getopenscreen#653 head; the cassette request bytes, manifest, report and review receipt follow the retained measurement-arithmetic changes (measuredRepetitions, fractional trials) and the final source identity. Recorded offline with the same scripted loopback control; no provider call was made; verify, replay, self-compare and the bound-baseline path all pass on this exact tree.
Require the established fingerprint fields while retaining legacy optional transport fingerprints. Check judge cassette round counts against repetitions and judged checks, with missing-field, overcount, and nonempty undercount regressions.
621676f to
de3c527
Compare
Summary
Add tracked measurement packages with usage accounting, validated baseline bindings and offline replay for Chat Completions and Responses. Reviewed measurements retain their cassettes, inputs, source identity and check results so a later comparison can detect incompatible evidence instead of treating old scores as a current baseline.
The package adoption path preserves hashed bytes across Git and formatting. A runtime-only retry override lets bounded workbench runs control provider attempts without changing the desktop application's default retry behavior.
Related issue
Refs getopenscreen#459. This implements the versioned-evidence, usage-accounting and baseline-validation portions. It does not freeze or validate the missing original real-take baselines.
Type of change
Release impact
Desktop impact
Workbench tooling and the shared model configuration change; no new editor UI or installer behavior is introduced.
Screenshots / video
No visual interface change.
Testing
Replaced the old combined-source sample with
local-pr15-scripted-control-20260913-v1, generated against this standalone workbench branch and independently reviewed before adoption.Actual export, verification, offline replay, bound-baseline generation and self-comparison completed successfully. To reproduce the main checks:
This sample uses a local scripted model, not a paid provider. Its provider usage is explicitly incomplete because the fixture responses do not include usage blocks. It demonstrates offline workflow reproducibility, not complete token accounting or model quality.
The earlier combined-tree workbench tests and live-provider/offline-replay run remain historical evidence. They are not presented as a live model-quality baseline or a new paid-provider test on this split branch.
Known limits
The original real-screencast fixture is absent from the checkout used for validation. Its dependent scenarios remain unavailable, and this PR does not substitute synthetic results under those scenario IDs or claim a completed original-fixture baseline. The live recordings and credentials used during development are not included.