feat(lab): CL-01 deterministic protocol conformance harness - #1320
Merged
Conversation
Implement the CL-00 protocol conformance runner with manifest loading, assertion DSL evaluation, shipped adapter/bridge execution paths, and eight negative controls for the initial five protocol suites.
Rebase onto dev CL-00 merge, remove Chat→Responses observation projection, apply source-protocol SSE [DONE] rules, synthetic provenance, MCP actions, and Chat tool_call_id selectors per final Protocol V1 authority.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds a fixture-driven CL-01 protocol-conformance harness. It defines contracts, validates manifests and digests, normalizes protocol events, executes scenarios, evaluates assertions, runs negative controls, corrects OpenAI Chat and Anthropic wire behavior, and records acceptance. ChangesCL-01 Conformance
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Runner
participant Executor
participant Adapter
participant Normalizer
participant Observation
participant Assertions
Runner->>Executor: Execute a conformance scenario
Executor->>Adapter: Build and parse protocol requests
Adapter-->>Executor: Return response or stream
Executor->>Normalizer: Normalize SSE or bridge events
Normalizer-->>Observation: Record normalized events
Observation-->>Executor: Finalize observation and verifiers
Executor->>Assertions: Evaluate required assertions
Assertions-->>Runner: Return scenario result
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
8 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 29
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@devlog/_plan/260807_compatibility_lab/051_cl01_acceptance_review.md`:
- Around line 25-37: Resolve the three harness integrity issues before retaining
the corresponding PASS claims: in the executor flow around the observation
normalization and scenario-event filtering, preserve shipped-stack output
without injecting final_answer data or narrowing events to errors; update the
negative control in runNegativeControls to use the Chat tool-result selector so
mutation of toolCallId is what causes rejection; and ensure the mcp-core
verifier outputs from executeMcpSyntheticAction survive attachVerifiers and the
runner path used by assertions. Then revise challenges 1, 5, and 12 in the
acceptance record to reflect the verified behavior.
In `@src/adapters/openai-chat.ts`:
- Around line 298-304: The text-only developer branch in the adapter must not
emit a direct role:"developer" entry, because it bypasses the tool-call barrier.
In the developer-message handling around the visible branch, build the text and
route it through the existing deferred/user barrier path, preserving folding
into the leading system message for non-OpenAI providers; remove the direct
out.push and add the requested regression coverage in the dangling-toolcalls
test.
In `@src/lab/conformance/assertion.ts`:
- Around line 182-208: Update evaluateIdStable to reject assertion.expected
pointer lists containing fewer than two entries before resolving pointers or
evaluating stability. Return the same invalid_expected failure shape and
semantics used by evaluateIdCorrelates, while preserving the existing behavior
for valid lists.
- Line 235: In src/lab/conformance/assertion.ts at lines 235-235 and 260-260,
add one shared identifier-comparison helper and use it in evaluateIdCorrelates
and evaluateToolResultCorrelates instead of String coercion. The helper must
require both resolved values to be non-empty strings and return true only when
they are equal; apply it to left.value/right.value and call.value/result.value
respectively.
In `@src/lab/conformance/executor.ts`:
- Around line 111-158: Wrap each switch clause in the executor’s scenario
dispatch that declares local const bindings—especially json-sse-equivalence,
anthropic request-mapping, and anthropic tool-round-trip—in its own block scope.
Preserve each clause’s existing statements and return behavior while ensuring
json, sseEvents, anthropicBody, translated, parsedAnthropic, responsesProvider,
toolBody, toolTranslated, and parsedTool are scoped only to their respective
cases.
- Around line 434-436: Update executeStreamScenario to use the parsed initiating
request: pass initiating through the stream adapter and record it with
recordUpstreamRequest before returning, preserving fixture-based response
handling. If stream scenarios are intentionally response-only, remove the unused
initiating binding and corresponding upstream request assertions instead.
- Around line 50-58: Replace the hand-rolled frame parsing in the executor with
the shared normalizeSseBytes flow, passing the collected bridge text as bytes
and using its normalized frames as the parser result. Remove the now-unused
eventsFromBridgeFrames import and related local parsing logic, after confirming
no other caller in this file depends on it.
- Around line 463-468: Remove the special-case `events[0].data.phase =
"final_answer"` mutation guarded by `caseRecord.id ===
"codex-core.protocol.streaming-turn"` in the conformance executor. Preserve the
normalized bridge output without harness-added fields; provide `phase` through
the upstream fixture or adapter only if required, otherwise mark the case
unsupported.
In `@src/lab/conformance/fixture-provider.ts`:
- Around line 15-28: Update upstreamAdapterForProtocol so unmapped protocol
values throw an unsupported-protocol error instead of returning "openai-chat";
preserve the explicit mappings for supported protocols and ensure values such as
"cursor-protobuf" are rejected so callers report them as harness_failure.
In `@src/lab/conformance/harness-budget.ts`:
- Around line 17-40: Update withHarnessTranslatorBudget to expose a disposal
handle on the returned TestAdapter and have it call dispose() on the budget
created by createTranslatorBudget(). Ensure the harness can invoke this handle
before discarding the wrapped adapter, while preserving the existing
buildRequest, parseStream, and parseResponse delegation behavior.
In `@src/lab/conformance/jcs.ts`:
- Around line 3-17: Update jcsStringify to handle undefined explicitly:
serialize undefined array elements as null, omit object properties whose values
are undefined, and ensure the function always returns a string. Preserve the
existing canonical key ordering and primitive handling, and reject non-finite
numbers such as NaN and Infinity if they can reach this function.
In `@src/lab/conformance/json-pointer.ts`:
- Around line 25-30: Update the property lookup in the JSON pointer traversal to
use an own-property check instead of the prototype-walking `in` operator. Change
the lookup within the visible traversal block before assigning `current =
obj[token]`, while preserving the existing `selector_missing` result for absent
segments and ensuring inherited names such as `constructor` and `__proto__` are
treated as absent.
In `@src/lab/conformance/manifest.ts`:
- Around line 76-78: Update expandFailureRules to validate that
authority.manifestDefaults.failureRuleSet resolves to an existing entry in
authority.failureRuleSets before spreading it, and surface the missing-set
condition through the harness’s established harness_failure/contract_integrity
classification rather than allowing a TypeError. Preserve the existing rule
expansion behavior when the configured set exists.
- Around line 136-140: Replace the unconditional length check in
validateScenarioManifestDigest with an actual comparison between
scenarioManifestDigest(expanded) and the corresponding recorded digest from
authority. Use the existing CaseAuthority digest field or accessor, and return
false when it is absent or does not match; otherwise remove this validator and
all callers so the harness does not claim integrity validation.
- Around line 14-22: Update AUTHORITY_FILE to match the runtime fixture filename
loaded by loadCaseAuthority(), namely protocol-v1-cases.json, so fixtureRef()
and validateExpandedFixtureRef() report and validate the actual authority
source.
In `@src/lab/conformance/mcp-stub.ts`:
- Around line 75-93: Update runSchemaBounds to validate limitBytes as a positive
integer and return failing verifiers without evaluating bounds when it is
missing or invalid. Replace direct JSON.parse comparisons with guarded
parse-success checks so malformed exactSchema or overSchema fixtures produce
"fail" rather than throwing. Ensure the conformance cases run through the
shipped MCP namespace/schema enforcement path, or add assertions proving that
runNamespaceRoundTrip and runCallResult exercise that path instead of only
round-tripping fixture data.
- Around line 135-150: Update the verifier assembly used by attachVerifiers so
MCP results produced by attachMcpVerifiers are merged into the existing
observation.verifiers object rather than replacing it. Preserve earlier values
such as exact_bound, one_over_rejected, and atomic while adding stub_received,
then route the MCP schema-and-bounds and call-result cases through runScenario
in the conformance harness so assertions exercise the full runner path.
In `@src/lab/conformance/negative-controls.ts`:
- Around line 150-158: Update buildNegativeControls around the control.mutate
call to remove inherited expectedFailure metadata from the cloned base case
before mutation, ensuring every generated control follows the ordinary
evaluation branch in runScenario while preserving the control’s mutated id,
assertions, and fixture fields.
- Around line 51-67: Update the result assertion in negative.tool-result-order
to use the Chat Completions tool-result call_id selector produced by
runToolRoundTrip and createOpenAIChatAdapter, replacing the Responses-specific
/upstream/requests/1/json/input/0/call_id path. Keep the assertion’s intended
comparison with /client/response/toolCalls/0/id so it reaches the wrong
toolResult.toolCallId case rather than selector_missing.
In `@src/lab/conformance/observation.ts`:
- Around line 158-178: Update finalizeObservation to resolve the effective
tool-call array once, using the event projection when non-empty and the JSON
output fallback otherwise. Pass that same resolved array to both the response’s
toolCalls field and projectMcpCalls so non-streaming JSON tool calls produce
matching MCP calls.
- Around line 324-339: Update deriveNormalizedText to call the existing
extractOutputText helper for the JSON output branch instead of duplicating the
output/content/output_text traversal, preserving the current normalized-text
behavior and using extractOutputText as the single projection rule.
- Line 176: The finalizeObservation flow must persist the actual client response
status instead of hardcoding status: 200, including for response.failed events,
so assertion handling can evaluate non-200 outcomes. Update finalizeObservation
and its response-status extraction to provide the real status before
http_status_equals in assertion.ts reads it; do not leave a fabricated default
that masks failed responses.
- Around line 99-112: Normalize MCP wire-name handling between mcp-stub
serialization and projectMcpCalls so conventional namespaces such as github
round-trip correctly. Update projectMcpCalls to parse the emitted
namespace__name format without requiring the namespace itself to begin with
mcp__, or otherwise align serialization and parsing on a single consistent
convention while preserving the returned namespace and name fields.
- Around line 220-238: The function evaluateCallResultOrder must correlate the
first function_call_output with the preceding function_call by call_id. Store
the function_call identifier as the pending call, require the first output to
have the same identifier before returning "pass", and return "fail" for
mismatches; preserve failure for outputs without a preceding call and ensure
later parallel pairs are not incorrectly treated as the validated pair.
In `@src/lab/conformance/runner.ts`:
- Around line 27-36: The shared ConformanceRunSummary contract ambiguously uses
passed for opposite meanings. Update runNegativeControls and its callers/tests
to expose the count of rejected negative controls under an explicit field, and
document each summary field’s semantics on the interface; also make
runConformanceSuite treat an empty discovered scenario set as an unsuccessful
run rather than returning a zero-success summary.
In `@src/lab/conformance/sse-normalize.ts`:
- Around line 34-45: Update the scalar-payload branch in the SSE normalization
logic around JSON.parse so parsed null and non-object values are preserved as
NormalizedEvent entries using the raw joined payload, matching the
malformed-JSON event behavior. Assign the same fallback event name and consume
an ordinal for these frames; retain the existing inferred event and
parsed-object data behavior for JSON objects.
In `@src/lab/conformance/types.ts`:
- Around line 155-161: Update CL01_SUITES to include the missing vision-core,
reasoning-core, and mcp-core suite names so discoverScenarios(),
runConformanceSuite(), and listScenarioIds() discover their fixture cases by
default; if any exclusion is intentional, document it explicitly instead.
In `@tests/lab-conformance-harness.test.ts`:
- Around line 172-178: Update the “listScenarioIds returns stable mapping” test
to call listScenarioIds() twice and compare the resulting arrays without
mutating either invocation, while retaining the expected count assertion. Assert
the order explicitly so changes in discoverScenarios ordering cause the test to
fail.
- Around line 103-113: Add focused tests in the existing “CL-01 SSE
normalization” describe block for normalizeSseBytes covering BOM removal, CRLF
and lone-CR normalization, comment-line skipping, multi-line data joining, and
malformed JSON producing an event named “malformed”. Ensure the malformed case
verifies the extra event required by negative.malformed-sse, while preserving
the existing protocol-specific [DONE] assertions.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 913566b9-3d89-42d6-87c2-eb4ece59fa01
📒 Files selected for processing (20)
devlog/_plan/260807_compatibility_lab/001_pr_stack_status.mddevlog/_plan/260807_compatibility_lab/051_cl01_acceptance_review.mdsrc/adapters/openai-chat.tssrc/lab/conformance/assertion.tssrc/lab/conformance/digest.tssrc/lab/conformance/executor.tssrc/lab/conformance/fixture-provider.tssrc/lab/conformance/fixtures/protocol-v1-cases.jsonsrc/lab/conformance/harness-budget.tssrc/lab/conformance/index.tssrc/lab/conformance/jcs.tssrc/lab/conformance/json-pointer.tssrc/lab/conformance/manifest.tssrc/lab/conformance/mcp-stub.tssrc/lab/conformance/negative-controls.tssrc/lab/conformance/observation.tssrc/lab/conformance/runner.tssrc/lab/conformance/sse-normalize.tssrc/lab/conformance/types.tstests/lab-conformance-harness.test.ts
Wibias
marked this pull request as ready for review
August 9, 2026 00:02
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (2)
src/lab/conformance/mcp-stub.ts (1)
89-116: 🎯 Functional Correctness | 🔵 Trivial | 🏗️ Heavy lift
one_over_rejectedstill measures the fixture, not any rejection.The
limitBytesvalidation at Line 94 and the guardedparsesJsonat Line 80 to Line 87 both resolve earlier findings. One half of that earlier finding remains open.Line 109 sets
one_over_rejectedto"pass"whenoverSchemais exactlylimit + 1bytes and parses as JSON. No code rejects anything. The check asserts two properties of a string that the fixture author wrote. Line 105 does the same forexactSchema. Line 113 hard-codespartial_commit: falserather than observing a commit.Failure mode:
mcp-core.protocol.schema-and-boundsasserts on/verifiers/exact_boundand/verifiers/one_over_rejected. Both verifiers pass whenever the fixture strings have the right byte lengths. If the shipped MCP schema-limit enforcement regresses, or is removed entirely, this case still reports"pass". The case cannot detect the regression it exists to detect.Route the two schemas through the shipped enforcement path so the verifier records what that code decided.
OcxProviderConfigalready declaresmcpMaxSchemaBytes; call the module that reads it, passexactSchemaandoverSchema, and setexact_boundandone_over_rejectedfrom its accept/reject outcome. If no such entry point is exported yet, mark this case unsupported in the manifest rather than shipping a verifier that always passes.I can draft the wiring once you confirm which module owns
mcpMaxSchemaBytesenforcement. Do you want me to open an issue to track it?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lab/conformance/mcp-stub.ts` around lines 89 - 116, The runSchemaBounds verifier currently validates fixture lengths instead of exercising enforcement. Route exactSchema and overSchema through the shipped mcpMaxSchemaBytes enforcement module, and derive exact_bound and one_over_rejected from its accept/reject results; if no callable entry point is exported, mark this case unsupported in the manifest rather than reporting unconditional passes.src/lab/conformance/observation.ts (1)
231-254: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
evaluateCallResultOrderrejects any turn with more than one tool call.The call-id correlation added here is correct and fixes the earlier mismatch gap. The new constraint is stricter than the verifier name implies.
Line 241 returns
"fail"as soon as a secondfunction_callappears, becausependingCallIdis still set. Line 250 returns"fail"on a secondfunction_call_output. Line 253 requires exactly one call and exactly one output. A conformant implementation that emits two parallel tool calls followed by their two matching outputs therefore reports"fail"forcall_result_order.This is not reachable today:
runCodexToolContinuationinsrc/lab/conformance/executor.tsLine 347 to Line 364 builds a single call/output pair fromturn1.outputandturn2.input, and thecodex-core.protocol.tool-continuationfixture carries one pair. The defect surfaces the first time a fixture models parallel tool calls, and it surfaces as a false negative against correct code.Track pending ids in a map so the check scales to parallel calls while keeping the correlation guarantee.
🐛 Proposed fix to support parallel call/result pairs
function evaluateCallResultOrder(observation: NormalizedObservation): string { const request = observation.upstream.requests[0]?.json as { input?: unknown[] } | undefined; const input = request?.input; if (!Array.isArray(input)) return "fail"; - let pendingCallId: string | undefined; - let resultCount = 0; + const pendingCallIds = new Set<string>(); + let callCount = 0; + let resultCount = 0; for (const item of input) { if (!item || typeof item !== "object") continue; const record = item as { type?: string; call_id?: unknown }; if (record.type === "function_call") { - if (pendingCallId !== undefined) return "fail"; if (typeof record.call_id !== "string" || record.call_id.length === 0) return "fail"; - pendingCallId = record.call_id; + if (pendingCallIds.has(record.call_id)) return "fail"; + pendingCallIds.add(record.call_id); + callCount++; continue; } if (record.type === "function_call_output") { - if (pendingCallId === undefined) return "fail"; - if (typeof record.call_id !== "string" || record.call_id !== pendingCallId) return "fail"; + if (typeof record.call_id !== "string" || !pendingCallIds.delete(record.call_id)) return "fail"; resultCount++; - if (resultCount > 1) return "fail"; } } - return pendingCallId !== undefined && resultCount === 1 ? "pass" : "fail"; + return callCount > 0 && resultCount === callCount && pendingCallIds.size === 0 ? "pass" : "fail"; }This keeps every rejection the current code performs: an output before its call fails at the
deletemiss, a mismatchedcall_idfails the same way, and an unanswered call fails the finalpendingCallIds.size === 0check.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lab/conformance/observation.ts` around lines 231 - 254, Update evaluateCallResultOrder to track all outstanding function_call IDs in a set or map instead of a single pendingCallId. Reject duplicate or invalid call IDs, reject outputs whose call_id is not outstanding, remove matched IDs after each output, and return "pass" only when at least one call was matched and no IDs remain pending.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/adapters/openai-chat.ts`:
- Around line 285-303: The native OpenAI developer-message barrier ordering is
not covered by the existing test. Update the relevant test in
tests/openai-chat-dangling-toolcalls.test.ts to use baseUrl
"https://api.openai.com/v1", ensuring isNativeOpenAIChatTarget() selects the
native path, and assert that the output preserves the adjacent assistant → tool
ordering while deferring the developer message until after pending tool results.
- Around line 706-708: Update the native-target branch in the reasoning body
construction around isNativeOpenAIChatTarget so direct OpenAI Chat Completions
requests use the top-level reasoning_effort field instead of the
gateway-specific reasoning object. Preserve the existing gateway-object format
for non-native providers, and ensure native targets do not emit the reasoning
object.
In `@src/lab/conformance/executor.ts`:
- Around line 634-637: Update the expected-failure validation in the
expectedFailure handling block to reject an empty assertionIds list before
evaluating controlPassed, throwing the existing harness failure used for
malformed controls so the surrounding try converts it into diagnostics. Preserve
the current every/find matching behavior for non-empty lists.
- Around line 585-597: Move the chat-core.protocol.nonstream-envelope handling
before the upstreamProtocol dispatch so it does not run the streaming adapter
first; remove the later duplicate branch that reparses the fixture and allocates
another translator budget. Set responseStatus from the fixture’s expected status
source rather than reading it from a locally constructed Response, and preserve
the fixture-derived response JSON and parsed events without manufacturing a
self-validated 200 status.
In `@src/lab/conformance/mcp-stub.ts`:
- Around line 40-46: Update runNamespaceRoundTrip and the MCP fixture
serialization to apply the parser’s mcp__ wire-name convention consistently:
ensure generated tool names use the mcp__ prefix before the namespace/name
components, while avoiding duplicate prefixes for namespaces that already begin
with mcp__. Preserve validation of non-empty namespace and name values.
In `@src/lab/conformance/observation.ts`:
- Around line 86-88: Make duplicate tool-call rejection observable instead of
returning the same empty result as a valid no-tool-call projection. Update the
relevant tool-call projection, including projectToolCallsFromEvents if it
applies, to preserve a duplicateToolCallIds signal, and have finalizeObservation
surface that signal as a verifier while retaining the rejected projection
behavior.
In `@tests/lab-conformance-harness.test.ts`:
- Around line 151-153: Update normalizeSseBytes to treat parsed JSON arrays as
Protocol V1 padding, alongside null and other excluded values, by normalizing
Array.isArray(parsed) to null before event inference. Preserve the existing
handling for scalar values and valid object events.
- Around line 183-191: Update the canonical selectors in protocol-v1-cases.json
for codex-core.protocol.streaming-turn and reasoning-core.protocol.replay to use
the actual SSE-normalized phase and replay text observation paths, respectively.
Restore the missing replay text check using the plan content/signature from the
upstream request content, while preserving the existing 35-scenario totals and
expectations in runConformanceSuite.
In `@tests/openai-chat-tool-result-images.test.ts`:
- Around line 86-91: Add a test case in the image tool-result tests with exactly
one valid image part and no text part, then locate its tool message and assert
its content matches the flattened fallback output from contentPartsToText. Keep
the existing mixed and empty-image cases unchanged.
---
Duplicate comments:
In `@src/lab/conformance/mcp-stub.ts`:
- Around line 89-116: The runSchemaBounds verifier currently validates fixture
lengths instead of exercising enforcement. Route exactSchema and overSchema
through the shipped mcpMaxSchemaBytes enforcement module, and derive exact_bound
and one_over_rejected from its accept/reject results; if no callable entry point
is exported, mark this case unsupported in the manifest rather than reporting
unconditional passes.
In `@src/lab/conformance/observation.ts`:
- Around line 231-254: Update evaluateCallResultOrder to track all outstanding
function_call IDs in a set or map instead of a single pendingCallId. Reject
duplicate or invalid call IDs, reject outputs whose call_id is not outstanding,
remove matched IDs after each output, and return "pass" only when at least one
call was matched and no IDs remain pending.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 72e1d1d5-6080-46a8-810f-6111a87bd9a1
📒 Files selected for processing (15)
src/adapters/openai-chat.tssrc/lab/conformance/assertion.tssrc/lab/conformance/executor.tssrc/lab/conformance/fixture-provider.tssrc/lab/conformance/harness-budget.tssrc/lab/conformance/jcs.tssrc/lab/conformance/json-pointer.tssrc/lab/conformance/manifest.tssrc/lab/conformance/mcp-stub.tssrc/lab/conformance/negative-controls.tssrc/lab/conformance/observation.tssrc/lab/conformance/runner.tssrc/lab/conformance/types.tstests/lab-conformance-harness.test.tstests/openai-chat-tool-result-images.test.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lab/conformance/executor.ts`:
- Around line 542-545: Update the response JSON selection near buildResponseJSON
so the parsed fixture body responseJson is used when adapter.parseResponse emits
no events. Check parsedEvents or the collected events for emptiness before
choosing reconstructed output, while preserving buildResponseJSON for non-empty
event streams.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4229dd5d-533e-486a-b141-825ca82d79ac
📒 Files selected for processing (3)
src/lab/conformance/executor.tstests/cl01-openai-chat-review-regressions.test.tstests/cl01-review-regressions.test.ts
Collaborator
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/lab/conformance/executor.ts (1)
542-546: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winUse the fixture JSON when
parseResponseemits no events.
buildResponseJSONalways returns aRecord<string, unknown>. The?? responseJsonfallback at Line 545 cannot run.If
parsedEventsis empty, the observation records harness-generated JSON instead of the fixture body. JSON assertions can then validate synthetic output. Select the JSON source fromparsedEvents.length. Add a regression test for an emptyparseResponseresult.Proposed fix
- const json = buildResponseJSON(parsedEvents, "fixture-model") as Record<string, unknown> ?? responseJson; + const json = parsedEvents.length > 0 + ? buildResponseJSON(parsedEvents, "fixture-model") + : responseJson;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lab/conformance/executor.ts` around lines 542 - 546, Update the response JSON selection in the executor flow to use responseJson when parsedEvents.length is zero, and call buildResponseJSON(parsedEvents, "fixture-model") only when events were emitted. Add a regression test covering an adapter whose parseResponse returns no events and verify assertions use the fixture body.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@src/lab/conformance/executor.ts`:
- Around line 542-546: Update the response JSON selection in the executor flow
to use responseJson when parsedEvents.length is zero, and call
buildResponseJSON(parsedEvents, "fixture-model") only when events were emitted.
Add a regression test covering an adapter whose parseResponse returns no events
and verify assertions use the fixture body.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a2bd3499-4a9e-4623-93de-295948ea62d8
📒 Files selected for processing (6)
src/adapters/openai-chat.tssrc/claude/outbound.tssrc/lab/conformance/executor.tstests/cl01-claude-outbound-review-regressions.test.tstests/cl01-openai-chat-review-regressions.test.tstests/cl01-review-regressions.test.ts
Contributor
|
✅ Deterministic PR hygiene checks passed. |
5 tasks
This was referenced Aug 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
src/lab/conformance/) with manifest loader, assertion DSL, SSE normalization, observation recording, scenario executor, negative controls, and runner.243c3f4905797aa11c62ba933bb03d6d721266fd).messages[]→ Responsesinput[]observation projection; observations record real upstream wire JSON./upstream/requests/N/json/messages/M/tool_call_id(tools-core.protocol.function-round-trip,codex-core.protocol.apply-patch-turn).[DONE]sentinel follows source protocol of the normalized byte stream (openai-chatonly).ocx-lab-synthetic-v1,lab_authored, authority file, source commit) with fail-closed manifest validation.mcp_namespace_round_trip_v1,mcp_schema_bounds_v1,mcp_call_result_v1,mcp_resource_round_trip_v1).openai-chat.tsalignment:toolResultTextForWireomits[image]marker when images flush to user carrier; developer wire role and single-tool requiredallowed_tools(from earlier CL-01 work).051_cl01_acceptance_review.md,001_pr_stack_status.md).Context: CL-01 was accepted earlier on a pre-remediation CL-00 tip; this PR is the contract-corrected and revalidated revision aligned with merged CL-00 #1286.
CL-02 not started.
Verification
bun x tsc --noEmit— passbun test tests/lab-conformance-harness.test.ts— 14/14 pass (24 canonical scenarios, 8 negative controls, provenance, SSE, MCP, manifest tests)bun run privacy:scan— passbun test tests/repo-hygiene.test.ts— 11/11 passbun test tests/bridge.test.ts tests/responses-parser.test.ts tests/chat-completions-endpoint.test.ts— 128/128 passgit diff --check— passbun run test— not run (documented Windows/Bun 1.3.14 baseline failures from CL-00 acceptance record)Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Documentation