mirror of
https://github.com/nexu-io/open-design.git
synced 2026-07-20 18:18:46 +08:00
fix: resolve P1 + P2 review feedback on Phase 10 (PR #1316)
Six fixes across the surfaces flagged by Codex, Siri-Ray, and lefarcen on PR #1316. 1. SSE variant guard rejects unknown enum values (lefarcen P2). hasValidVariantShape now checks role against PANELIST_ROLES and status / reason / cause / kind / decision against the contracts literal sets. A malformed frame with role='__proto__' or status='wat' is dropped before it reaches the reducer. 2. Replay transcript parser reuses the SSE variant guard (lefarcen + codex P2). parseTranscript in useCritiqueReplay now layers hasValidVariantShape on top of the shallow isPanelEvent check, so a corrupt transcript line like '{type:"ship",runId:"r"}' is dropped instead of crashing the reducer on composite.toFixed(). 3. Conformance harness treats any parser_warning as degraded (lefarcen P2). The classifier previously returned 'shipped' on the first ship event even if score_clamped / unknown_role / composite_mismatch / duplicate_ship warnings were emitted earlier. New behavior: any parser_warning in the stream classifies as degraded with local reason 'parser_warning' (mapped to contracts 'malformed_block' when marking the registry). 4. Conformance harness verifies panel completeness on ship (codex P2). Previously a ship event with only some panelists closed was accepted as shipped because the parser only enforces the round-1 designer artifact invariant. The harness now tracks closed roles vs the cast declared in run_started and returns degraded with local reason 'incomplete_panel' when any role missed close (mapped to contracts 'missing_artifact'). 5. Conformance tests for oversize_block and adapter-throws (lefarcen P2). Added the two cases the prior PR body claimed were covered but were not: oversize via a tight parserMaxBlockBytes on the good fixture, and an async iterable that throws mid-stream to drive the unexpected_error path. Two more tests cover the new parser_warning and incomplete_panel classifications. 9 vitest cases total, all green. 6. Interrupt dispatch waits for daemon ack (Siri-Ray + lefarcen P1). CritiqueTheaterMount used to optimistically dispatch 'interrupted' synchronously alongside the fetch, so a daemon that responded 404 or 409 (endpoint not wired, run already terminal) still moved the UI to the sticky interrupted phase and ignored every real terminal event. The new flow snapshots runId / bestRound / composite at click time, awaits the fetch, and only terminalizes on res.ok. On rejection or non-2xx, it clears interruptPending so the user can retry and the live SSE keeps emitting. 7. Native i18n key critiqueTheater.interruptedSummary backfilled in de / ja / ko / zh-TW (Siri-Ray P2). The other 12 locales inherit from en via spread, so they were already typecheck- safe; this commit gives the native locales native interrupted- summary strings instead of falling through to English. Tests: 16 daemon + 108 web Theater + locales suite all green; web typecheck clean.
This commit is contained in:
Nagendhra
@@ -19,9 +19,31 @@
|
||||
* Those policies live in the scheduler that calls this helper N times
|
||||
* across the adapter × template matrix; keeping the harness scoped to
|
||||
* a single run makes it testable in isolation.
|
||||
*
|
||||
* Classification rules (each row that matches wins, top to bottom):
|
||||
*
|
||||
* 1. The parser threw a MalformedBlockError / OversizeBlockError /
|
||||
* MissingArtifactError → degraded with that reason.
|
||||
* 2. The adapter source threw any other error → failed
|
||||
* (`unexpected_error`).
|
||||
* 3. The parser yielded a `parser_warning` event anywhere in the
|
||||
* stream → degraded (`parser_warning`). The parser tolerated a
|
||||
* soft violation (weak debate, unknown role, clamped score,
|
||||
* composite mismatch, duplicate ship) but the conformance gate
|
||||
* treats any of those as a "this adapter is not protocol-clean"
|
||||
* signal (lefarcen P2 on PR #1316).
|
||||
* 4. The parser yielded a `ship` event but the cast declared by
|
||||
* `run_started` did not all emit `panelist_close` → degraded
|
||||
* (`incomplete_panel`). The parser only enforces the round-1
|
||||
* designer-artifact invariant; the harness is what catches a
|
||||
* ship that skipped panelists or never scored them (codex P2 on
|
||||
* PR #1316).
|
||||
* 5. The parser yielded a `ship` event with a complete panel and no
|
||||
* parser warnings → shipped.
|
||||
* 6. The stream ended without a `ship` event → failed (`no_ship`).
|
||||
*/
|
||||
|
||||
import type { PanelEvent } from '@open-design/contracts/critique';
|
||||
import type { DegradedReason, PanelEvent, PanelistRole } from '@open-design/contracts/critique';
|
||||
|
||||
import { parseCritiqueStream, type ShipArtifactPayload } from './parser.js';
|
||||
import {
|
||||
@@ -34,9 +56,24 @@ import {
|
||||
markDegraded,
|
||||
} from './adapter-degraded.js';
|
||||
|
||||
/**
|
||||
* Local degraded reasons. `'parser_warning'` and `'incomplete_panel'`
|
||||
* are conformance-harness-only: they describe a stream the contracts
|
||||
* `DegradedReason` does not currently model as a discrete value. The
|
||||
* adapter-degraded registry stores the closest contracts reason via
|
||||
* `toContractReason` below, so a downstream listing of degraded
|
||||
* adapters still uses the wire-shape enum.
|
||||
*/
|
||||
export type ConformanceDegradedReason =
|
||||
| 'malformed_block'
|
||||
| 'oversize_block'
|
||||
| 'missing_artifact'
|
||||
| 'parser_warning'
|
||||
| 'incomplete_panel';
|
||||
|
||||
export type ConformanceOutcome =
|
||||
| { kind: 'shipped'; round: number; composite: number; events: PanelEvent[] }
|
||||
| { kind: 'degraded'; reason: 'malformed_block' | 'oversize_block' | 'missing_artifact'; events: PanelEvent[] }
|
||||
| { kind: 'degraded'; reason: ConformanceDegradedReason; events: PanelEvent[] }
|
||||
| { kind: 'failed'; cause: 'no_ship' | 'unexpected_error'; events: PanelEvent[]; error?: string };
|
||||
|
||||
export interface RunConformanceParams {
|
||||
@@ -48,6 +85,22 @@ export interface RunConformanceParams {
|
||||
artifactId?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Map a local conformance reason back to a contracts `DegradedReason`
|
||||
* so the adapter-degraded registry stays consistent with the wire
|
||||
* enum. Parser warnings collapse to `malformed_block` (the closest
|
||||
* "the stream is not well-formed" reason) and an incomplete panel
|
||||
* collapses to `missing_artifact` (the closest "required pieces
|
||||
* absent" reason).
|
||||
*/
|
||||
function toContractReason(r: ConformanceDegradedReason): DegradedReason {
|
||||
switch (r) {
|
||||
case 'parser_warning': return 'malformed_block';
|
||||
case 'incomplete_panel': return 'missing_artifact';
|
||||
default: return r;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Run a synthetic (or recorded) adapter source through the parser and
|
||||
* classify the outcome. Side-effect: when the outcome is `degraded`,
|
||||
@@ -61,6 +114,9 @@ export async function runAdapterConformance(
|
||||
): Promise<ConformanceOutcome> {
|
||||
const events: PanelEvent[] = [];
|
||||
let shipPayload: ShipArtifactPayload | null = null;
|
||||
let parserWarningSeen = false;
|
||||
let castRoles: PanelistRole[] | null = null;
|
||||
const closedRoles = new Set<string>();
|
||||
|
||||
try {
|
||||
for await (const event of parseCritiqueStream(params.source, {
|
||||
@@ -74,7 +130,24 @@ export async function runAdapterConformance(
|
||||
},
|
||||
})) {
|
||||
events.push(event);
|
||||
if (event.type === 'ship') {
|
||||
if (event.type === 'run_started') {
|
||||
castRoles = event.cast;
|
||||
} else if (event.type === 'panelist_close') {
|
||||
closedRoles.add(event.role);
|
||||
} else if (event.type === 'parser_warning') {
|
||||
parserWarningSeen = true;
|
||||
} else if (event.type === 'ship') {
|
||||
// Tightening over "ship event seen = shipped": any earlier
|
||||
// parser_warning makes the stream non-clean (lefarcen P2);
|
||||
// an incomplete cast makes the ship premature (codex P2).
|
||||
if (parserWarningSeen) {
|
||||
return mark(params.adapterId, 'parser_warning', events);
|
||||
}
|
||||
const expected = castRoles ?? ['designer', 'critic', 'brand', 'a11y', 'copy'];
|
||||
const missing = expected.filter((r) => !closedRoles.has(r));
|
||||
if (missing.length > 0) {
|
||||
return mark(params.adapterId, 'incomplete_panel', events);
|
||||
}
|
||||
return {
|
||||
kind: 'shipped',
|
||||
round: event.round,
|
||||
@@ -90,8 +163,7 @@ export async function runAdapterConformance(
|
||||
: err instanceof MissingArtifactError ? 'missing_artifact'
|
||||
: null;
|
||||
if (reason) {
|
||||
markDegraded(params.adapterId, reason, ADAPTER_DEGRADED_DEFAULT_TTL_MS, 'conformance');
|
||||
return { kind: 'degraded', reason, events };
|
||||
return mark(params.adapterId, reason, events);
|
||||
}
|
||||
return {
|
||||
kind: 'failed',
|
||||
@@ -108,3 +180,17 @@ export async function runAdapterConformance(
|
||||
|
||||
return { kind: 'failed', cause: 'no_ship', events };
|
||||
}
|
||||
|
||||
function mark(
|
||||
adapterId: string,
|
||||
reason: ConformanceDegradedReason,
|
||||
events: PanelEvent[],
|
||||
): ConformanceOutcome {
|
||||
markDegraded(
|
||||
adapterId,
|
||||
toContractReason(reason),
|
||||
ADAPTER_DEGRADED_DEFAULT_TTL_MS,
|
||||
'conformance',
|
||||
);
|
||||
return { kind: 'degraded', reason, events };
|
||||
}
|
||||
|
||||
@@ -118,4 +118,120 @@ describe('adapter conformance harness (Phase 10)', () => {
|
||||
expect(ship.artifactRef.projectId).toBe('proj-conformance');
|
||||
expect(ship.artifactRef.artifactId).toBe('artifact-conformance');
|
||||
});
|
||||
|
||||
it('classifies an oversize block as degraded oversize_block (lefarcen P2)', async () => {
|
||||
// The synthetic-good transcript is fine under the default 256 KB
|
||||
// block budget. Replay it through the harness with a tiny budget so
|
||||
// the parser throws OversizeBlockError on the first ARTIFACT body
|
||||
// and the harness has to surface `degraded: oversize_block`.
|
||||
const outcome = await runAdapterConformance({
|
||||
adapterId: 'synthetic-oversize',
|
||||
runId: 'run-oversize',
|
||||
source: syntheticGoodStream(),
|
||||
parserMaxBlockBytes: 256,
|
||||
});
|
||||
expect(outcome.kind).toBe('degraded');
|
||||
if (outcome.kind !== 'degraded') return;
|
||||
expect(outcome.reason).toBe('oversize_block');
|
||||
expect(isDegraded('synthetic-oversize')).toBe(true);
|
||||
});
|
||||
|
||||
it('classifies an adapter that throws mid-stream as failed unexpected_error (lefarcen P2)', async () => {
|
||||
class AdapterBoom extends Error {
|
||||
constructor() {
|
||||
super('adapter blew up');
|
||||
this.name = 'AdapterBoom';
|
||||
}
|
||||
}
|
||||
async function* throwing(): AsyncIterable<string> {
|
||||
yield '<CRITIQUE_RUN version="1" runId="run-boom" projectId="p" artifactId="a">\n';
|
||||
yield '<ROUND n="1">\n';
|
||||
throw new AdapterBoom();
|
||||
}
|
||||
const outcome = await runAdapterConformance({
|
||||
adapterId: 'synthetic-throwing',
|
||||
runId: 'run-boom',
|
||||
source: throwing(),
|
||||
});
|
||||
expect(outcome.kind).toBe('failed');
|
||||
if (outcome.kind !== 'failed') return;
|
||||
expect(outcome.cause).toBe('unexpected_error');
|
||||
expect(outcome.error).toContain('adapter blew up');
|
||||
// The adapter is NOT marked degraded here: an unexpected throw is a
|
||||
// failure to evaluate, not evidence of a malformed stream. A real
|
||||
// policy could choose to mark it after N consecutive throws; the
|
||||
// harness leaves that decision to the caller.
|
||||
expect(isDegraded('synthetic-throwing')).toBe(false);
|
||||
});
|
||||
|
||||
it('classifies a clean SHIP that arrived alongside parser warnings as degraded parser_warning (lefarcen P2)', async () => {
|
||||
// A panelist score outside [0, scale] makes the parser yield a
|
||||
// `parser_warning` with kind=`score_clamped` BEFORE the panelist
|
||||
// closes. The harness must promote the run to degraded even though
|
||||
// a syntactically valid SHIP arrives later.
|
||||
async function* withClampedScore(): AsyncIterable<string> {
|
||||
yield '<CRITIQUE_RUN version="1" maxRounds="1" threshold="0.1" scale="10">\n';
|
||||
yield ' <ROUND n="1">\n';
|
||||
// designer must include an ARTIFACT in round 1 (parser invariant).
|
||||
yield ' <PANELIST role="designer">\n';
|
||||
yield ' <ARTIFACT mime="text/html"><![CDATA[<p>x</p>]]></ARTIFACT>\n';
|
||||
yield ' </PANELIST>\n';
|
||||
// Out-of-range score on `critic` triggers score_clamped warning.
|
||||
yield ' <PANELIST role="critic" score="99"><DIM name="x" score="6">n</DIM></PANELIST>\n';
|
||||
yield ' <PANELIST role="brand" score="6"><DIM name="x" score="6">n</DIM></PANELIST>\n';
|
||||
yield ' <PANELIST role="a11y" score="6"><DIM name="x" score="6">n</DIM></PANELIST>\n';
|
||||
yield ' <PANELIST role="copy" score="6"><DIM name="x" score="6">n</DIM></PANELIST>\n';
|
||||
yield ' <ROUND_END n="1" composite="6.0" must_fix="0" decision="ship">\n';
|
||||
yield ' <REASON>ok</REASON>\n';
|
||||
yield ' </ROUND_END>\n';
|
||||
yield ' </ROUND>\n';
|
||||
yield ' <SHIP round="1" composite="6.0" status="shipped">\n';
|
||||
yield ' <ARTIFACT mime="text/html"><![CDATA[<p>final</p>]]></ARTIFACT>\n';
|
||||
yield ' <SUMMARY>ok</SUMMARY>\n';
|
||||
yield ' </SHIP>\n';
|
||||
yield '</CRITIQUE_RUN>\n';
|
||||
}
|
||||
const outcome = await runAdapterConformance({
|
||||
adapterId: 'synthetic-warned',
|
||||
runId: 'run-warned',
|
||||
source: withClampedScore(),
|
||||
});
|
||||
expect(outcome.kind).toBe('degraded');
|
||||
if (outcome.kind !== 'degraded') return;
|
||||
expect(outcome.reason).toBe('parser_warning');
|
||||
expect(outcome.events.some((e) => e.type === 'parser_warning')).toBe(true);
|
||||
expect(isDegraded('synthetic-warned')).toBe(true);
|
||||
});
|
||||
|
||||
it('classifies a SHIP that arrived before every panelist closed as degraded incomplete_panel (codex P2)', async () => {
|
||||
// run_started declares the full 5-role cast, but only `designer`
|
||||
// and `critic` ever emit panelist_close. The parser does not reject
|
||||
// this on its own; the harness is the gate that catches it.
|
||||
async function* incomplete(): AsyncIterable<string> {
|
||||
yield '<CRITIQUE_RUN version="1" maxRounds="1" threshold="0.1" scale="10">\n';
|
||||
yield ' <ROUND n="1">\n';
|
||||
yield ' <PANELIST role="designer">\n';
|
||||
yield ' <ARTIFACT mime="text/html"><![CDATA[<p>x</p>]]></ARTIFACT>\n';
|
||||
yield ' </PANELIST>\n';
|
||||
yield ' <PANELIST role="critic" score="6"><DIM name="x" score="6">n</DIM></PANELIST>\n';
|
||||
yield ' <ROUND_END n="1" composite="6.0" must_fix="0" decision="ship">\n';
|
||||
yield ' <REASON>ok</REASON>\n';
|
||||
yield ' </ROUND_END>\n';
|
||||
yield ' </ROUND>\n';
|
||||
yield ' <SHIP round="1" composite="6.0" status="shipped">\n';
|
||||
yield ' <ARTIFACT mime="text/html"><![CDATA[<p>final</p>]]></ARTIFACT>\n';
|
||||
yield ' <SUMMARY>ok</SUMMARY>\n';
|
||||
yield ' </SHIP>\n';
|
||||
yield '</CRITIQUE_RUN>\n';
|
||||
}
|
||||
const outcome = await runAdapterConformance({
|
||||
adapterId: 'synthetic-incomplete',
|
||||
runId: 'run-incomplete',
|
||||
source: incomplete(),
|
||||
});
|
||||
expect(outcome.kind).toBe('degraded');
|
||||
if (outcome.kind !== 'degraded') return;
|
||||
expect(outcome.reason).toBe('incomplete_panel');
|
||||
expect(isDegraded('synthetic-incomplete')).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -71,21 +71,48 @@ export function CritiqueTheaterMount({
|
||||
if (interruptPending) return;
|
||||
if (state.phase !== 'running') return;
|
||||
if (!projectId) return;
|
||||
|
||||
// Snapshot the state values at click time. The fetch is async; by the
|
||||
// time it resolves a fresh run could have started and `state.runId`
|
||||
// could refer to a different run. The dispatch must carry the runId
|
||||
// the user clicked on, not the latest one.
|
||||
const runId = state.runId;
|
||||
const bestRound = bestRoundOf(state);
|
||||
const composite = bestCompositeOf(state);
|
||||
|
||||
setInterruptPending(true);
|
||||
|
||||
// Lefarcen + codex P1 on PR #1315: the previous revision did the
|
||||
// optimistic local dispatch only, so the daemon-side run kept
|
||||
// executing while the UI ignored the real terminal event. Fire
|
||||
// the daemon kill request alongside the optimistic dispatch.
|
||||
// Daemon contract: `POST /api/projects/:id/critique/:runId/interrupt`
|
||||
// returns 204 on success, 404 if the endpoint has not been wired
|
||||
// up yet (Phase 15). Either response is treated as a best-effort
|
||||
// signal: the UI moves to `interrupted` because the user asked
|
||||
// for it; any real outcome the daemon emits later is dropped
|
||||
// because terminal phases are sticky in the reducer.
|
||||
// Siri-Ray + lefarcen P1 on PR #1316: the prior revision dispatched
|
||||
// `interrupted` synchronously alongside the fetch, so a daemon that
|
||||
// returned 404 / 409 (endpoint not wired, run already finished)
|
||||
// still moved the UI to the sticky `interrupted` terminal phase and
|
||||
// ignored every real terminal event the daemon emitted later. The
|
||||
// new flow waits for the daemon ack: only on a successful response
|
||||
// (HTTP 2xx) do we mark the run interrupted locally. On rejection,
|
||||
// we clear `interruptPending` so the user can retry, and the real
|
||||
// SSE terminal event the daemon emits later still wins.
|
||||
const fetcher = fetchInterrupt ?? ((url, init) => fetch(url, init));
|
||||
const url = `/api/projects/${encodeURIComponent(projectId)}/critique/${encodeURIComponent(state.runId)}/interrupt`;
|
||||
fetcher(url, { method: 'POST' }).catch((err) => {
|
||||
const url = `/api/projects/${encodeURIComponent(projectId)}/critique/${encodeURIComponent(runId)}/interrupt`;
|
||||
fetcher(url, { method: 'POST' }).then((res) => {
|
||||
if (res.ok) {
|
||||
dispatch({ type: 'interrupted', runId, bestRound, composite });
|
||||
return;
|
||||
}
|
||||
// Daemon rejected the request (e.g. 404 endpoint not wired,
|
||||
// 409 run already terminal). Surface the error in dev and let
|
||||
// the user retry; do NOT terminalize the UI.
|
||||
setInterruptPending(false);
|
||||
if (
|
||||
typeof process !== 'undefined'
|
||||
&& process.env?.NODE_ENV === 'development'
|
||||
) {
|
||||
// eslint-disable-next-line no-console
|
||||
console.warn(
|
||||
`[critique-theater] interrupt rejected by daemon (HTTP ${res.status})`,
|
||||
);
|
||||
}
|
||||
}).catch((err) => {
|
||||
setInterruptPending(false);
|
||||
if (
|
||||
typeof process !== 'undefined'
|
||||
&& process.env?.NODE_ENV === 'development'
|
||||
@@ -94,13 +121,6 @@ export function CritiqueTheaterMount({
|
||||
console.warn('[critique-theater] interrupt request failed', err);
|
||||
}
|
||||
});
|
||||
|
||||
dispatch({
|
||||
type: 'interrupted',
|
||||
runId: state.runId,
|
||||
bestRound: bestRoundOf(state),
|
||||
composite: bestCompositeOf(state),
|
||||
});
|
||||
}, [interruptPending, state, dispatch, projectId, fetchInterrupt]);
|
||||
|
||||
if (!enabled) return null;
|
||||
|
||||
@@ -9,6 +9,7 @@ import {
|
||||
type CritiqueAction,
|
||||
type CritiqueState,
|
||||
} from '../state/reducer';
|
||||
import { hasValidVariantShape } from '../state/sse';
|
||||
|
||||
export type ReplaySpeed = 'paused' | 'instant' | 'live' | { intervalMs: number };
|
||||
|
||||
@@ -240,7 +241,13 @@ function parseTranscript(raw: string): PanelEvent[] {
|
||||
if (!trimmed) continue;
|
||||
try {
|
||||
const parsed = JSON.parse(trimmed);
|
||||
if (isPanelEvent(parsed)) out.push(parsed);
|
||||
// Match the live SSE path: pass both the shallow contracts predicate
|
||||
// (known `type`, non-empty `runId`) and the variant-level guard
|
||||
// (required fields, enum membership for role / status / cause /
|
||||
// reason / kind). A corrupt transcript line like `{type:'ship',
|
||||
// runId:'r'}` would otherwise be dispatched and crash the reducer
|
||||
// on `composite.toFixed()` (lefarcen + codex P2 on PR #1316).
|
||||
if (isPanelEvent(parsed) && hasValidVariantShape(parsed)) out.push(parsed);
|
||||
} catch {
|
||||
// Tolerate stray lines; the orchestrator writes one event per line so
|
||||
// a bad line is recoverable. Production loggers should record the
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import {
|
||||
CRITIQUE_SSE_EVENT_NAMES,
|
||||
PANELIST_ROLES,
|
||||
isPanelEvent,
|
||||
type CritiqueSseEvent,
|
||||
type CritiqueSseEventName,
|
||||
@@ -8,6 +9,39 @@ import {
|
||||
|
||||
import type { CritiqueAction } from './reducer';
|
||||
|
||||
// Runtime literal sets for the union types `contracts` only exports as
|
||||
// TypeScript types (`ShipStatus`, `DegradedReason`, `FailedCause`,
|
||||
// `ParserWarningKind`). The variant guard below uses these to reject
|
||||
// frames whose enum-shaped field is unknown, e.g. a malformed or
|
||||
// compromised frame with `role: "__proto__"` or `status: "wat"`. The
|
||||
// arrays must stay in sync with the type unions in
|
||||
// `packages/contracts/src/critique.ts`. The locale alignment test plus
|
||||
// the surface coverage walker in apps/web/tests/components/Theater/
|
||||
// catch drift between the two on the next CI run.
|
||||
const SHIP_STATUSES = new Set<string>(['shipped', 'below_threshold', 'timed_out', 'interrupted']);
|
||||
const DEGRADED_REASONS = new Set<string>([
|
||||
'malformed_block',
|
||||
'oversize_block',
|
||||
'adapter_unsupported',
|
||||
'protocol_version_mismatch',
|
||||
'missing_artifact',
|
||||
]);
|
||||
const FAILED_CAUSES = new Set<string>([
|
||||
'cli_exit_nonzero',
|
||||
'per_round_timeout',
|
||||
'total_timeout',
|
||||
'orchestrator_internal',
|
||||
]);
|
||||
const PARSER_WARNING_KINDS = new Set<string>([
|
||||
'weak_debate',
|
||||
'unknown_role',
|
||||
'score_clamped',
|
||||
'composite_mismatch',
|
||||
'duplicate_ship',
|
||||
]);
|
||||
const ROUND_DECISIONS = new Set<string>(['continue', 'ship']);
|
||||
const PANELIST_ROLE_SET = new Set<string>(PANELIST_ROLES);
|
||||
|
||||
export interface CritiqueEventsConnection {
|
||||
close(): void;
|
||||
}
|
||||
@@ -56,56 +90,63 @@ export function critiqueEventsUrl(projectId: string): string {
|
||||
* `{ type: 'ship', runId: 'r' }` would slip through to the reducer with every
|
||||
* other field undefined and crash downstream code that calls
|
||||
* `final.composite.toFixed(1)`. This second-pass filter enforces the shape
|
||||
* of each variant before the action is dispatched (lefarcen + Siri-Ray +
|
||||
* codex P2 on PR #1314). */
|
||||
function hasValidVariantShape(event: PanelEvent): boolean {
|
||||
* AND the enum membership of each variant before the action is dispatched
|
||||
* (lefarcen + Siri-Ray + codex P2 on PR #1314, plus lefarcen P2 on PR
|
||||
* #1316 re: a malformed `role: "__proto__"` frame walking past the
|
||||
* string-only check and indexing Object.prototype as a panelist view).
|
||||
* Exported so the replay-transcript parser in `useCritiqueReplay`
|
||||
* applies the same rejection rules as the live SSE path. */
|
||||
export function hasValidVariantShape(event: PanelEvent): boolean {
|
||||
switch (event.type) {
|
||||
case 'run_started':
|
||||
return typeof event.protocolVersion === 'number'
|
||||
&& Array.isArray(event.cast) && event.cast.length > 0
|
||||
&& event.cast.every((r) => typeof r === 'string')
|
||||
&& event.cast.every((r) => typeof r === 'string' && PANELIST_ROLE_SET.has(r))
|
||||
&& typeof event.maxRounds === 'number'
|
||||
&& typeof event.threshold === 'number'
|
||||
&& typeof event.scale === 'number';
|
||||
case 'panelist_open':
|
||||
return typeof event.round === 'number' && typeof event.role === 'string';
|
||||
return typeof event.round === 'number'
|
||||
&& typeof event.role === 'string' && PANELIST_ROLE_SET.has(event.role);
|
||||
case 'panelist_dim':
|
||||
return typeof event.round === 'number'
|
||||
&& typeof event.role === 'string'
|
||||
&& typeof event.role === 'string' && PANELIST_ROLE_SET.has(event.role)
|
||||
&& typeof event.dimName === 'string'
|
||||
&& typeof event.dimScore === 'number'
|
||||
&& typeof event.dimNote === 'string';
|
||||
case 'panelist_must_fix':
|
||||
return typeof event.round === 'number'
|
||||
&& typeof event.role === 'string'
|
||||
&& typeof event.role === 'string' && PANELIST_ROLE_SET.has(event.role)
|
||||
&& typeof event.text === 'string';
|
||||
case 'panelist_close':
|
||||
return typeof event.round === 'number'
|
||||
&& typeof event.role === 'string'
|
||||
&& typeof event.role === 'string' && PANELIST_ROLE_SET.has(event.role)
|
||||
&& typeof event.score === 'number';
|
||||
case 'round_end':
|
||||
return typeof event.round === 'number'
|
||||
&& typeof event.composite === 'number'
|
||||
&& typeof event.mustFix === 'number'
|
||||
&& (event.decision === 'continue' || event.decision === 'ship')
|
||||
&& typeof event.decision === 'string' && ROUND_DECISIONS.has(event.decision)
|
||||
&& typeof event.reason === 'string';
|
||||
case 'ship':
|
||||
return typeof event.round === 'number'
|
||||
&& typeof event.composite === 'number'
|
||||
&& typeof event.status === 'string'
|
||||
&& typeof event.status === 'string' && SHIP_STATUSES.has(event.status)
|
||||
&& event.artifactRef !== null
|
||||
&& typeof event.artifactRef === 'object'
|
||||
&& typeof (event.artifactRef as { projectId?: unknown }).projectId === 'string'
|
||||
&& typeof (event.artifactRef as { artifactId?: unknown }).artifactId === 'string'
|
||||
&& typeof event.summary === 'string';
|
||||
case 'degraded':
|
||||
return typeof event.reason === 'string' && typeof event.adapter === 'string';
|
||||
return typeof event.reason === 'string' && DEGRADED_REASONS.has(event.reason)
|
||||
&& typeof event.adapter === 'string';
|
||||
case 'interrupted':
|
||||
return typeof event.bestRound === 'number' && typeof event.composite === 'number';
|
||||
case 'failed':
|
||||
return typeof event.cause === 'string';
|
||||
return typeof event.cause === 'string' && FAILED_CAUSES.has(event.cause);
|
||||
case 'parser_warning':
|
||||
return typeof event.kind === 'string' && typeof event.position === 'number';
|
||||
return typeof event.kind === 'string' && PARSER_WARNING_KINDS.has(event.kind)
|
||||
&& typeof event.position === 'number';
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -899,6 +899,7 @@ export const de: Dict = {
|
||||
'critiqueTheater.replaySpeed': 'Wiedergabegeschwindigkeit',
|
||||
'critiqueTheater.readOnly': 'Nur lesen',
|
||||
'critiqueTheater.shippedSummary': 'In Runde {round} freigegeben · Gesamtscore {composite}',
|
||||
'critiqueTheater.interruptedSummary': 'In Runde {round} unterbrochen · bester Gesamtscore {composite}',
|
||||
'critiqueTheater.shippedBadge': 'Freigegeben',
|
||||
'critiqueTheater.belowThresholdBadge': 'Unter der Schwelle',
|
||||
'critiqueTheater.timedOutBadge': 'Zeitüberschreitung',
|
||||
|
||||
@@ -898,6 +898,7 @@ export const ja: Dict = {
|
||||
'critiqueTheater.replaySpeed': 'リプレイ速度',
|
||||
'critiqueTheater.readOnly': '読み取り専用',
|
||||
'critiqueTheater.shippedSummary': 'ラウンド {round} で出荷 · 総合 {composite}',
|
||||
'critiqueTheater.interruptedSummary': 'ラウンド {round} で中断 · 最高総合 {composite}',
|
||||
'critiqueTheater.shippedBadge': '出荷済み',
|
||||
'critiqueTheater.belowThresholdBadge': 'しきい値未達',
|
||||
'critiqueTheater.timedOutBadge': 'タイムアウト',
|
||||
|
||||
@@ -1011,6 +1011,7 @@ export const ko: Dict = {
|
||||
'critiqueTheater.replaySpeed': '리플레이 속도',
|
||||
'critiqueTheater.readOnly': '읽기 전용',
|
||||
'critiqueTheater.shippedSummary': '{round} 라운드에서 배포 · 종합 {composite}',
|
||||
'critiqueTheater.interruptedSummary': '{round} 라운드에서 중단 · 최고 종합 {composite}',
|
||||
'critiqueTheater.shippedBadge': '배포됨',
|
||||
'critiqueTheater.belowThresholdBadge': '임계값 미달',
|
||||
'critiqueTheater.timedOutBadge': '시간 초과',
|
||||
|
||||
@@ -1076,6 +1076,7 @@ export const zhTW: Dict = {
|
||||
'critiqueTheater.replaySpeed': '回放速度',
|
||||
'critiqueTheater.readOnly': '唯讀',
|
||||
'critiqueTheater.shippedSummary': '已於第 {round} 輪發布 · 綜合分 {composite}',
|
||||
'critiqueTheater.interruptedSummary': '已於第 {round} 輪中斷 · 最佳綜合分 {composite}',
|
||||
'critiqueTheater.shippedBadge': '已發布',
|
||||
'critiqueTheater.belowThresholdBadge': '低於門檻',
|
||||
'critiqueTheater.timedOutBadge': '已逾時',
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
// @vitest-environment jsdom
|
||||
|
||||
import { act, cleanup, fireEvent, render, screen } from '@testing-library/react';
|
||||
import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react';
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
import { CritiqueTheaterMount } from '../../../src/components/Theater/CritiqueTheaterMount';
|
||||
@@ -66,9 +66,16 @@ describe('<CritiqueTheaterMount> (Phase 9.1)', () => {
|
||||
expect(screen.getByRole('region').getAttribute('data-phase')).toBe('running');
|
||||
});
|
||||
|
||||
it('flips the kill button to pending and synthesizes interrupted on click', () => {
|
||||
it('flips the kill button to pending and synthesizes interrupted on click', async () => {
|
||||
const { factory, handles } = makeFactory();
|
||||
render(<CritiqueTheaterMount projectId="p-1" enabled connectionFactory={factory} />);
|
||||
render(
|
||||
<CritiqueTheaterMount
|
||||
projectId="p-1"
|
||||
enabled
|
||||
connectionFactory={factory}
|
||||
fetchInterrupt={vi.fn(async () => new Response(null, { status: 204 }))}
|
||||
/>,
|
||||
);
|
||||
act(() => {
|
||||
handles[0]!.send({
|
||||
type: 'run_started',
|
||||
@@ -92,9 +99,12 @@ describe('<CritiqueTheaterMount> (Phase 9.1)', () => {
|
||||
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Interrupt' }));
|
||||
|
||||
// Phase flips to interrupted -> collapsed surface mounts in place
|
||||
// of the live stage.
|
||||
expect(screen.getByRole('status').getAttribute('data-phase')).toBe('interrupted');
|
||||
// Daemon ack lands asynchronously: the dispatch waits for the
|
||||
// POST response before terminalizing (Siri-Ray + lefarcen P1 on
|
||||
// PR #1316). Wait for the collapsed surface to appear.
|
||||
await waitFor(() => {
|
||||
expect(screen.getByRole('status').getAttribute('data-phase')).toBe('interrupted');
|
||||
});
|
||||
expect(screen.getByText('Interrupted')).toBeTruthy();
|
||||
});
|
||||
|
||||
@@ -151,10 +161,13 @@ describe('<CritiqueTheaterMount> (Phase 9.1)', () => {
|
||||
expect(fetchCalls[0]!.init.method).toBe('POST');
|
||||
});
|
||||
|
||||
it('swallows a rejected interrupt fetch and still moves the UI to interrupted', () => {
|
||||
// If the daemon endpoint has not landed yet (Phase 15), the
|
||||
// user's click should still flip the UI; the warning surfaces
|
||||
// on the dev console rather than tearing the React tree.
|
||||
it('leaves the UI in running phase when the daemon rejects the interrupt (Siri-Ray P1 on PR #1316)', async () => {
|
||||
// Previously a rejected fetch (network error OR 404 / 409 from
|
||||
// the daemon) still optimistically terminalized the run, which
|
||||
// meant a real later SSE event for the same run was ignored by
|
||||
// the sticky `interrupted` phase. The new flow only terminalizes
|
||||
// on a successful daemon ack; rejection clears interruptPending
|
||||
// so the user can retry and the live run keeps emitting SSE.
|
||||
const { factory, handles } = makeFactory();
|
||||
const fetchInterrupt = vi.fn(async () => {
|
||||
throw new Error('boom');
|
||||
@@ -179,12 +192,53 @@ describe('<CritiqueTheaterMount> (Phase 9.1)', () => {
|
||||
});
|
||||
});
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Interrupt' }));
|
||||
// Optimistic dispatch already fired so the collapsed surface
|
||||
// mounts in place of the live stage.
|
||||
expect(screen.getByRole('status').getAttribute('data-phase')).toBe('interrupted');
|
||||
// Wait for the rejected promise to resolve through the catch
|
||||
// handler. The button should clear back to enabled, and the
|
||||
// theater should NOT have terminalized.
|
||||
await waitFor(() => {
|
||||
const btn = screen.getByRole('button', { name: 'Interrupt' }) as HTMLButtonElement;
|
||||
expect(btn.disabled).toBe(false);
|
||||
});
|
||||
expect(screen.getByRole('region').getAttribute('data-phase')).toBe('running');
|
||||
});
|
||||
|
||||
it('resets interruptPending when a fresh run starts after an interrupt (codex P2 on PR #1315)', () => {
|
||||
it('leaves the UI in running phase when the daemon returns a non-2xx response (Siri-Ray P1 on PR #1316)', async () => {
|
||||
// Same as the rejection case but for an HTTP failure: fetch
|
||||
// resolves with `ok: false` instead of throwing, and the old
|
||||
// flow swallowed that too. The new flow treats every non-2xx
|
||||
// as "do not terminalize".
|
||||
const { factory, handles } = makeFactory();
|
||||
const fetchInterrupt = vi.fn(
|
||||
async () => new Response(null, { status: 404 }),
|
||||
);
|
||||
render(
|
||||
<CritiqueTheaterMount
|
||||
projectId="proj-1"
|
||||
enabled
|
||||
connectionFactory={factory}
|
||||
fetchInterrupt={fetchInterrupt}
|
||||
/>,
|
||||
);
|
||||
act(() => {
|
||||
handles[0]!.send({
|
||||
type: 'run_started',
|
||||
runId: 'run-abc',
|
||||
protocolVersion: 1,
|
||||
cast: ['critic'],
|
||||
maxRounds: 3,
|
||||
threshold: 8,
|
||||
scale: 10,
|
||||
});
|
||||
});
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Interrupt' }));
|
||||
await waitFor(() => {
|
||||
const btn = screen.getByRole('button', { name: 'Interrupt' }) as HTMLButtonElement;
|
||||
expect(btn.disabled).toBe(false);
|
||||
});
|
||||
expect(screen.getByRole('region').getAttribute('data-phase')).toBe('running');
|
||||
});
|
||||
|
||||
it('resets interruptPending when a fresh run starts after an interrupt (codex P2 on PR #1315)', async () => {
|
||||
// Previously `interruptPending` stayed true forever once clicked,
|
||||
// so a second run on the same mount would render the kill
|
||||
// button stuck in the "Interrupting…" state.
|
||||
@@ -209,7 +263,10 @@ describe('<CritiqueTheaterMount> (Phase 9.1)', () => {
|
||||
});
|
||||
});
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Interrupt' }));
|
||||
expect(screen.getByRole('status').getAttribute('data-phase')).toBe('interrupted');
|
||||
// Daemon ack is async; wait for the collapsed surface to mount.
|
||||
await waitFor(() => {
|
||||
expect(screen.getByRole('status').getAttribute('data-phase')).toBe('interrupted');
|
||||
});
|
||||
|
||||
// Daemon emits a fresh run_started for the next rerun. The
|
||||
// collapsed badge should give way to a live stage with a fresh
|
||||
|
||||
Reference in New Issue
Block a user