mirror of
https://github.com/nexu-io/open-design.git
synced 2026-07-20 10:08:36 +08:00
fix(web): resolve P2 + P3 review feedback on Phase 8 (PR #1314)
Addresses all 4 P2 + 3 P3 items from codex, Siri-Ray, and lefarcen.
State-lifecycle fixes (3 x P2)
1. Reducer learns a synthetic `__reset__` action (`CritiqueResetAction`).
Host hooks dispatch it when their gating prop changes so a stale
run from a prior project / transcript cannot bleed into the next
context. Reset is idempotent on idle (returns the same reference).
2. `useCritiqueStream` dispatches `__reset__` at the top of its
connection effect, so a workspace switch from project A (which
streamed a critique) to project B clears the reducer before the
new EventSource opens. enabled=false also clears.
3. `useCritiqueReplay` dispatches `__reset__` at the top of its
parse effect, so transcriptUrl swaps (including swap-to-null after
a replay reached `shipped`) lift the reducer back to idle before
the new fetch starts.
SSE validation (1 x P2)
4. `sseToPanelEvent` now runs a per-variant `hasValidVariantShape`
check after the cheap `isPanelEvent` predicate. A
`critique.ship` frame missing `composite` / `round` / `status` /
`artifactRef` is rejected before reaching the reducer, so
TheaterCollapsed can no longer crash on `undefined.toFixed(1)`.
Every variant's required fields are validated: run_started
(protocolVersion, non-empty cast, maxRounds, threshold, scale),
panelist_* (round, role, plus variant-specific shape), round_end
(round, composite, mustFix, decision in {continue,ship}, reason),
ship (round, composite, status, artifactRef.{projectId,artifactId},
summary), degraded (reason, adapter), interrupted (bestRound,
composite), failed (cause), parser_warning (kind, position).
Reducer correctness (1 x P2)
5. `panelist_open` now materializes the round + an empty panelist
view (`{dims: [], mustFixes: []}`) so TheaterStage can highlight
the in-progress lane the instant the tag opens. Before this, a
stream that emitted only `panelist_open` after `run_started` left
`rounds = []` and the UI rendered no current round until a later
`panelist_dim` arrived.
Polish (3 x P3)
6. Brand role tint swaps from `var(--magenta, var(--accent))` to
`var(--purple, var(--accent))`. `--purple` is actually defined
across the design systems; `--magenta` is not, so Brand was
silently falling through to `--accent` and looking identical to
Designer.
7. New i18n key `critiqueTheater.interruptedSummary` for the
interrupted-collapse copy ("Interrupted at round N, best
composite X.X"). Previously the interrupted branch reused
`shippedSummary` and the UI read "Shipped at round..." for a run
that specifically did not ship. Native value in en + zh-CN; other
locales fall back via `...en` spread.
8. `TheaterDegraded` heading id comes from `useId()` instead of a
hardcoded `theater-degraded-heading`, so two chips rendered on
the same page (chat history with multiple completed runs) keep
their aria-labelledby references unambiguous.
Tests (15 new cases)
- reducer.test.ts (+5): __reset__ on running/terminal/idle, panelist_open materializes round, panelist_open does not stomp prior panelist data.
- sse.test.ts (+6): variant-level rejection for ship without required fields, degraded without adapter, run_started with empty cast, panelist_dim with non-numeric score, round_end with unknown decision, plus a positive fully-formed ship.
- useCritiqueStream.test.tsx (+2): state reset on projectId change, state reset on enabled flip false.
- useCritiqueReplay.test.tsx (+1): state reset on transcriptUrl swap to null after a replay reached shipped.
- TheaterCollapsed.test.tsx (text-pinning update): asserts the interrupted branch reads "Interrupted at round 1" + "best composite 7.9", and explicitly NOT "Shipped at round...".
- TheaterDegraded.test.tsx (+1): two chips on the same page get unique aria-labelledby ids that each resolve to an `<h3>`.
Validated
- pnpm guard clean
- pnpm --filter @open-design/web typecheck clean
- Theater suite: 13 files, 101 tests (was 86 on the first Phase 8 push, +15 new)
- tests/i18n/locales.test.ts 5 of 5 across 18 locales
This commit is contained in:
Nagendhra
@@ -49,7 +49,7 @@ export function TheaterCollapsed({ state }: Props) {
|
||||
{t('critiqueTheater.interrupted')}
|
||||
</span>
|
||||
<span className="theater-collapsed-summary">
|
||||
{t('critiqueTheater.shippedSummary', {
|
||||
{t('critiqueTheater.interruptedSummary', {
|
||||
round: state.bestRound,
|
||||
composite: state.composite.toFixed(1),
|
||||
})}
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { useId } from 'react';
|
||||
import { useT } from '../../i18n';
|
||||
import type { Dict } from '../../i18n/types';
|
||||
import type { DegradedReason } from '@open-design/contracts/critique';
|
||||
@@ -24,14 +25,18 @@ const REASON_KEY: Record<DegradedReason, keyof Dict> = {
|
||||
*/
|
||||
export function TheaterDegraded({ reason, adapter }: Props) {
|
||||
const t = useT();
|
||||
// Per-instance heading id so two chips on the same page (e.g. a
|
||||
// chat history that renders multiple completed runs) keep their
|
||||
// aria-labelledby references unambiguous. Lefarcen P3 on PR #1314.
|
||||
const headingId = useId();
|
||||
return (
|
||||
<section
|
||||
className="theater-degraded"
|
||||
role="status"
|
||||
data-reason={reason}
|
||||
aria-labelledby="theater-degraded-heading"
|
||||
aria-labelledby={headingId}
|
||||
>
|
||||
<h3 id="theater-degraded-heading" className="theater-degraded-heading">
|
||||
<h3 id={headingId} className="theater-degraded-heading">
|
||||
{t('critiqueTheater.degradedHeading')}
|
||||
</h3>
|
||||
<p className="theater-degraded-reason">
|
||||
|
||||
@@ -91,6 +91,14 @@ export function useCritiqueReplay(
|
||||
// Stores the parsed events in component state so the pace effect can
|
||||
// react to them; cursor is reset because a new transcript is a new run.
|
||||
useEffect(() => {
|
||||
// Whenever the URL changes (including swap to null), lift the
|
||||
// reducer back to idle so a prior replay's terminal state cannot
|
||||
// bleed into the new transcript or the "no transcript" empty
|
||||
// state. Lefarcen + Siri-Ray + codex P2 on PR #1314: without this
|
||||
// reset, a finished replay → null URL would leave the previous
|
||||
// run's rounds visible while TheaterTranscript reported
|
||||
// status: 'idle'.
|
||||
dispatchRef.current({ type: '__reset__' });
|
||||
if (!transcriptUrl) {
|
||||
setMeta({ status: 'idle', error: null });
|
||||
setEvents(null);
|
||||
|
||||
@@ -55,6 +55,17 @@ export function useCritiqueStream(
|
||||
const factory = options.connectionFactory ?? createCritiqueEventsConnection;
|
||||
|
||||
useEffect(() => {
|
||||
// Clear any state from the previous project / enabled session
|
||||
// before we decide whether to open a new connection. Without this,
|
||||
// a workspace that switches from project A (which already streamed
|
||||
// a critique) to project B would keep rendering project A's
|
||||
// Theater state until B's run_started arrived, and a flip from
|
||||
// enabled=true to enabled=false would leave the in-flight run
|
||||
// visible after the connection had been torn down (lefarcen +
|
||||
// Siri-Ray + codex P2 on PR #1314). The reducer's reset handler
|
||||
// is a no-op when state is already idle, so this is cheap on
|
||||
// the common path.
|
||||
dispatchRef.current({ type: '__reset__' });
|
||||
if (!enabled || !projectId) return;
|
||||
if (typeof window === 'undefined' && !options.EventSourceCtor) return;
|
||||
const conn = factory(
|
||||
|
||||
@@ -8,7 +8,20 @@ import type {
|
||||
ShipStatus,
|
||||
} from '@open-design/contracts/critique';
|
||||
|
||||
export type CritiqueAction = PanelEvent;
|
||||
/**
|
||||
* Synthetic reducer action the host hooks (`useCritiqueStream`,
|
||||
* `useCritiqueReplay`) dispatch when their gating prop changes
|
||||
* (projectId switch, enabled flip, transcriptUrl swap). Lifts the
|
||||
* reducer back to idle so the surrounding UI cannot render a stale
|
||||
* critique that belonged to the previous project / transcript. Lives
|
||||
* in the action union (not as a separate setter) so the reducer
|
||||
* remains the single source of truth for state transitions.
|
||||
*/
|
||||
export interface CritiqueResetAction {
|
||||
type: '__reset__';
|
||||
}
|
||||
|
||||
export type CritiqueAction = PanelEvent | CritiqueResetAction;
|
||||
|
||||
export interface CritiqueDimScore {
|
||||
name: string;
|
||||
@@ -165,6 +178,13 @@ function ensurePanelist(
|
||||
* launch consecutive runs without an explicit reset action.
|
||||
*/
|
||||
export function reduce(state: CritiqueState, action: CritiqueAction): CritiqueState {
|
||||
// Host-dispatched reset (project change, enabled flip, transcript
|
||||
// swap) always lifts us back to idle so a stale run from a prior
|
||||
// project/transcript cannot bleed into the new context. Lefarcen +
|
||||
// Siri-Ray + codex P2 on PR #1314.
|
||||
if (action.type === '__reset__') {
|
||||
return state.phase === 'idle' ? state : initialState;
|
||||
}
|
||||
// `run_started` is always accepted: from idle it boots a new run, and
|
||||
// mid-stream it discards any prior state and reboots cleanly (the daemon
|
||||
// does not multiplex two runs onto one SSE channel, so this only fires on
|
||||
@@ -209,12 +229,23 @@ export function reduce(state: CritiqueState, action: CritiqueAction): CritiqueSt
|
||||
}
|
||||
|
||||
switch (action.type) {
|
||||
case 'panelist_open':
|
||||
case 'panelist_open': {
|
||||
// Materialize the round + an empty panelist view so TheaterStage
|
||||
// can render the in-progress lane the instant the tag opens
|
||||
// (lefarcen + Siri-Ray P2 on PR #1314): without this, a stream
|
||||
// that emits only `panelist_open` after `run_started` would
|
||||
// leave `rounds = []` and the UI would render no current
|
||||
// round until a later `panelist_dim` arrived.
|
||||
const rounds = withRound(state.rounds, action.round, (round) => {
|
||||
ensurePanelist(round, action.role);
|
||||
});
|
||||
return {
|
||||
...state,
|
||||
rounds,
|
||||
activePanelist: action.role,
|
||||
activeRound: action.round,
|
||||
};
|
||||
}
|
||||
case 'panelist_dim': {
|
||||
const rounds = withRound(state.rounds, action.round, (round) => {
|
||||
const panelist = ensurePanelist(round, action.role);
|
||||
|
||||
@@ -51,11 +51,74 @@ export function critiqueEventsUrl(projectId: string): string {
|
||||
* fails (missing runId, unknown type), we drop the frame and the
|
||||
* reducer never sees it.
|
||||
*/
|
||||
/** Per-variant required-fields validator. `isPanelEvent` from contracts only
|
||||
* checks `type` is known and `runId` is non-empty, so a frame like
|
||||
* `{ 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 {
|
||||
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')
|
||||
&& typeof event.maxRounds === 'number'
|
||||
&& typeof event.threshold === 'number'
|
||||
&& typeof event.scale === 'number';
|
||||
case 'panelist_open':
|
||||
return typeof event.round === 'number' && typeof event.role === 'string';
|
||||
case 'panelist_dim':
|
||||
return typeof event.round === 'number'
|
||||
&& typeof event.role === 'string'
|
||||
&& 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.text === 'string';
|
||||
case 'panelist_close':
|
||||
return typeof event.round === 'number'
|
||||
&& typeof event.role === 'string'
|
||||
&& 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.reason === 'string';
|
||||
case 'ship':
|
||||
return typeof event.round === 'number'
|
||||
&& typeof event.composite === 'number'
|
||||
&& typeof event.status === 'string'
|
||||
&& 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';
|
||||
case 'interrupted':
|
||||
return typeof event.bestRound === 'number' && typeof event.composite === 'number';
|
||||
case 'failed':
|
||||
return typeof event.cause === 'string';
|
||||
case 'parser_warning':
|
||||
return typeof event.kind === 'string' && typeof event.position === 'number';
|
||||
}
|
||||
}
|
||||
|
||||
export function sseToPanelEvent(eventName: CritiqueSseEventName, data: unknown): PanelEvent | null {
|
||||
if (data === null || typeof data !== 'object') return null;
|
||||
const type = eventName.slice('critique.'.length);
|
||||
const candidate = { ...(data as Record<string, unknown>), type };
|
||||
return isPanelEvent(candidate) ? candidate : null;
|
||||
if (!isPanelEvent(candidate)) return null;
|
||||
// Variant-level guard: a frame that passes the cheap predicate but
|
||||
// is missing variant-specific fields would otherwise reach the
|
||||
// reducer and crash the UI on `undefined.toFixed()` / `undefined.cast`
|
||||
// (lefarcen + Siri-Ray + codex P2 on PR #1314).
|
||||
return hasValidVariantShape(candidate) ? candidate : null;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -1113,6 +1113,7 @@ export const en: Dict = {
|
||||
'critiqueTheater.replaySpeed': 'Replay speed',
|
||||
'critiqueTheater.readOnly': 'Read-only',
|
||||
'critiqueTheater.shippedSummary': 'Shipped at round {round} · composite {composite}',
|
||||
'critiqueTheater.interruptedSummary': 'Interrupted at round {round} · best composite {composite}',
|
||||
'critiqueTheater.shippedBadge': 'Shipped',
|
||||
'critiqueTheater.belowThresholdBadge': 'Below threshold',
|
||||
'critiqueTheater.timedOutBadge': 'Timed out',
|
||||
|
||||
@@ -1083,6 +1083,7 @@ export const zhCN: Dict = {
|
||||
'critiqueTheater.replaySpeed': '回放速度',
|
||||
'critiqueTheater.readOnly': '只读',
|
||||
'critiqueTheater.shippedSummary': '已于第 {round} 轮发布 · 综合分 {composite}',
|
||||
'critiqueTheater.interruptedSummary': '已于第 {round} 轮中断 · 最佳综合分 {composite}',
|
||||
'critiqueTheater.shippedBadge': '已发布',
|
||||
'critiqueTheater.belowThresholdBadge': '低于阈值',
|
||||
'critiqueTheater.timedOutBadge': '已超时',
|
||||
|
||||
@@ -1494,6 +1494,7 @@ export interface Dict {
|
||||
'critiqueTheater.replaySpeed': string;
|
||||
'critiqueTheater.readOnly': string;
|
||||
'critiqueTheater.shippedSummary': string;
|
||||
'critiqueTheater.interruptedSummary': string;
|
||||
'critiqueTheater.shippedBadge': string;
|
||||
'critiqueTheater.belowThresholdBadge': string;
|
||||
'critiqueTheater.timedOutBadge': string;
|
||||
|
||||
@@ -11953,7 +11953,7 @@ body.entry-resizing { cursor: col-resize; user-select: none; }
|
||||
}
|
||||
.theater-lane[data-role="designer"] { border-left-color: color-mix(in srgb, var(--accent) 60%, var(--border)); }
|
||||
.theater-lane[data-role="critic"] { border-left-color: color-mix(in srgb, var(--amber) 60%, var(--border)); }
|
||||
.theater-lane[data-role="brand"] { border-left-color: color-mix(in srgb, var(--magenta, var(--accent)) 60%, var(--border)); }
|
||||
.theater-lane[data-role="brand"] { border-left-color: color-mix(in srgb, var(--purple, var(--accent)) 60%, var(--border)); }
|
||||
.theater-lane[data-role="a11y"] { border-left-color: color-mix(in srgb, var(--green, var(--accent)) 60%, var(--border)); }
|
||||
.theater-lane[data-role="copy"] { border-left-color: color-mix(in srgb, var(--blue, var(--accent)) 60%, var(--border)); }
|
||||
.theater-lane-head {
|
||||
|
||||
@@ -72,7 +72,15 @@ describe('<TheaterCollapsed> (Phase 8)', () => {
|
||||
render(<TheaterCollapsed state={state} />);
|
||||
expect(screen.getByRole('status').getAttribute('data-phase')).toBe('interrupted');
|
||||
expect(screen.getByText('Interrupted')).toBeTruthy();
|
||||
expect(screen.getByText(/composite 7\.9/)).toBeTruthy();
|
||||
// Lefarcen P3 on PR #1314: the interrupted branch must NOT reuse
|
||||
// the shipped summary copy ("Shipped at round...") since the run
|
||||
// was specifically NOT shipped. Pin the wording so a future
|
||||
// refactor that swaps the keys back trips this assertion.
|
||||
expect(screen.getByText(/Interrupted at round 1/)).toBeTruthy();
|
||||
expect(screen.getByText(/best composite 7\.9/)).toBeTruthy();
|
||||
expect(
|
||||
screen.queryByText((_, node) => /Shipped at round/.test(node?.textContent ?? '')),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it('renders the failed phase with the cause-specific reason', () => {
|
||||
|
||||
@@ -37,4 +37,27 @@ describe('<TheaterDegraded> (Phase 8)', () => {
|
||||
expect(text.length).toBeGreaterThan(20);
|
||||
}
|
||||
});
|
||||
|
||||
it('assigns a unique heading id per instance via useId (PR #1314 review)', () => {
|
||||
// Lefarcen P3: the previous hardcoded `id="theater-degraded-heading"`
|
||||
// would produce duplicate ids when two chips render on the same page
|
||||
// (chat history rendering several completed runs). Two chips must
|
||||
// resolve their own `aria-labelledby` references.
|
||||
render(
|
||||
<div>
|
||||
<TheaterDegraded reason="malformed_block" adapter="pi-rpc" />
|
||||
<TheaterDegraded reason="missing_artifact" adapter="codex" />
|
||||
</div>,
|
||||
);
|
||||
const sections = screen.getAllByRole('status');
|
||||
expect(sections).toHaveLength(2);
|
||||
const labelledByA = sections[0]!.getAttribute('aria-labelledby');
|
||||
const labelledByB = sections[1]!.getAttribute('aria-labelledby');
|
||||
expect(labelledByA).toBeTruthy();
|
||||
expect(labelledByB).toBeTruthy();
|
||||
expect(labelledByA).not.toBe(labelledByB);
|
||||
// Each heading id is actually referenced by its own section.
|
||||
expect(document.getElementById(labelledByA!)?.tagName).toBe('H3');
|
||||
expect(document.getElementById(labelledByB!)?.tagName).toBe('H3');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -353,4 +353,37 @@ describe('useCritiqueReplay (Phase 7.3)', () => {
|
||||
expect(gunzip).toHaveBeenCalledTimes(1);
|
||||
expect(sink.state.phase).toBe('shipped');
|
||||
});
|
||||
|
||||
it('resets reducer state to idle when transcriptUrl flips to null after a replay (PR #1314 review)', async () => {
|
||||
// Lefarcen + Siri-Ray + codex P2: clearing the URL after a
|
||||
// replay reached `shipped` previously only cleared meta + events
|
||||
// + cursor. The reducer state stayed at `shipped`, so
|
||||
// TheaterTranscript could render stale rounds while reporting
|
||||
// `status: 'idle'`.
|
||||
const sink: Sink = { state: { phase: 'idle' }, status: 'idle', error: null };
|
||||
const { rerender } = render(
|
||||
<Probe
|
||||
url="/api/replay.ndjson"
|
||||
speed="instant"
|
||||
options={{ fetchTranscript: async () => ndjson(TRANSCRIPT) }}
|
||||
sink={sink}
|
||||
/>,
|
||||
);
|
||||
await waitFor(() => {
|
||||
expect(sink.state.phase).toBe('shipped');
|
||||
});
|
||||
|
||||
rerender(
|
||||
<Probe
|
||||
url={null}
|
||||
speed="instant"
|
||||
options={{ fetchTranscript: async () => ndjson(TRANSCRIPT) }}
|
||||
sink={sink}
|
||||
/>,
|
||||
);
|
||||
await waitFor(() => {
|
||||
expect(sink.state.phase).toBe('idle');
|
||||
});
|
||||
expect(sink.status).toBe('idle');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -153,4 +153,55 @@ describe('useCritiqueStream (Phase 7.2)', () => {
|
||||
expect(handles[0]!.closed).toBe(true);
|
||||
expect(handles[1]!.closed).toBe(false);
|
||||
});
|
||||
|
||||
it('resets reducer state to idle when projectId changes (PR #1314 review)', () => {
|
||||
// Lefarcen + Siri-Ray + codex P2: a workspace switch from project
|
||||
// A (which already streamed a critique) to project B must not
|
||||
// leave project A's Theater state visible while waiting for B's
|
||||
// run_started.
|
||||
const { factory, handles } = makeFactory();
|
||||
const sink: Harness = { state: { phase: 'idle' } };
|
||||
const { rerender } = render(
|
||||
<Probe projectId="proj-1" enabled factory={factory} sink={sink} />,
|
||||
);
|
||||
act(() => {
|
||||
handles[0]!.send({
|
||||
type: 'run_started',
|
||||
runId: 'r1',
|
||||
protocolVersion: 1,
|
||||
cast: ['critic'],
|
||||
maxRounds: 3,
|
||||
threshold: 8,
|
||||
scale: 10,
|
||||
});
|
||||
});
|
||||
expect(sink.state.phase).toBe('running');
|
||||
|
||||
rerender(<Probe projectId="proj-2" enabled factory={factory} sink={sink} />);
|
||||
expect(sink.state.phase).toBe('idle');
|
||||
});
|
||||
|
||||
it('resets reducer state to idle when enabled flips to false mid-run', () => {
|
||||
const { factory, handles } = makeFactory();
|
||||
const sink: Harness = { state: { phase: 'idle' } };
|
||||
const { rerender } = render(
|
||||
<Probe projectId="proj-1" enabled factory={factory} sink={sink} />,
|
||||
);
|
||||
act(() => {
|
||||
handles[0]!.send({
|
||||
type: 'run_started',
|
||||
runId: 'r1',
|
||||
protocolVersion: 1,
|
||||
cast: ['critic'],
|
||||
maxRounds: 3,
|
||||
threshold: 8,
|
||||
scale: 10,
|
||||
});
|
||||
});
|
||||
expect(sink.state.phase).toBe('running');
|
||||
|
||||
rerender(<Probe projectId="proj-1" enabled={false} factory={factory} sink={sink} />);
|
||||
expect(sink.state.phase).toBe('idle');
|
||||
expect(handles[0]!.closed).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -311,4 +311,62 @@ describe('Critique Theater reducer (Phase 7)', () => {
|
||||
});
|
||||
expect(s2).toBe(s1);
|
||||
});
|
||||
|
||||
it('__reset__ lifts a running state back to idle (PR #1314 review)', () => {
|
||||
// Project / transcript context changes dispatch this synthetic
|
||||
// action so the next mount cannot see a stale prior run.
|
||||
let s: CritiqueState = runningState();
|
||||
s = reduce(s, {
|
||||
type: 'panelist_must_fix', runId: RUN_ID, round: 1, role: 'critic', text: 'low contrast',
|
||||
});
|
||||
expect(s.phase).toBe('running');
|
||||
s = reduce(s, { type: '__reset__' });
|
||||
expect(s.phase).toBe('idle');
|
||||
});
|
||||
|
||||
it('__reset__ lifts a terminal state back to idle too', () => {
|
||||
let s: CritiqueState = runningState();
|
||||
s = reduce(s, {
|
||||
type: 'ship', runId: RUN_ID, round: 1, composite: 8.6, status: 'shipped',
|
||||
artifactRef: { projectId: 'p1', artifactId: 'a1' }, summary: 'ok',
|
||||
});
|
||||
expect(s.phase).toBe('shipped');
|
||||
s = reduce(s, { type: '__reset__' });
|
||||
expect(s.phase).toBe('idle');
|
||||
});
|
||||
|
||||
it('__reset__ on idle state returns the same reference (no allocation churn)', () => {
|
||||
const next = reduce(initialState, { type: '__reset__' });
|
||||
expect(next).toBe(initialState);
|
||||
});
|
||||
|
||||
it('panelist_open materializes the round + empty panelist view (PR #1314 review)', () => {
|
||||
// Before this fix, the reducer set activePanelist/activeRound but
|
||||
// left `rounds = []`, so TheaterStage had no in-progress round
|
||||
// cell to highlight until a later dim/must-fix/close event
|
||||
// arrived. The protocol treats panelist_open as the tag-open
|
||||
// event, so the UI must become visible at that point.
|
||||
let s: CritiqueState = runningState();
|
||||
s = reduce(s, { type: 'panelist_open', runId: RUN_ID, round: 1, role: 'critic' });
|
||||
expect(s.phase).toBe('running');
|
||||
if (s.phase !== 'running') return;
|
||||
expect(s.rounds).toHaveLength(1);
|
||||
expect(s.rounds[0]?.n).toBe(1);
|
||||
expect(s.rounds[0]?.panelists.critic).toEqual({ dims: [], mustFixes: [] });
|
||||
expect(s.activePanelist).toBe('critic');
|
||||
expect(s.activeRound).toBe(1);
|
||||
});
|
||||
|
||||
it('panelist_open does not stomp an existing round with prior panelist data', () => {
|
||||
let s: CritiqueState = runningState();
|
||||
s = reduce(s, {
|
||||
type: 'panelist_dim', runId: RUN_ID, round: 1, role: 'designer',
|
||||
dimName: 'composition', dimScore: 8, dimNote: 'tight',
|
||||
});
|
||||
s = reduce(s, { type: 'panelist_open', runId: RUN_ID, round: 1, role: 'critic' });
|
||||
expect(s.phase).toBe('running');
|
||||
if (s.phase !== 'running') return;
|
||||
expect(s.rounds[0]?.panelists.designer?.dims).toHaveLength(1);
|
||||
expect(s.rounds[0]?.panelists.critic).toEqual({ dims: [], mustFixes: [] });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -307,4 +307,85 @@ describe('sseToPanelEvent (Phase 7.2)', () => {
|
||||
}),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it('drops a critique.ship frame missing variant-specific required fields (PR #1314 review)', () => {
|
||||
// Lefarcen + Siri-Ray + codex P2: `isPanelEvent` only checks
|
||||
// `type` and `runId`, so a frame like `{ runId: 'r' }` on the
|
||||
// critique.ship channel would reach the reducer with composite,
|
||||
// round, status, artifactRef all undefined. TheaterCollapsed
|
||||
// would then call `final.composite.toFixed(1)` and crash.
|
||||
expect(
|
||||
sseToPanelEvent('critique.ship' as CritiqueSseEventName, { runId: 'r' }),
|
||||
).toBeNull();
|
||||
// Even with most fields, missing `summary` still rejects.
|
||||
expect(
|
||||
sseToPanelEvent('critique.ship' as CritiqueSseEventName, {
|
||||
runId: 'r',
|
||||
round: 1,
|
||||
composite: 8.6,
|
||||
status: 'shipped',
|
||||
artifactRef: { projectId: 'p', artifactId: 'a' },
|
||||
}),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it('accepts a fully-formed critique.ship frame', () => {
|
||||
const action = sseToPanelEvent('critique.ship' as CritiqueSseEventName, {
|
||||
runId: 'r',
|
||||
round: 1,
|
||||
composite: 8.6,
|
||||
status: 'shipped',
|
||||
artifactRef: { projectId: 'p', artifactId: 'a' },
|
||||
summary: 'looks good',
|
||||
});
|
||||
expect(action?.type).toBe('ship');
|
||||
});
|
||||
|
||||
it('drops a critique.degraded frame missing the adapter field', () => {
|
||||
expect(
|
||||
sseToPanelEvent('critique.degraded' as CritiqueSseEventName, {
|
||||
runId: 'r',
|
||||
reason: 'malformed_block',
|
||||
}),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it('drops a critique.run_started frame with an empty cast', () => {
|
||||
expect(
|
||||
sseToPanelEvent('critique.run_started' as CritiqueSseEventName, {
|
||||
runId: 'r',
|
||||
protocolVersion: 1,
|
||||
cast: [],
|
||||
maxRounds: 3,
|
||||
threshold: 8,
|
||||
scale: 10,
|
||||
}),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it('drops a critique.panelist_dim frame with non-numeric dimScore', () => {
|
||||
expect(
|
||||
sseToPanelEvent('critique.panelist_dim' as CritiqueSseEventName, {
|
||||
runId: 'r',
|
||||
round: 1,
|
||||
role: 'critic',
|
||||
dimName: 'contrast',
|
||||
dimScore: 'eight',
|
||||
dimNote: 'borderline',
|
||||
}),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it('drops a critique.round_end frame with an unknown decision value', () => {
|
||||
expect(
|
||||
sseToPanelEvent('critique.round_end' as CritiqueSseEventName, {
|
||||
runId: 'r',
|
||||
round: 1,
|
||||
composite: 7.4,
|
||||
mustFix: 2,
|
||||
decision: 'maybe',
|
||||
reason: 'unclear',
|
||||
}),
|
||||
).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user