Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions packages/cli/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ const draft = defineCommand({
report: { type: 'string', description: 'write the full draft QA report to this JSON path' },
findings: { type: 'string', description: 'write check findings JSON to this path' },
'evidence-dir': { type: 'string', description: 'directory for sampled frame evidence/contact sheet' },
waive: { type: 'string', description: 'comma-separated QA finding codes the user chose to skip (persisted in qa/waivers.json; evidence-integrity codes are refused)' },
},
async run({ args }) {
const r = await renderTool.draft({
Expand All @@ -92,6 +93,7 @@ const draft = defineCommand({
reportPath: args.report ? String(args.report) : undefined,
findingsPath: args.findings ? String(args.findings) : undefined,
frameEvidenceDir: args['evidence-dir'] ? String(args['evidence-dir']) : undefined,
waive: args.waive ? String(args.waive).split(',').map((code) => code.trim()).filter(Boolean) : undefined,
onProgress: (c) => process.stderr.write(c),
});
printJson(r);
Expand Down Expand Up @@ -401,11 +403,16 @@ const plan = defineCommand({
meta: { name: 'plan', description: 'Work with the plan.json video IR.' },
subCommands: {
validate: defineCommand({
meta: { name: 'validate', description: 'Validate a plan.json; exit 1 on errors.' },
meta: { name: 'validate', description: 'Validate a plan.json; exit 1 on errors. A valid plan prints its summary — present that to the user, not your own abstract.' },
args: { file: { type: 'positional', required: true } },
run({ args }) {
const r = validateEdl(readPlan(String(args.file)));
printJson(r);
const planJson = readPlan(String(args.file));
const r = validateEdl(planJson);
// A valid plan is about to be shown for approval, and this result is
// the last thing the model reads before writing that message — attach
// the host's own rendering so the user reviews the real plan, not a
// hand-written abstract of it.
printJson(r.ok ? { ...r, summary: summarizeEdl(planJson as VideoEdl) } : r);
if (!r.ok) process.exitCode = 1;
},
}),
Expand Down Expand Up @@ -502,6 +509,7 @@ const gate = defineCommand({
gate: { type: 'string', default: 'none', description: 'gate_a | gate_b | gate_c | preview | gate_d' },
decision: { type: 'string', default: 'none', description: 'approve | revise | none' },
scope: { type: 'string', default: 'unknown', description: 'visual_only | gate_b_payload | none | unknown' },
origin: { type: 'string', default: 'unknown', description: 'who asked for the change: user (current turn names it in the user\'s own words) | model (model-initiated or mixed reply) | unknown' },
recovery: { type: 'string', default: 'unknown', description: 'available | not_available | unknown' },
'recovery-decision': { type: 'string', default: 'none', description: 'legacy input only: new_visual_revision | pause | none; never emit a new recovery form' },
'artifact-state': { type: 'string', default: 'unknown', description: 'new | unchanged | changed | unknown' },
Expand All @@ -515,6 +523,7 @@ const gate = defineCommand({
gate: String(args.gate) as GateTransitionInput['gate'],
decision: String(args.decision) as GateTransitionInput['decision'],
scope: String(args.scope) as GateTransitionInput['scope'],
origin: String(args.origin) as GateTransitionInput['origin'],
recovery: String(args.recovery) as GateTransitionInput['recovery'],
recoveryDecision: String(args['recovery-decision']) as GateTransitionInput['recoveryDecision'],
artifactState: String(args['artifact-state']) as GateTransitionInput['artifactState'],
Expand Down
39 changes: 32 additions & 7 deletions packages/core/src/gates/transition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,20 @@ export type RecoveryState = 'unknown' | 'available' | 'not_available';
export type RecoveryDecision = 'none' | 'new_visual_revision' | 'pause';
export type ArtifactState = 'unknown' | 'new' | 'unchanged' | 'changed';
export type ApprovalStatus = 'unknown' | 'none' | 'pending' | 'approved';
/** Who asked for the change decides whether to ask again. `user` means the
* CURRENT turn names the change in the user's own words; a model-initiated
* change — or a reply that mixes an instruction with the model's own
* proposal — is `model` (the higher bar wins). No host verifies this in OVS:
* it is the driving agent's honest self-report. */
export type ChangeOrigin = 'unknown' | 'user' | 'model';

export interface GateTransitionInput {
line?: VideoLine;
artifact?: GateArtifact;
gate?: GateName;
decision?: GateDecision;
scope?: RevisionScope;
origin?: ChangeOrigin;
recovery?: RecoveryState;
recoveryDecision?: RecoveryDecision;
artifactState?: ArtifactState;
Expand All @@ -37,6 +44,7 @@ const VALID = {
gate: new Set<GateName>(['none', 'gate_a', 'gate_b', 'gate_c', 'preview', 'gate_d']),
decision: new Set<GateDecision>(['none', 'approve', 'revise']),
scope: new Set<RevisionScope>(['unknown', 'none', 'visual_only', 'gate_b_payload']),
origin: new Set<ChangeOrigin>(['unknown', 'user', 'model']),
recovery: new Set<RecoveryState>(['unknown', 'available', 'not_available']),
recoveryDecision: new Set<RecoveryDecision>(['none', 'new_visual_revision', 'pause']),
artifactState: new Set<ArtifactState>(['unknown', 'new', 'unchanged', 'changed']),
Expand Down Expand Up @@ -95,6 +103,7 @@ export function resolveGateTransition(raw: GateTransitionInput = {}): GateTransi
gate: raw.gate ?? 'none',
decision: raw.decision ?? 'none',
scope: raw.scope ?? 'unknown',
origin: raw.origin ?? 'unknown',
recovery: raw.recovery ?? 'unknown',
recoveryDecision: raw.recoveryDecision ?? 'none',
artifactState: raw.artifactState ?? 'unknown',
Expand All @@ -106,6 +115,7 @@ export function resolveGateTransition(raw: GateTransitionInput = {}): GateTransi
assertEnum('gate', input.gate, VALID.gate);
assertEnum('decision', input.decision, VALID.decision);
assertEnum('scope', input.scope, VALID.scope);
assertEnum('origin', input.origin, VALID.origin);
assertEnum('recovery', input.recovery, VALID.recovery);
assertEnum('recoveryDecision', input.recoveryDecision, VALID.recoveryDecision);
assertEnum('artifactState', input.artifactState, VALID.artifactState);
Expand All @@ -115,6 +125,21 @@ export function resolveGateTransition(raw: GateTransitionInput = {}): GateTransi
}
const lineOps = lineOperations(input.line, input.artifact);

// Who asked for the change decides whether to ask again: a change the
// current user turn dictates is applied directly — the instruction is itself
// the authorization, and asking them to confirm a change they just asked for
// costs a full round trip and teaches them their instructions are not taken
// at face value. A mixed reply (instruction + model proposal) is `model`.
if (input.decision === 'revise' && input.scope === 'gate_b_payload' && input.origin === 'user') {
return result({
nextAction: 'apply_user_instruction_then_approve_plan',
authorities: ['edit_current_artifact', 'approve_gate_b'],
allowedOps: ['edit_current_artifact', 'continue_approved_plan'],
prohibitedOps: NO_VISUAL_RESET,
reason: 'The current user turn names this change in the user\'s own words; that instruction is itself the authorization. Apply exactly that change and re-sign — never ask them to confirm a change they dictated.',
});
}

// A signed-payload amendment creates a new signature and therefore a fresh
// OVS draft-repair cycle. Recovery evidence for the old signature is stale.
if (input.decision === 'revise' && input.scope === 'gate_b_payload') {
Expand Down Expand Up @@ -165,9 +190,9 @@ export function resolveGateTransition(raw: GateTransitionInput = {}): GateTransi
});
}
return result({
nextAction: 'report_visual_qa_blocker',
prohibitedOps: ['emit_form', 'edit_files', ...NO_VISUAL_RESET],
reason: 'Technical QA exhaustion never creates a user authorization form. Wait for a real revision request, which authorizes the next bounded cycle.',
nextAction: 'present_findings_and_ask_user_direction',
prohibitedOps: ['emit_form', 'edit_files', 'restart_visual_qa_cycle', ...NO_VISUAL_RESET],
reason: 'The visual QA cycle is exhausted. Show the current frames and remaining findings, offer another repair round or skipping the named check, and end the turn — the user\'s reply grants the next cycle. Then make a materially different edit: the failed strategies are recorded, and repeating one spends the new budget for nothing.',
});
}

Expand Down Expand Up @@ -263,7 +288,7 @@ export function resolveGateTransition(raw: GateTransitionInput = {}): GateTransi
authorities: ['edit_current_artifact', 'restart_visual_qa_cycle'],
allowedOps: lineOps.edit,
prohibitedOps: ['emit_form', ...NO_VISUAL_RESET],
reason: 'Consume the legacy recovery submission once. New turns use the original revise decision and OVS content-signature reset.',
reason: 'Consume the legacy recovery submission once — by making a materially different edit, never by repeating a strategy the recorded evidence already shows failed.',
});
}
if (input.recovery === 'unknown') {
Expand All @@ -285,9 +310,9 @@ export function resolveGateTransition(raw: GateTransitionInput = {}): GateTransi

if (input.recovery === 'available') {
return result({
nextAction: 'report_visual_qa_blocker',
prohibitedOps: ['emit_form', 'edit_files', ...NO_VISUAL_RESET],
reason: 'Technical QA exhaustion is not a separate user decision. Report the blocker and wait for a real revision request; never emit a recovery form.',
nextAction: 'present_findings_and_ask_user_direction',
prohibitedOps: ['emit_form', 'edit_files', 'restart_visual_qa_cycle', ...NO_VISUAL_RESET],
reason: 'An exhausted visual QA cycle is a user fork, not a silent wait: show the current frames and remaining findings, offer another repair round or skipping the named check (`ovs draft --waive <code>`), and end the turn. The user\'s reply grants the next cycle; never restart one as the silent default.',
});
}

Expand Down
27 changes: 24 additions & 3 deletions packages/core/src/ir/edl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ export function validateEdl(obj: unknown): EdlValidation {
referenceIds.add(reference.id);
}
if (!VIDEO_REFERENCE_MEDIA_TYPES.includes(reference.media_type as VideoReferenceMediaType)) {
err(`${at}.media_type`, 'E_REFERENCE_MEDIA_TYPE', 'media_type must be image or video');
err(`${at}.media_type`, 'E_REFERENCE_MEDIA_TYPE', `media_type must be one of ${VIDEO_REFERENCE_MEDIA_TYPES.join(' | ')}`);
}
if (!isStr(reference.source)) {
err(`${at}.source`, 'E_REFERENCE_SOURCE', 'reference source path or URL is required');
Expand Down Expand Up @@ -405,7 +405,7 @@ export function validateEdl(obj: unknown): EdlValidation {
if (Array.isArray(reference.roles)) {
for (const role of reference.roles) {
if (!VIDEO_REFERENCE_ROLES.includes(role as VideoReferenceRole)) {
err(`${at}.roles`, 'E_REFERENCE_ROLE', `unknown reference role "${String(role)}"`);
err(`${at}.roles`, 'E_REFERENCE_ROLE', `unknown reference role "${String(role)}" — must be one of ${VIDEO_REFERENCE_ROLES.join(' | ')}`);
}
}
}
Expand Down Expand Up @@ -476,7 +476,7 @@ export function validateEdl(obj: unknown): EdlValidation {
if (Array.isArray(editStrategy.decision_signals)) {
for (const signal of editStrategy.decision_signals) {
if (!VIDEO_EDIT_DECISION_SIGNALS.includes(signal as VideoEditDecisionSignal)) {
err('edit_strategy.decision_signals', 'E_EDIT_STRATEGY_SIGNAL', `unknown decision signal "${String(signal)}"`);
err('edit_strategy.decision_signals', 'E_EDIT_STRATEGY_SIGNAL', `unknown decision signal "${String(signal)}" — must be one of ${VIDEO_EDIT_DECISION_SIGNALS.join(' | ')}`);
}
}
}
Expand Down Expand Up @@ -619,6 +619,27 @@ export function validateEdl(obj: unknown): EdlValidation {
warn(`tracks.narration.segments[${i}].produced_path`, 'W_NARRATION_PRODUCED', 'produced_path should be a string path when present');
}
});
// Overlapping line windows produce two voices at once at mix time.
// The classic mistake is writing `target_sec` as each line's END
// time instead of its duration — windows like [11, +20] swallow
// their successors and the shipped mix carries double narration.
// Catch it at free validation, before any synthesis is attempted.
const windows = nar.segments
.map((ln, i) => ({ i, start: isObject(ln) ? Number(ln.start_sec) : NaN, dur: isObject(ln) ? Number(ln.target_sec) : NaN }))
.filter((w) => Number.isFinite(w.start) && Number.isFinite(w.dur) && w.dur > 0)
.sort((a, z) => a.start - z.start);
for (let k = 1; k < windows.length; k += 1) {
const prev = windows[k - 1];
const cur = windows[k];
const overlapSec = prev.start + prev.dur - cur.start;
if (overlapSec > 0.05) {
err(
`tracks.narration.segments[${prev.i}]`,
'E_NARRATION_WINDOWS_OVERLAP',
`line window [${prev.start}s +${prev.dur}s] runs ${overlapSec.toFixed(2)}s into the next line at ${cur.start}s — target_sec is the line DURATION, not its end time; two overlapping windows mix as two voices speaking at once`,
);
}
}
} else {
warn('tracks.narration', 'W_EMPTY_TRACK_DISABLED', 'empty narration is disabled; omit it or use null');
}
Expand Down
53 changes: 53 additions & 0 deletions packages/core/test/edl.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,59 @@ describe('validateEdl — references and specs', () => {
});
});

// --- validateEdl: narration windows ----------------------------------------

describe('validateEdl — narration windows', () => {
const narrated = (segments: Array<Record<string, unknown>>) =>
plan({ tracks: { narration: { voice: 'demo-voice', segments } } });

it('rejects overlapping line windows (target_sec written as an end time)', () => {
// [11s +20s] is the end-time mistake: the window runs to 31s and swallows
// the line that starts at 20s.
const r = validateEdl(narrated([
{ text: 'line one', start_sec: 11, target_sec: 20 },
{ text: 'line two', start_sec: 20, target_sec: 8 },
]));
expect(codes(r.errors)).toContain('E_NARRATION_WINDOWS_OVERLAP');
const issue = r.errors.find((e) => e.code === 'E_NARRATION_WINDOWS_OVERLAP');
expect(issue?.message).toContain('DURATION');
});

it('accepts adjacent, non-overlapping windows', () => {
const r = validateEdl(narrated([
{ text: 'line one', start_sec: 0, target_sec: 5 },
{ text: 'line two', start_sec: 5.02, target_sec: 4 },
]));
expect(codes(r.errors)).not.toContain('E_NARRATION_WINDOWS_OVERLAP');
});

it('skips lines without usable timing instead of failing them', () => {
const r = validateEdl(narrated([
{ text: 'untimed line' },
{ text: 'timed line', start_sec: 3, target_sec: 4 },
]));
expect(codes(r.errors)).not.toContain('E_NARRATION_WINDOWS_OVERLAP');
});

it('names the legal values when rejecting an unknown decision signal', () => {
const r = validateEdl(plan({
delivery_promise: { type: 'source_led', source_required: true, motion_min_ratio: 0 },
segments: [
{ id: 's1', order: 1, role: 'body', layer: 'primary', source: 'edit', target_sec: 30, spec: { input_id: 'a', in_sec: 0, out_sec: 30 } },
],
edit_strategy: {
mode: 'highlight',
objectives: ['keep the goal'],
decision_signals: ['not-a-signal'],
preserve: ['intro'],
may_change: ['pacing'],
} as never,
}));
const issue = r.errors.find((e) => e.code === 'E_EDIT_STRATEGY_SIGNAL');
expect(issue?.message).toContain('must be one of');
});
});

// --- validateEdl: promise consistency --------------------------------------

describe('validateEdl — promise consistency', () => {
Expand Down
45 changes: 43 additions & 2 deletions packages/core/test/gate-transition.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,56 @@ describe('resolveGateTransition', () => {
expect(result.prohibited_ops).toContain('emit_form');
});

it('reports an exhausted QA blocker without creating a recovery form', () => {
it('turns an exhausted QA cycle into a user fork, not a silent wait', () => {
const result = resolveGateTransition({
line: 'compose',
artifact: 'composition',
recovery: 'available',
errorCode: 'E_VISUAL_REVISION_EXPLICIT_AUTHORIZATION_REQUIRED',
});
expect(result).toMatchObject({ next_action: 'report_visual_qa_blocker', form: null });
expect(result).toMatchObject({ next_action: 'present_findings_and_ask_user_direction', form: null });
expect(result.prohibited_ops).toContain('emit_form');
expect(result.prohibited_ops).toContain('edit_files');
expect(result.prohibited_ops).toContain('restart_visual_qa_cycle');
expect(result.reason).toMatch(/materially different edit/i);
});

it('offers the fork on bare exhausted recovery with no decision at all', () => {
const result = resolveGateTransition({
line: 'compose',
artifact: 'composition',
recovery: 'available',
});
expect(result).toMatchObject({ next_action: 'present_findings_and_ask_user_direction', form: null });
expect(result.reason).toMatch(/waive|skipping/i);
});

it('applies a user-dictated amendment directly instead of re-confirming it', () => {
const result = resolveGateTransition({
line: 'compose',
artifact: 'composition',
gate: 'gate_b',
decision: 'revise',
scope: 'gate_b_payload',
origin: 'user',
});
expect(result).toMatchObject({ next_action: 'apply_user_instruction_then_approve_plan', form: null });
expect(result.authorities).toContain('approve_gate_b');
expect(result.reason).toMatch(/own words/i);
});

it('keeps the Gate B amendment for model-initiated or mixed changes', () => {
for (const origin of ['model', 'unknown'] as const) {
const result = resolveGateTransition({
line: 'compose',
artifact: 'composition',
gate: 'gate_b',
decision: 'revise',
scope: 'gate_b_payload',
origin,
});
expect(result.next_action).toBe('open_gate_b_amendment');
}
});

it('rejects mixed current and legacy decision fields', () => {
Expand Down
Loading
Loading