Skip to content

GPT-5.4 parity proof rollup#65216

Closed
100yenadmin wants to merge 49 commits into
openclaw:mainfrom
electricsheephq:rollup/gpt54-parity-proof
Closed

GPT-5.4 parity proof rollup#65216
100yenadmin wants to merge 49 commits into
openclaw:mainfrom
electricsheephq:rollup/gpt54-parity-proof

Conversation

@100yenadmin

Copy link
Copy Markdown
Contributor

Summary

This is the parity proof rollup for the GPT-5.4 / Codex parity program.

It supersedes:

The goal is to make the remaining proof/release-certification work reviewable as one coherent qa-lab/docs PR instead of six small slices.

What This Fixes From The Original GPT-5.4 Prompt

The original prompt was not just “make GPT-5.4 look better in anecdotes.” It was “make GPT-5.4 behave more agentically in practice, prove it, and make sure it really follows repo instructions instead of only reading them.”

This rollup carries forward the proof-side work needed for that:

  • expands the parity pack and gate behavior onto one proof branch
  • enforces real tool evidence on prose-shaped scenarios
  • adds the Anthropic mock baseline lane, including /v1/messages routing and SSE support
  • keeps remember-prompt and exact-reply handling honest in the mock server
  • makes qa-suite-summary.json self-describing with run provenance
  • carries the mock auth staging needed for offline parity runs
  • rewrites the docs/runbook around the final 2-PR closeout
  • adds instruction-followthrough-repo-contract, the missing proof for the original AGENT.md / SOUL.md followthrough complaint

No new public runtime API is introduced here.

Verification

Branch-owned proof coverage passed locally:

CI=1 pnpm exec vitest run \
  extensions/qa-lab/src/agentic-parity-report.test.ts \
  extensions/qa-lab/src/cli.runtime.test.ts \
  extensions/qa-lab/src/scenario-catalog.test.ts \
  extensions/qa-lab/src/mock-openai-server.test.ts \
  extensions/qa-lab/src/qa-gateway-config.test.ts \
  extensions/qa-lab/src/suite.summary-json.test.ts \
  extensions/qa-lab/src/gateway-child.test.ts

That suite passed 143/143 locally.

Workflow sanity also passed:

npx github-actionlint .github/workflows/parity-gate.yml

On the merged runtime+proof integration branch:

  1. the new instruction-followthrough-repo-contract scenario passed on the real QA harness
  2. the full offline structural parity rerun passed end to end
pnpm openclaw qa suite \
  --provider-mode mock-openai \
  --model openai/gpt-5.4 \
  --alt-model openai/gpt-5.4-alt \
  --parity-pack agentic \
  --output-dir .artifacts/qa-rollup/gpt54-mock

pnpm openclaw qa suite \
  --provider-mode mock-openai \
  --model anthropic/claude-opus-4-6 \
  --alt-model anthropic/claude-sonnet-4-6 \
  --parity-pack agentic \
  --output-dir .artifacts/qa-rollup/opus46-mock

pnpm openclaw qa parity-report \
  --repo-root . \
  --candidate-summary .artifacts/qa-rollup/gpt54-mock/qa-suite-summary.json \
  --baseline-summary .artifacts/qa-rollup/opus46-mock/qa-suite-summary.json \
  --candidate-label openai/gpt-5.4 \
  --baseline-label anthropic/claude-opus-4-6 \
  --output-dir .artifacts/qa-rollup/parity-mock

That produced a green offline parity verdict on the merged 11-scenario pack:

  • candidate: 11/11 pass
  • baseline: 11/11 pass
  • parity verdict: pass
  • fake-success count: 0 on both sides

Separately, we already have a live GPT-5.4 10/10 harness pass on the patched stack. This PR does not overclaim a finished live Opus baseline; it keeps the structural parity proof and release artifacts merge-ready so the final live-provider proof can be attached cleanly once provider access is stable.

Dependency Note

This PR is the proof half of the 2-PR closeout. It is intended to land alongside the runtime completion rollup, while keeping A/B/C/D closed and preserving their review history.

Eva added 30 commits April 12, 2026 02:48
…ment

Addresses two loop-7 Copilot findings on PR #64662:

1. Hard-coded 'GPT-5.4 / Opus 4.6' markdown H1: the renderer now uses a
   template string that interpolates candidateLabel and baselineLabel, so
   any parity run (not only gpt-5.4 vs opus 4.6) renders an accurate
   title in saved reports. Default CLI flags still produce
   openai/gpt-5.4 vs anthropic/claude-opus-4-6 as the baseline pair.

2. Stale 'declared first-wave parity scenarios' comment in
   scopeSummaryToParityPack: the parity pack is now the ten-scenario
   first-wave+second-wave set (PR D + PR E). Comment updated to drop
   the first-wave qualifier and name the full QA_AGENTIC_PARITY_SCENARIOS
   constant the scope is filtering against.

New regression: 'parametrizes the markdown header from the comparison
labels' — asserts that non-default labels (openai/gpt-5.4-alt vs
openai/gpt-5.4) render in the H1.

Validation: pnpm test extensions/qa-lab/src/agentic-parity-report.test.ts
(13/13 pass).

Refs #64227
Closes criterion 2 of the GPT-5.4 parity completion gate in #64227 ('no
fake progress / fake tool completion') for the two first/second-wave
parity scenarios that can currently pass with a prose-only reply.

Background: the scenario framework already exposes tool-call assertions
via /debug/requests on the mock server (see approval-turn-tool-followthrough
for the pattern). Most parity scenarios use this seam to require a specific
plannedToolName, but source-docs-discovery-report and subagent-handoff
only checked the assistant's prose text, which means a model could fabricate:

- a Worked / Failed / Blocked / Follow-up report without ever calling
  the read tool on the docs / source files the prompt named
- three labeled 'Delegated task', 'Result', 'Evidence' sections without
  ever calling sessions_spawn to delegate

Both gaps are fake-progress loopholes for the parity gate.

Changes:

- source-docs-discovery-report: require at least one read tool call tied
  to the 'worked, failed, blocked' prompt in /debug/requests. Failure
  message dumps the observed plannedToolName list for debugging.
- subagent-handoff: require at least one sessions_spawn tool call tied
  to the 'delegate' / 'subagent handoff' prompt in /debug/requests. Same
  debug-friendly failure message.

Both assertions are gated behind !env.mock so they no-op in live-frontier
mode where the real provider exposes plannedToolName through a different
channel (or not at all).

Not touched: memory-recall is also in the parity pack but its pass path
is legitimately 'read the fact from prior-turn context'. That is a valid
recall strategy, not fake progress, so it is out of scope for this PR.
memory-recall's fake-progress story (no real memory_search call) would
require bigger mock-server changes and belongs in a follow-up that
extends the mock memory pipeline.

Validation:

- pnpm test extensions/qa-lab/src/scenario-catalog.test.ts

Refs #64227
Closes the last local-runnability gap on criterion 5 of the GPT-5.4 parity
completion gate in #64227 ('the parity gate shows GPT-5.4 matches or beats
Opus 4.6 on the agreed metrics').

Background: the parity gate needs two comparable scenario runs - one
against openai/gpt-5.4 and one against anthropic/claude-opus-4-6 - so the
aggregate metrics and verdict in PR D (#64441) can be computed. Today the
qa-lab mock server only implements /v1/responses, so the baseline run
against Claude Opus 4.6 requires a real Anthropic API key. That makes the
gate impossible to prove end-to-end from a local worktree and means the
CI story is always 'two real providers + quota + keys'.

This PR adds a /v1/messages Anthropic-compatible route to the existing
mock OpenAI server. The route is a thin adapter that:

- Parses Anthropic Messages API request shapes (system as string or
  [{type:text,text}], messages with string or block content, text and
  tool_result and tool_use and image blocks)
- Translates them into the ResponsesInputItem[] shape the existing shared
  scenario dispatcher (buildResponsesPayload) already understands
- Calls the shared dispatcher so both the OpenAI and Anthropic lanes run
  through the exact same scenario prompt-matching logic (same subagent
  fanout state machine, same extractRememberedFact helper, same
  '/debug/requests' telemetry)
- Converts the resulting OpenAI-format events back into an Anthropic
  message response with text and tool_use content blocks and a correct
  stop_reason (tool_use vs end_turn)

Non-streaming only: the QA suite runner falls back to non-streaming mock
mode so real Anthropic SSE isn't necessary for the parity baseline.

Also adds claude-opus-4-6 and claude-sonnet-4-6 to /v1/models so baseline
model-list probes from the suite runner resolve without extra config.

Tests added:

- advertises Anthropic claude-opus-4-6 baseline model on /v1/models
- dispatches an Anthropic /v1/messages read tool call for source discovery
  prompts (tool_use stop_reason, correct input path, /debug/requests
  records plannedToolName=read)
- dispatches Anthropic /v1/messages tool_result follow-ups through the
  shared scenario logic (subagent-handoff two-stage flow: tool_use -
  tool_result - 'Delegated task / Evidence' prose summary)

Local validation:

- pnpm test extensions/qa-lab/src/mock-openai-server.test.ts (18/18 pass)
- pnpm test extensions/qa-lab/src/mock-openai-server.test.ts extensions/qa-lab/src/cli.runtime.test.ts extensions/qa-lab/src/scenario-catalog.test.ts (47/47 pass)

Refs #64227
Unblocks #64441 (parity harness) and the forthcoming qa parity run wrapper
by giving the baseline lane a local-only mock path.
Addresses loop-6 review feedback on PR #64681:

1. Copilot / Greptile / codex-connector all flagged that the discovery
   scenario's .includes('worked, failed, blocked') assertion is
   case-sensitive but the real prompt says 'Worked, Failed, Blocked...',
   so the mock-mode assertion never matches. Fix: lowercase-normalize
   allInputText before the contains check.
2. Greptile P2: the expr and message.expr each called fetchJson
   separately, incurring two round-trips to /debug/requests. Fix: hoist
   the fetch to a set step (discoveryDebugRequests / subagentDebugRequests)
   and reuse the snapshot.
3. Copilot: the subagent-handoff assertion scanned the entire request
   log and matched the first request with 'delegate' in its input text,
   which could false-pass on a stale prior scenario. Fix: reverse the
   array and take the most recent matching request instead.

Validation: pnpm test extensions/qa-lab/src/scenario-catalog.test.ts
(4/4 pass).

Refs #64227
Addresses the loop-6 Copilot / Greptile finding on PR #64685: in
`convertAnthropicMessagesToResponsesInput`, `tool_result` blocks were
pushed to `items` inside the per-block loop while the surrounding
user/assistant message was only pushed after the loop finished. That
reordered the function_call_output BEFORE its parent user message
whenever a user turn mixed `tool_result` with fresh text/image blocks,
which broke `extractToolOutput` (it scans AFTER the last user-role
index; function_call_output placed BEFORE that index is invisible to it)
and made the downstream scenario dispatcher behave as if no tool output
had been returned on mixed-content turns.

Fix: buffer `tool_result` and `tool_use` blocks in local arrays during
the per-block loop, push the parent role message first (when it has any
text/image pieces), then push the accumulated function_call /
function_call_output items in original order. tool_result-only user
turns still omit the parent message as before, so the non-mixed
subagent-fanout-synthesis two-stage flow that already worked keeps
working.

Regression added:

- `places tool_result after the parent user message even in mixed-content
  turns` — sends a user turn that mixes a `tool_result` block with a
  trailing fresh text block, then inspects `/debug/last-request` to
  assert that `toolOutput === 'SUBAGENT-OK'` (extractToolOutput found
  the function_call_output AFTER the last user index) and
  `prompt === 'Keep going with the fanout.'` (extractLastUserText picked
  up the trailing fresh text).

Local validation: pnpm test extensions/qa-lab/src/mock-openai-server.test.ts
(19/19 pass).

Refs #64227
…uests

Pass-2 codex-connector P1 finding on #64681: the reverse-find pattern I
used on pass 1 usually lands on the FOLLOW-UP request after the mock
runs sessions_spawn, not the pre-tool planning request that actually
has plannedToolName === 'sessions_spawn'. The mock only plans that tool
on requests with !toolOutput (mock-openai-server.ts:662), so the
post-tool request has plannedToolName unset and the assertion fails
even when the handoff succeeded.

Fix: switch the assertion back to a forward .some() match but add a
!request.toolOutput filter so the match is pinned to the pre-tool
planning phase. The case-insensitive regex, the fetchJson dedupe, and
the failure-message diagnostic from pass 1 are unchanged.

Validation: pnpm test extensions/qa-lab/src/scenario-catalog.test.ts
(4/4 pass).

Refs #64227
Closes the 'summary cannot be label-verified' half of criterion 5 on the
GPT-5.4 parity completion gate in #64227.

Background: the parity gate in #64441 compares two qa-suite-summary.json
files and trusts whatever candidateLabel / baselineLabel the caller
passes. Today the summary JSON only contains { scenarios, counts }, so
nothing in the summary records which provider/model the run actually
used. If a maintainer swaps candidate and baseline summary paths in a
parity-report call, the verdict is silently mislabeled and nobody can
retroactively verify which run produced which summary.

Changes:

- Add a 'run' block to qa-suite-summary.json with startedAt, finishedAt,
  providerMode, primaryModel (+ provider and model splits),
  alternateModel (+ provider and model splits), fastMode, concurrency,
  scenarioIds (when explicitly filtered).
- Extract a pure 'buildQaSuiteSummaryJson(params)' helper so the summary
  JSON shape is unit-testable and the parity gate (and any future parity
  wrapper) can import the exact same type rather than reverse-engineering
  the JSON shape at runtime.
- Thread 'scenarioIds' from 'runQaSuite' into writeQaSuiteArtifacts so
  --scenario-ids flags are recorded in the summary.

Unit tests added (src/suite.summary-json.test.ts, 5 cases):

- records provider/model/mode so parity gates can verify labels
- includes scenarioIds in run metadata when provided
- records an Anthropic baseline lane cleanly for parity runs
- leaves split fields null when a model ref is malformed
- keeps scenarios and counts alongside the run metadata

This is additive: existing callers of qa-suite-summary.json continue to
see the same { scenarios, counts } shape, just with an extra run field.
No existing consumers of the JSON need to change.

The follow-up 'qa parity run' CLI wrapper (run the parity pack twice
against candidate + baseline, emit two labeled summaries in one command)
stacks cleanly on top of this change and will land as a separate PR
once #64441 and #64662 merge so the wrapper can call runQaParityReportCommand
directly.

Local validation:

- pnpm test extensions/qa-lab/src/suite.summary-json.test.ts (5/5 pass)
- pnpm test extensions/qa-lab/src/suite.summary-json.test.ts extensions/qa-lab/src/cli.runtime.test.ts extensions/qa-lab/src/scenario-catalog.test.ts (34/34 pass)

Refs #64227
Unblocks the final parity run for #64441 / #64662 by making summaries
self-describing.
Addresses the pass-3 codex-connector P1 on #64681: the pass-2 fix
filtered to pre-tool requests but still used a broad
`/delegate|subagent handoff/i` regex. The `subagent-fanout-synthesis`
scenario runs BEFORE `subagent-handoff` in catalog order (scenarios
are sorted by path), and the fanout prompt reads
'Subagent fanout synthesis check: delegate exactly two bounded
subagents sequentially' — which contains 'delegate' and also plans
sessions_spawn pre-tool. That produces a cross-scenario false pass
where the fanout's earlier sessions_spawn request satisfies the
handoff assertion even when the handoff run never delegates.

Fix: tighten the input-text match from `/delegate|subagent handoff/i`
to `/delegate one bounded qa task/i`, which is the exact scenario-
unique substring from the `subagent-handoff` config.prompt. That
pins the assertion to this scenario's request window and closes the
cross-scenario false positive.

Validation: pnpm test extensions/qa-lab/src/scenario-catalog.test.ts
(4/4 pass).

Refs #64227
…antics

Addresses 4 loop-6 Copilot / codex-connector findings on PR #64689
(re-opened as #64789):

1. P2 codex + Copilot: empty `scenarioIds` array was serialized as
   `[]` because of a truthiness check. The CLI passes an empty array
   when --scenario is omitted, so full-suite runs would incorrectly
   record an explicit empty selection. Fix: switch to a
   `length > 0` check so '[] or undefined' both encode as `null`
   in the summary run metadata.

2. Copilot: `buildQaSuiteSummaryJson` was exported for parity-gate
   consumers but its return type was `Record<string, unknown>`, which
   defeated the point of exporting it. Fix: introduce a concrete
   `QaSuiteSummaryJson` type that matches the JSON shape 1-for-1 and
   make the builder return it. Downstream code (parity gate, parity
   run wrapper) can now import the type and keep consumers
   type-checked.

3. Copilot: `QaSuiteSummaryJsonParams.providerMode` re-declared the
   `'mock-openai' | 'live-frontier'` string union even though
   `QaProviderMode` is already imported from model-selection.ts. Fix:
   reuse `QaProviderMode` so provider-mode additions flow through
   both types at once.

4. Copilot: test fixtures omitted `steps` from the fake scenario
   results, creating shape drift with the real suite scenario-result
   shape. Fix: pad the test fixtures with `steps: []` and tighten the
   scenarioIds assertion to read `json.run.scenarioIds` directly (the
   new concrete return type makes the type-cast unnecessary).

New regression: `treats an empty scenarioIds array as unspecified
(no filter)` — passes `scenarioIds: []` and asserts the summary
records `scenarioIds: null`.

Validation: pnpm test extensions/qa-lab/src/suite.summary-json.test.ts
(6/6 pass).

Refs #64227
Addresses two loop-7 Copilot findings on PR #64681:

1. source-docs-discovery-report.md: the explanatory comment said the
   debug request log was 'lowercased for case-insensitive matching',
   but the code actually lowercases each request's allInputText inline
   inside the .some() predicate, not the discoveryDebugRequests
   snapshot. Rewrite the comment to describe the inline-lowercase
   pattern so a future reader matches the code they see.

2. subagent-handoff.md: the comment said the assertion 'must be
   pinned to THIS scenario's request window' but the implementation
   actually relies on matching a scenario-unique prompt substring
   (/delegate one bounded qa task/i), not a request-window. Rewrite
   the comment to describe the substring pinning and keep the
   pre-tool filter rationale intact.

No runtime change; comment-only fix to keep reviewer expectations
aligned with the actual assertion shape.

Validation: pnpm test extensions/qa-lab/src/scenario-catalog.test.ts
(4/4 pass).

Refs #64227
Addresses the pass-3 codex-connector P2 on #64789 (repl of #64689):
`run.scenarioIds` was copied from the raw `params.scenarioIds`
caller input, but `runQaSuite` normalizes that input through
`selectQaSuiteScenarios` which dedupes via `Set` and reorders the
selection to catalog order. When callers repeat --scenario ids or
pass them in non-catalog order, the summary metadata drifted from
the scenarios actually executed, which can make parity/report
tooling treat equivalent runs as different or trust inaccurate
provenance.

Fix: both writeQaSuiteArtifacts call sites in runQaSuite now pass
`selectedCatalogScenarios.map(scenario => scenario.id)` instead of
`params?.scenarioIds`, so the summary records the post-selection
executed list. This also covers the full-suite case automatically
(the executed list is the full lane-filtered catalog), giving parity
consumers a stable record of exactly which scenarios landed in the
run regardless of how the caller phrased the request.

buildQaSuiteSummaryJson's `length > 0 ? [...] : null` pass-2
semantics are preserved so the public helper still treats an empty
array as 'unspecified' for any future caller that legitimately passes
one.

Validation: pnpm test extensions/qa-lab/src/suite.summary-json.test.ts
(6/6 pass).

Refs #64227
Copilot AI review requested due to automatic review settings April 12, 2026 06:55
@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation extensions: qa-lab size: XL r: too-many-prs Auto-close: author has more than twenty active PRs. labels Apr 12, 2026
@100yenadmin

Copy link
Copy Markdown
Contributor Author

Current verification on this rollup head:

  • branch-owned proof suite passed: 143/143
  • actionlint passed on .github/workflows/parity-gate.yml
  • on the merged runtime+proof integration branch, the new instruction-followthrough-repo-contract scenario passed on the real QA harness
  • the full offline structural parity rerun on the merged 11-scenario pack passed end to end
    • candidate: 11/11
    • baseline: 11/11
    • parity verdict: pass
    • fake-success count: 0 on both sides
  • we also already have the live GPT-5.4 10/10 harness pass on the patched stack

This PR supersedes #64662, #64681, #64685, #64789, #64837, and #64909 so maintainers only have one proof/release-certification branch to review.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR rolls up the remaining GPT-5.4/Codex agentic parity proof work into a single QA-lab + docs change set, strengthening the parity gate with additional scenarios, stricter “real tool use” evidence, offline dual-provider mock support (OpenAI + Anthropic), and self-describing run artifacts.

Changes:

  • Expands the agentic parity pack to 11 scenarios (including a new repo-instruction followthrough contract) and tightens required-scenario gate semantics.
  • Enforces tool-call evidence in previously prose-satisfiable scenarios; adds/extends mock-server telemetry and Anthropic /v1/messages (incl. SSE) support for offline baseline runs.
  • Makes qa-suite-summary.json self-describing with run provenance (provider/mode/models/scenario filter), updates QA gateway config for mock provider lanes, and refreshes docs + adds a CI parity-gate workflow.

Reviewed changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
qa/scenarios/subagent-handoff.md Adds /debug/requests-based sessions_spawn assertion to prevent prose-only delegation.
qa/scenarios/subagent-fanout-synthesis.md Adds mock tool-call assertions to confirm real fanout spawning occurred.
qa/scenarios/source-docs-discovery-report.md Requires at least one read tool call before accepting the prose report (mock-only).
qa/scenarios/model-switch-tool-continuity.md Generalizes alternate-model assertion to match the configured --alt-model.
qa/scenarios/memory-recall.md Documents intentional prose-only nature of memory recall scenario.
qa/scenarios/instruction-followthrough-repo-contract.md Introduces new repo-contract followthrough scenario (read→read→read→write + explicit reply format).
qa/scenarios/image-understanding-attachment.md Refactors image-input assertion to reuse a single debug request lookup (mock-only).
qa/scenarios/config-restart-capability-flip.md Adds mock /debug/requests assertion requiring image_generate after restart.
extensions/qa-lab/src/suite.ts Adds buildQaSuiteSummaryJson and a run provenance block to qa-suite-summary.json; threads scenarioIds through writers.
extensions/qa-lab/src/suite.summary-json.test.ts Adds unit tests for the new summary JSON provenance behavior.
extensions/qa-lab/src/scenario-catalog.test.ts Adds regressions ensuring mock-only assertions are guarded; validates new scenario config.
extensions/qa-lab/src/qa-gateway-config.ts Adds mock “openai” and “anthropic” provider configs (incl. private-network allowance) to support offline parity lanes.
extensions/qa-lab/src/qa-gateway-config.test.ts Tests provider-qualified refs mapping through the mock lane and request config.
extensions/qa-lab/src/mock-openai-server.ts Adds provider-variant tagging, Anthropic /v1/messages adapter (incl. SSE), and routing fixes for remember/exact-reply handling.
extensions/qa-lab/src/mock-openai-server.test.ts Adds extensive coverage for Anthropic route behavior, streaming, remember/exact-reply precedence, and provider variant tagging.
extensions/qa-lab/src/gateway-child.ts Stages placeholder mock auth profiles for offline runs; defaults provider mode consistently.
extensions/qa-lab/src/gateway-child.test.ts Tests mock auth staging and default provider-mode behavior.
extensions/qa-lab/src/cli.runtime.test.ts Updates expected agentic parity scenario IDs list to include the expanded pack.
extensions/qa-lab/src/agentic-parity.ts Adds per-scenario “countsTowardValidToolCallRate” metadata and exposes tool-backed titles.
extensions/qa-lab/src/agentic-parity-report.ts Tightens gate behavior (required scenario failures fail the gate), improves fake-success detection, label provenance verification, and parametrizes report header.
extensions/qa-lab/src/agentic-parity-report.test.ts Adds regressions for new gate semantics, provenance checking, positive-tone fake-success detection, and updated header rendering.
docs/help/gpt54-codex-agentic-parity.md Rewrites parity docs around the final 2-PR closeout and the 11-scenario pack; clarifies mock vs live proof.
docs/help/gpt54-codex-agentic-parity-maintainers.md Updates maintainer review guide/checklist for the rollup shape and proof expectations.
.github/workflows/parity-gate.yml Adds CI workflow to run the offline mock structural parity gate and upload artifacts.

Comment on lines +1331 to +1341
export type QaSuiteSummaryJsonParams = {
scenarios: QaSuiteScenarioResult[];
startedAt: Date;
finishedAt: Date;
providerMode: QaProviderMode;
primaryModel: string;
alternateModel: string;
fastMode: boolean;
concurrency: number;
scenarioIds?: readonly string[];
};

Copilot AI Apr 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QaSuiteSummaryJsonParams and QaSuiteSummaryJson are exported but reference the non-exported QaSuiteScenarioResult type. With declaration: true in the repo tsconfig, this will trigger a TypeScript error (exported type uses private name) and can break type-check/build. Export QaSuiteScenarioResult (or inline/duplicate the structural shape in the exported types) so the public signature only references exported types.

Copilot uses AI. Check for mistakes.
Comment on lines +884 to +887
// Scope: handles non-streaming Anthropic Messages requests with text and
// tool_result content blocks, which is what the QA suite runner actually
// sends. Streaming is intentionally out of scope for this mock because the
// suite runner supports non-streaming fallback.

Copilot AI Apr 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment says streaming is out of scope for the Anthropic /v1/messages mock, but the route below implements and tests SSE streaming (body.stream === true, writeAnthropicSse). Please update the comment to match the current behavior so future readers don’t disable or remove the streaming path accidentally.

Suggested change
// Scope: handles non-streaming Anthropic Messages requests with text and
// tool_result content blocks, which is what the QA suite runner actually
// sends. Streaming is intentionally out of scope for this mock because the
// suite runner supports non-streaming fallback.
// Scope: handles Anthropic Messages requests with text and tool_result
// content blocks, which is what the QA suite runner actually sends. The
// mock supports both non-streaming responses and SSE streaming when
// `body.stream === true`, re-serializing the shared dispatcher output into
// Anthropic message events for parity coverage.

Copilot uses AI. Check for mistakes.
Comment on lines +160 to +162
# against a post-restart request. Scoped to `!env.mock` so
# live-frontier runs (which don't expose `/debug/requests`)
# still pass the rest of the scenario.

Copilot AI Apr 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment mismatch: this assertion is guarded by !env.mock || ..., meaning it runs only in mock mode. The note currently says "Scoped to !env.mock", which reads as the opposite. Please reword to avoid confusion (e.g., "Guarded by !env.mock || so live-frontier runs skip the /debug/requests check").

Suggested change
# against a post-restart request. Scoped to `!env.mock` so
# live-frontier runs (which don't expose `/debug/requests`)
# still pass the rest of the scenario.
# against a post-restart request. Guarded by `!env.mock ||`
# so live-frontier runs (which don't expose `/debug/requests`)
# skip this check and still pass the rest of the scenario.

Copilot uses AI. Check for mistakes.
@openclaw-barnacle

Copy link
Copy Markdown

Closing this PR because the author has more than 10 active PRs in this repo. Please reduce the active PR queue and reopen or resubmit once it is back under the limit. You can close your own PRs to get back under the limit.

@greptile-apps

greptile-apps Bot commented Apr 12, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This rollup consolidates six earlier parity-proof slices into a single reviewable PR: it adds an Anthropic /v1/messages route to the QA mock server, introduces a new instruction-followthrough-repo-contract scenario to the agentic parity pack, wires up mock auth-profile staging so offline parity runs never need real API keys, and ships a CI parity gate workflow that runs both model lanes against the mock and compares verdicts.

All inline findings are P2 (style/quality) — the most actionable is moving subagentFanoutPhase inside the startQaMockOpenAiServer closure to avoid module-level state pollution under concurrent Vitest workers.

Confidence Score: 5/5

Safe to merge — all findings are P2 style/quality suggestions with no blocking correctness issues.

The parity engine logic, label-mismatch detection, fake-success heuristics, Anthropic SSE adapter, and CI workflow are all well-designed and well-tested (143 tests passing). The three inline comments are minor: a stale comment, a module-level state scoping concern, and non-deterministic mock IDs. None affect the gate verdict or runtime correctness.

extensions/qa-lab/src/mock-openai-server.ts — module-level subagentFanoutPhase state and stale streaming comment.

Comments Outside Diff (1)

  1. extensions/qa-lab/src/mock-openai-server.ts, line 126-127 (link)

    P2 Module-level mutable state risks test interference

    subagentFanoutPhase is shared across all server instances created within the same module. Each startQaMockOpenAiServer call resets it to 0, so sequential tests are fine — but if Vitest schedules two tests concurrently in the same worker thread (possible with the default threads pool and --isolate=false), a second server start could reset the phase mid-fanout for an already-running test. The testing guidelines require module state to be cleaned up for --isolate=false safety.

    Moving the phase inside the closure returned by startQaMockOpenAiServer would scope it to each server instance:

    export async function startQaMockOpenAiServer(params?: { host?: string; port?: number }) {
      const host = params?.host ?? "127.0.0.1";
      let subagentFanoutPhase = 0; // scoped to this server instance
      // …
    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: extensions/qa-lab/src/mock-openai-server.ts
    Line: 126-127
    
    Comment:
    **Module-level mutable state risks test interference**
    
    `subagentFanoutPhase` is shared across all server instances created within the same module. Each `startQaMockOpenAiServer` call resets it to `0`, so sequential tests are fine — but if Vitest schedules two tests concurrently in the same worker thread (possible with the default `threads` pool and `--isolate=false`), a second server start could reset the phase mid-fanout for an already-running test. The testing guidelines require module state to be cleaned up for `--isolate=false` safety.
    
    Moving the phase inside the closure returned by `startQaMockOpenAiServer` would scope it to each server instance:
    ```ts
    export async function startQaMockOpenAiServer(params?: { host?: string; port?: number }) {
      const host = params?.host ?? "127.0.0.1";
      let subagentFanoutPhase = 0; // scoped to this server instance
      //
    ```
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: extensions/qa-lab/src/mock-openai-server.ts
Line: 882-888

Comment:
**Stale comment: streaming is implemented here**

The block comment on line 887 says "Streaming is intentionally out of scope for this mock", but the route handler directly below (line 1397) branches on `body.stream === true` and calls `writeAnthropicSse(res, streamEvents)` — so streaming is in fact supported. A developer reading this comment before working on the Anthropic client path could believe streaming needs to be added and introduce duplicate or conflicting code.

```suggestion
// Scope: handles both streaming and non-streaming Anthropic Messages requests
// with text, image, tool_use, and tool_result content blocks. Uses the same
// shared buildResponsesPayload() dispatcher as the /v1/responses route so the
// parity harness gets a baseline lane without needing real Anthropic API keys.
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: extensions/qa-lab/src/mock-openai-server.ts
Line: 126-127

Comment:
**Module-level mutable state risks test interference**

`subagentFanoutPhase` is shared across all server instances created within the same module. Each `startQaMockOpenAiServer` call resets it to `0`, so sequential tests are fine — but if Vitest schedules two tests concurrently in the same worker thread (possible with the default `threads` pool and `--isolate=false`), a second server start could reset the phase mid-fanout for an already-running test. The testing guidelines require module state to be cleaned up for `--isolate=false` safety.

Moving the phase inside the closure returned by `startQaMockOpenAiServer` would scope it to each server instance:
```ts
export async function startQaMockOpenAiServer(params?: { host?: string; port?: number }) {
  const host = params?.host ?? "127.0.0.1";
  let subagentFanoutPhase = 0; // scoped to this server instance
  //
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: extensions/qa-lab/src/mock-openai-server.ts
Line: 1096

Comment:
**Non-deterministic message IDs in mock responses**

`Math.random()` produces a different `id` on every response, including across parity report reruns. This is fine today because the `/debug/requests` snapshot stores the request, not the response. However, if a future test or parity assertion ever serialises the full response body (for example to compare a snapshot), these IDs will cause spurious diffs. A simple counter or a hash of the response content would keep the IDs stable across runs.

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "qa: roll up parity proof closeout" | Re-trigger Greptile

Comment on lines +882 to +888
// real Anthropic API keys.
//
// Scope: handles non-streaming Anthropic Messages requests with text and
// tool_result content blocks, which is what the QA suite runner actually
// sends. Streaming is intentionally out of scope for this mock because the
// suite runner supports non-streaming fallback.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Stale comment: streaming is implemented here

The block comment on line 887 says "Streaming is intentionally out of scope for this mock", but the route handler directly below (line 1397) branches on body.stream === true and calls writeAnthropicSse(res, streamEvents) — so streaming is in fact supported. A developer reading this comment before working on the Anthropic client path could believe streaming needs to be added and introduce duplicate or conflicting code.

Suggested change
// real Anthropic API keys.
//
// Scope: handles non-streaming Anthropic Messages requests with text and
// tool_result content blocks, which is what the QA suite runner actually
// sends. Streaming is intentionally out of scope for this mock because the
// suite runner supports non-streaming fallback.
// Scope: handles both streaming and non-streaming Anthropic Messages requests
// with text, image, tool_use, and tool_result content blocks. Uses the same
// shared buildResponsesPayload() dispatcher as the /v1/responses route so the
// parity harness gets a baseline lane without needing real Anthropic API keys.
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/qa-lab/src/mock-openai-server.ts
Line: 882-888

Comment:
**Stale comment: streaming is implemented here**

The block comment on line 887 says "Streaming is intentionally out of scope for this mock", but the route handler directly below (line 1397) branches on `body.stream === true` and calls `writeAnthropicSse(res, streamEvents)` — so streaming is in fact supported. A developer reading this comment before working on the Anthropic client path could believe streaming needs to be added and introduce duplicate or conflicting code.

```suggestion
// Scope: handles both streaming and non-streaming Anthropic Messages requests
// with text, image, tool_use, and tool_result content blocks. Uses the same
// shared buildResponsesPayload() dispatcher as the /v1/responses route so the
// parity harness gets a baseline lane without needing real Anthropic API keys.
```

How can I resolve this? If you propose a fix, please make it concise.

countApproxTokens(params.extracted.text) + params.extracted.toolCalls.length * 16,
);
return {
id: `msg_mock_${Math.floor(Math.random() * 1_000_000).toString(16)}`,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Non-deterministic message IDs in mock responses

Math.random() produces a different id on every response, including across parity report reruns. This is fine today because the /debug/requests snapshot stores the request, not the response. However, if a future test or parity assertion ever serialises the full response body (for example to compare a snapshot), these IDs will cause spurious diffs. A simple counter or a hash of the response content would keep the IDs stable across runs.

Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/qa-lab/src/mock-openai-server.ts
Line: 1096

Comment:
**Non-deterministic message IDs in mock responses**

`Math.random()` produces a different `id` on every response, including across parity report reruns. This is fine today because the `/debug/requests` snapshot stores the request, not the response. However, if a future test or parity assertion ever serialises the full response body (for example to compare a snapshot), these IDs will cause spurious diffs. A simple counter or a hash of the response content would keep the IDs stable across runs.

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Copy Markdown
Contributor Author

This parity rollup was auto-closed, but the branch/work is still valid. A fresh replacement PR from the same branch is now open at #65224 so the intended 2-PR rollup shape is restored.

Everything carried here remains the same proof surface:

  • 11-scenario parity pack, including instruction-followthrough-repo-contract
  • offline structural parity rerun green end to end
  • live GPT-5.4 harness previously verified 10/10 on the patched stack

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Improvements or additions to documentation extensions: qa-lab r: too-many-prs Auto-close: author has more than twenty active PRs. size: XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants