Skip to content

Commit b53691c

Browse files
committed
fix(protocol): drop the conditional empty spread in scene normalization
The enforced anti-slop rule `no-conditional-empty-object-spread` rejects it. A plain assignment keeps the same behaviour: a screenplay that carried no scenes stays without the key rather than gaining an empty one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TL39a2ajm5iVkRrnjbVER4
1 parent e69816d commit b53691c

22 files changed

Lines changed: 1548 additions & 63 deletions

package-lock.json

Lines changed: 4 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/agents/package.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,12 @@
102102
"import": "./dist/tools/base-tool.js",
103103
"default": "./dist/tools/base-tool.js"
104104
},
105+
"./host-modules/mammoth": {
106+
"nodetool-dev": "./src/host-modules/mammoth.ts",
107+
"types": "./dist/host-modules/mammoth.d.ts",
108+
"import": "./dist/host-modules/mammoth.js",
109+
"default": "./dist/host-modules/mammoth.js"
110+
},
105111
"./js-sandbox": {
106112
"nodetool-dev": "./src/js-sandbox.ts",
107113
"types": "./dist/js-sandbox.d.ts",
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/**
2+
* PDF text extraction as a pure function: bytes in, per-page text out.
3+
*
4+
* The `lib.pdf.*` nodes resolve a document ref and carry the node-sdk registry
5+
* with them. Callers that only have a buffer — the websocket import route —
6+
* import this module instead, so they load liteparse (and its bundled pdfium)
7+
* without loading the nodes.
8+
*/
9+
import type { ParseResult } from "@llamaindex/liteparse";
10+
11+
/**
12+
* Parse a PDF buffer. `liteparse` is imported lazily because it pulls in
13+
* pdfium and pdf.js, which cost far more than this module's own load.
14+
*/
15+
export async function parsePdfBuffer(buffer: Buffer): Promise<ParseResult> {
16+
const { LiteParse } = await import("@llamaindex/liteparse");
17+
const parser = new LiteParse({ ocrEnabled: false });
18+
return parser.parse(buffer, true);
19+
}
20+
21+
export interface PdfTextResult {
22+
/** Every page's text, joined by a blank line. Empty for a scanned PDF. */
23+
text: string;
24+
/** Pages liteparse could read. Zero means the file is not a readable PDF. */
25+
pages: number;
26+
}
27+
28+
/** A PDF's whole text layer. No OCR: a scanned page contributes nothing. */
29+
export async function extractPdfText(buffer: Buffer): Promise<PdfTextResult> {
30+
const result = await parsePdfBuffer(buffer);
31+
return {
32+
text: result.pages
33+
.map((page) => page.text)
34+
.join("\n\n")
35+
.trim(),
36+
pages: result.pages.length
37+
};
38+
}

packages/document-nodes/src/nodes/lib-pdf.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
requireDocumentBytes,
1313
type DocumentRefLike
1414
} from "../document-bytes.js";
15+
import { parsePdfBuffer } from "../lib/pdf-text.js";
1516

1617
async function resolvePdfBuffer(
1718
pdf: DocumentRefLike,
@@ -24,10 +25,7 @@ async function parsePdf(
2425
pdf: DocumentRefLike,
2526
context?: ProcessingContext
2627
): Promise<ParseResult> {
27-
const { LiteParse } = await import("@llamaindex/liteparse");
28-
const pdfBuffer = await resolvePdfBuffer(pdf, context);
29-
const parser = new LiteParse({ ocrEnabled: false });
30-
return parser.parse(pdfBuffer, true);
28+
return parsePdfBuffer(await resolvePdfBuffer(pdf, context));
3129
}
3230

3331
function resolvePageRange(

packages/protocol/src/api-schemas/storyboards.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -231,22 +231,22 @@ export function normalizeStoryboardScreenplay(
231231
);
232232
}
233233
const play = applyAliases(input, SCREENPLAY_KEY_ALIASES);
234-
const candidate = {
234+
const candidate: Record<string, unknown> = {
235235
...play,
236236
type: "screenplay",
237237
id: isNonEmptyString(play.id) ? play.id : newId(),
238238
title: isString(play.title) ? play.title : "",
239239
shots: (play.shots as unknown[]).map((shot, index) =>
240240
normalizeStoryboardShot(shot, index, options)
241-
),
242-
...(Array.isArray(play.scenes)
243-
? {
244-
scenes: (play.scenes as unknown[]).map((scene, index) =>
245-
normalizeStoryboardScene(scene, index, options)
246-
)
247-
}
248-
: {})
241+
)
249242
};
243+
// Only overwrite `scenes` when the payload carried one, so a screenplay
244+
// without scenes stays without the key rather than gaining an empty one.
245+
if (Array.isArray(play.scenes)) {
246+
candidate.scenes = (play.scenes as unknown[]).map((scene, index) =>
247+
normalizeStoryboardScene(scene, index, options)
248+
);
249+
}
250250
const parsed = storyboardScreenplay.safeParse(candidate);
251251
if (!parsed.success) {
252252
const paths = parsed.error.issues

packages/protocol/src/creative.ts

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -222,29 +222,6 @@ export type VersionRef<T> = T & { render_inputs?: RenderInputs };
222222
export type KeyframeVersion = VersionRef<ImageRef>;
223223
export type ClipVersion = VersionRef<VideoRef>;
224224

225-
/**
226-
* The render record on a version ref, or null. Defensive because refs arrive
227-
* from stored documents and from providers, where the field is absent or
228-
* whatever the document held.
229-
*/
230-
export function versionRenderInputs(ref: unknown): RenderInputs | null {
231-
if (!ref || typeof ref !== "object") {
232-
return null;
233-
}
234-
const record = (ref as { render_inputs?: unknown }).render_inputs;
235-
if (!record || typeof record !== "object") {
236-
return null;
237-
}
238-
const { kind, prompt_hash, model } = record as Partial<RenderInputs>;
239-
if (kind !== "keyframe" && kind !== "clip") {
240-
return null;
241-
}
242-
if (typeof prompt_hash !== "string" || typeof model !== "string") {
243-
return null;
244-
}
245-
return record as RenderInputs;
246-
}
247-
248225
/**
249226
* Field-by-field equality of two render records, ignoring `recorded_at` — a
250227
* timestamp is not an input, and every re-render would otherwise read as

packages/protocol/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ export {
2525
} from "./wrap-primitives.js";
2626
export * from "./toolSchemas.js";
2727
export * from "./creative.js";
28+
export * from "./shot-prompt.js";
2829
export * from "./screenplay-authoring.js";
2930
export * from "./script-link.js";
3031
export * from "./sha256.js";
Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
/**
2+
* Shot → prompt composition.
3+
*
4+
* One pure module so the editor (`useGenerateShot`) and the headless render
5+
* capabilities (`render_storyboard_stills`, `render_storyboard_clips`) send the
6+
* same words to the same model. A prompt that differs between the two surfaces
7+
* makes a board impossible to reason about: the same shot would come back
8+
* looking different depending on who pressed render.
9+
*
10+
* The field → mode mapping is the contract (PRD § 7.7.5):
11+
*
12+
* | Field | Still | Clip (keyframe) | Clip (direct) |
13+
* | ------------------ | ---------------- | --------------- | --------------- |
14+
* | `action` | yes | yes | yes |
15+
* | `camera.framing` | `<framing> shot` | — | `<framing> shot`|
16+
* | `camera.angle` | yes | — | yes |
17+
* | `camera.lens` | `<lens> lens` | — | `<lens> lens` |
18+
* | scene `lighting` | yes | — | yes |
19+
* | `motion` | — | yes | yes |
20+
* | `camera.movement` | — | yes | yes |
21+
* | `camera.equipment` | — | yes | yes |
22+
* | board `style` | yes | — | yes |
23+
*
24+
* A keyframe-mode clip animates a still that already carries framing, lens,
25+
* lighting and style, so repeating them in the video prompt only fights the
26+
* first-frame conditioning. A direct clip has no still, so it carries
27+
* everything.
28+
*
29+
* `dialogue`, `notes` and `duration_seconds` never enter a prompt: the first
30+
* two are words for people, the third is a render parameter.
31+
*
32+
* Entity seasoning is not here — the two surfaces pass entities differently
33+
* (the editor appends `entity://` tokens for the server to expand, the
34+
* capabilities send a structured `entities` param), and both start from
35+
* `entitiesForShot`.
36+
*/
37+
38+
import type { Scene, Shot } from "./creative.js";
39+
40+
/** What a prompt needs beyond the shot itself. */
41+
export interface ShotPromptContext {
42+
/** The shot's scene, when it has one. Supplies `lighting`. */
43+
scene?: Scene | null;
44+
/** The board's style descriptor, applied to every shot. */
45+
style?: string;
46+
}
47+
48+
/** Trimmed, comma-separated, empties dropped. */
49+
const compose = (parts: (string | undefined | null)[]): string =>
50+
parts
51+
.map((part) => (part ?? "").trim())
52+
.filter((part) => part.length > 0)
53+
.join(", ");
54+
55+
/** `"85mm"` + `"lens"` → `"85mm lens"`, so the model reads it as direction. */
56+
const qualified = (
57+
value: string | undefined,
58+
noun: string
59+
): string | undefined => {
60+
const trimmed = (value ?? "").trim();
61+
return trimmed.length > 0 ? `${trimmed} ${noun}` : undefined;
62+
};
63+
64+
/**
65+
* The scene a shot belongs to, or null when it has none (legacy shots) or the
66+
* scene has been dropped. Both render surfaces need it to reach `lighting`.
67+
*/
68+
export function sceneForShot(
69+
shot: Shot,
70+
scenes?: readonly Scene[] | null
71+
): Scene | null {
72+
if (!shot.scene_id || !scenes) {
73+
return null;
74+
}
75+
return scenes.find((scene) => scene.id === shot.scene_id) ?? null;
76+
}
77+
78+
/** Still prompt: what is in frame, how it is shot, how it is lit, the look. */
79+
export function keyframePrompt(
80+
shot: Shot,
81+
context: ShotPromptContext = {}
82+
): string {
83+
const camera = shot.camera;
84+
return compose([
85+
shot.action,
86+
qualified(camera?.framing, "shot"),
87+
camera?.angle,
88+
qualified(camera?.lens, "lens"),
89+
context.scene?.lighting,
90+
context.style
91+
]);
92+
}
93+
94+
/**
95+
* Keyframe-mode clip prompt: what moves, what is in frame, and how the camera
96+
* moves. Framing, lighting and style come from the still being animated, so
97+
* they are deliberately absent — this takes no context.
98+
*/
99+
export function clipPrompt(shot: Shot): string {
100+
const camera = shot.camera;
101+
return compose([
102+
shot.motion,
103+
shot.action,
104+
camera?.movement,
105+
camera?.equipment
106+
]);
107+
}
108+
109+
/**
110+
* Direct-mode clip prompt: no still carries the look into the render, so the
111+
* prompt carries all of it — the still's fields plus the motion ones.
112+
*/
113+
export function directClipPrompt(
114+
shot: Shot,
115+
context: ShotPromptContext = {}
116+
): string {
117+
const camera = shot.camera;
118+
return compose([
119+
shot.action,
120+
qualified(camera?.framing, "shot"),
121+
camera?.angle,
122+
qualified(camera?.lens, "lens"),
123+
context.scene?.lighting,
124+
shot.motion,
125+
camera?.movement,
126+
camera?.equipment,
127+
context.style
128+
]);
129+
}

packages/protocol/tests/creative-scenes.test.ts

Lines changed: 15 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,5 @@
11
import { describe, expect, it } from "vitest";
2-
import {
3-
isScene,
4-
isScreenplay,
5-
renderInputsMatch,
6-
versionRenderInputs
7-
} from "../src/creative.js";
2+
import { isScene, isScreenplay, renderInputsMatch } from "../src/creative.js";
83
import type {
94
ClipVersion,
105
KeyframeVersion,
@@ -109,8 +104,8 @@ describe("renderInputsMatch", () => {
109104
});
110105
});
111106

112-
describe("versionRenderInputs", () => {
113-
it("reads the record off a version ref", () => {
107+
describe("render_inputs on a version ref", () => {
108+
it("rides on the ref, so a shot's versions carry what produced them", () => {
114109
const keyframe: KeyframeVersion = {
115110
type: "image",
116111
asset_id: "as_1",
@@ -121,24 +116,20 @@ describe("versionRenderInputs", () => {
121116
asset_id: "as_2",
122117
render_inputs: inputs({ kind: "clip", source_version_id: "as_1" })
123118
};
124-
expect(versionRenderInputs(keyframe)).toEqual(inputs());
125-
expect(versionRenderInputs(clip)?.source_version_id).toBe("as_1");
119+
const rendered = shot({
120+
keyframe,
121+
keyframe_versions: [keyframe],
122+
clip,
123+
clip_versions: [clip],
124+
status: "rendered"
125+
});
126126

127-
const rendered = shot({ keyframe, keyframe_versions: [keyframe], clip });
128-
expect(versionRenderInputs(rendered.keyframe_versions?.[0])).toEqual(
129-
inputs()
130-
);
127+
expect(rendered.keyframe_versions?.[0].render_inputs).toEqual(inputs());
128+
expect(rendered.clip?.render_inputs?.source_version_id).toBe("as_1");
131129
});
132130

133-
it("returns null for a ref with no usable record", () => {
134-
expect(versionRenderInputs({ type: "image", asset_id: "as_1" })).toBeNull();
135-
expect(versionRenderInputs(null)).toBeNull();
136-
expect(versionRenderInputs("as_1")).toBeNull();
137-
expect(
138-
versionRenderInputs({ render_inputs: { kind: "poster" } })
139-
).toBeNull();
140-
expect(
141-
versionRenderInputs({ render_inputs: { kind: "clip", model: 7 } })
142-
).toBeNull();
131+
it("is absent on a version that predates the record", () => {
132+
const legacy: KeyframeVersion = { type: "image", asset_id: "as_0" };
133+
expect(legacy.render_inputs).toBeUndefined();
143134
});
144135
});

packages/websocket/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
"@nodetool-ai/chat": "*",
3939
"@nodetool-ai/compute": "*",
4040
"@nodetool-ai/config": "*",
41+
"@nodetool-ai/document-nodes": "*",
4142
"@nodetool-ai/dsl": "^0.7.0-rc.36",
4243
"@nodetool-ai/elevenlabs-nodes": "*",
4344
"@nodetool-ai/execution": "*",

0 commit comments

Comments
 (0)