refactor(orchestration): migrate prompt builders to file-based templates

[wyuc]

Summary

Phase 1 of a two-phase refactor of the orchestration prompts subsystem.

Migrates three orchestration-tier prompt builders (buildStructuredPrompt, buildDirectorPrompt, buildPBLSystemPrompt) from inline TS template literals to file-based markdown templates, sharing the loader infrastructure already used by the generation pipeline.

Unblocks much of Phase 2 content optimization via markdown edits (LaTeX few-shot examples, speech guidelines, snippet additions). A subset — viewport bounds (1000×562), role-specific length targets, and role guidelines — still lives as TS template literals in prompt-builder.ts and will migrate in a follow-up extraction pass.

What changed

• lib/generation/prompts/ → lib/prompts/ (project-level loader, used by both generation + orchestration)
• lib/orchestration/prompt-builder.ts 890 → 314 lines (thin assembler)
• lib/orchestration/director-prompt.ts 290 → 271 lines (thin assembler)
• lib/pbl/pbl-system-prompt.ts 93 → 30 lines (thin assembler)
• New lib/orchestration/summarizers/ — 5 focused modules
• New lib/orchestration/types.ts — shared WhiteboardActionRecord + AgentTurnSummary
• New templates: lib/prompts/templates/{agent-system,director,pbl-design}/system.md
• New snippet: lib/prompts/snippets/speech-guidelines.md
• New lib/prompts/README.md — conventions, syntax, gotchas, local-testing recipe for template authors
• PROMPT_IDS gains as const satisfies Record<string, PromptId> type guard

Test posture

• tests/prompts/loader.test.ts — bounds the loader infrastructure (3 tests)
• tests/prompts/templates.test.ts — bounds behavior-level invariants (8 tests): no unresolved {{...}} placeholders in any rendered prompt, role dispatch (teacher → LEAD TEACHER, student → not), scene-type action stripping (quiz scene strips spotlight/laser), director output-spec shape
• pnpm eval:whiteboard — bounds end-to-end composition. 1-scenario smoke run on this branch passed (agent loop ran 3 turns, 2 screenshots captured, scores within sampling noise of pre-refactor baseline)

An earlier iteration of this branch added ~1900 lines of byte-equal snapshot tests as a refactor safety net. Those were removed after review surfaced that byte-equality was the wrong invariant for a refactor whose goal is enabling Phase 2 content changes — every intentional Phase 2 tweak would have produced large snapshot diffs for no benefit beyond what the eval and structural assertions already cover.

Phase 2 deferrals (deliberate)

• ROLE_GUIDELINES, buildLengthGuidelines, buildWhiteboardGuidelines remain as TS template literals in prompt-builder.ts — role-conditional prose; snippet extraction deferred until eval-driven changes show what needs the treatment.
• lib/prompts/templates/slide-content/{system,user}.md still uses snake_case placeholders (pre-existing). Normalize when generation pipeline gets a similar pass. The new README flags this.

Test plan

• npx tsc --noEmit clean
• pnpm lint zero errors
• pnpm check (Prettier) clean
• pnpm test tests/prompts — 10/10 pass
• 1-scenario end-to-end smoke eval passed
• CI green

Note: 3 unrelated failures in tests/server/provider-config.test.ts are environment-config tests that don't sandbox process.env; they fail in any setup with .env.local containing provider keys (verified same 3 fail on a fresh main checkout). Not a regression from this branch.

🤖 Generated with Claude Code

[wyuc]

Agent Code Review Process

This PR was developed and reviewed using superpowers:subagent-driven-development. 20 subagent dispatches total — 7 implementers, 5 spec-compliance reviewers, 5 code-quality reviewers, 3 follow-up fix implementers, 2 final PR-level reviews. Every task ran the cycle: implementer → spec review → code-quality review.

Substantive findings and how they were addressed

Stage	Reviewer finding	Action taken
Task 2 snapshot coverage	The original convertMessagesToOpenAI test passed currentAgentId equal to the message's agentId, so the cross-agent role-conversion branch was never exercised. Variant 7 ("whiteboard-open scene") was mis-named: the actual spotlight/laser strip path (non-slide scene types) was uncovered.	Split into same-agent + cross-agent tests; added a real quiz-scene variant to cover the strip path; added an assistant-role variant.
Task 4 placeholder convention	agent-system template used snake_case placeholders while other templates (slide-actions, slide-content) use camelCase. Inconsistency would compound in Task 5.	Renamed all 17 placeholders to camelCase; extracted the static ## Speech Guidelines (CRITICAL) block into a reusable speech-guidelines.md snippet.
Final PR review — type placement	WhiteboardActionRecord and AgentTurnSummary lived in lib/orchestration/director-prompt.ts but were imported by 6 modules including the neutral summarizers/ modules, creating an upward import direction.	Extracted to lib/orchestration/types.ts; updated all 8 importers.
User observation	Byte-equal snapshot tests (~1900 lines of .snap) locked the wrong invariant for this refactor — the goal is a stable substrate for future prompt editing, not frozen bytes. Phase 2 tweaks would produce large snapshot diffs per commit for no benefit beyond what the eval already provides.	Removed the full snapshot suite.
Post-update review	Missing lib/prompts/README.md (non-engineer editability was a stated goal). The PR description's "Phase 2 is pure markdown editing" claim didn't hold: viewport bounds, length targets, and role guidelines remain in TS. Two trailing empty ci: commits (workflow re-trigger attempts) polluted history.	Added lib/prompts/README.md (syntax, naming conventions, silent-passthrough gotcha, local-testing recipe); added 8 structural assertion tests (tests/prompts/templates.test.ts) covering no-unresolved-placeholders, role dispatch, scene-type action stripping, and director output-spec shape; softened Phase 2 language in PR body; squashed out the empty commits.

Final state

• pnpm test tests/prompts passes (10 tests: 3 loader + 8 structural)
• npx tsc --noEmit, pnpm lint, pnpm check all clean
• 1-scenario end-to-end smoke eval (econ-tech-innovation) passed: agent loop ran 3 turns, 2 screenshots captured
• Previous CI run on dee3abd was all green; this incremental (~235 lines README + tests, plus squash) needs CI to re-trigger after the force-push.

[cosarah]

Behavior preservation looks good to me for the three migrated prompts — summarizer extraction, types.ts move, and template naming convention are all improvements. Two things worth doing before merge, plus one real bug that's low-impact today but worth fixing.

Important

• Structural tests cover only about half the conditional surface. tests/prompts/templates.test.ts asserts role dispatch and scene-type stripping but skips the branches Phase 2 is most likely to touch:

  • Director discussion-context branch — lib/orchestration/director-prompt.ts:51-60
  • Peer context section — lib/orchestration/prompt-builder.ts:145 (largest single conditional block in agent-system)
  • Assistant role — teacher and student are tested; buildLengthGuidelines/buildWhiteboardGuidelines assistant branches are silent
  • Language directive — fixture always sets languageDirective: 'zh-CN'; the null path isn't exercised
  • PBL {{issueCount}} appears three times in the template; no assertion confirms all three are filled

  These are small additions, not a re-snapshot.

• README doesn't tell editors what is still in TS. The PR body correctly notes viewport bounds (1000×562), role-specific length targets, and ROLE_GUIDELINES still live in prompt-builder.ts. A contributor reading lib/prompts/README.md expecting a pure-markdown workflow won't know to go there. A short "Still in TS" section would prevent a real friction point.

Minor

• Snippet typos ship silently. lib/prompts/loader.ts:44 returns the literal string `{{snippet:${id}}}` on a missing snippet and only logs a warn. The UNRESOLVED_PLACEHOLDER regex at tests/prompts/templates.test.ts:88 is /\{\{\w[\w-]*\}\}/, which does not match `{{snippet:foo}}` — the : breaks it. A typo like `{{snippet:speach-guidelines}}` would reach the LLM with no test failure. Either broaden the regex (e.g. /\{\{(\w[\w-]*|snippet:[\w-]+)\}\}/) or have loadSnippet throw on missing files.

• interpolateVariables regex is narrower than processSnippets. loader.ts:102 uses \{\{(\w+)\}\} — a kebab-case placeholder like `{{next-agent}}` would silently pass through. The README mandates camelCase so it's not a bug today; worth either a comment above the regex or a lint test that scans every templates/*/{system,user}.md for non-camelCase placeholders.

• lib/orchestration/summarizers/peer-context.ts has a single consumer. Not a blocker; flag if the one-caller situation persists after Phase 2 — at that point folding it back into prompt-builder.ts removes indirection.

Verdict

Approve with nits. The testing and README suggestions would substantially improve Phase 2's downstream safety; the snippet-regex fix is real but low-impact. Nothing here is structural.

[wyuc]

Thanks @cosarah. All 4 handled in 0c5fefa.

Brief on why each slipped:

• Tests half-covered the conditionals — I wrote the structural assertions from memory after removing the snapshots, rather than walking the template conditionals. Re-did it by reading the {{...}} call sites; 9 more assertions cover peer/language/assistant/discussion/issueCount.
• README missing "Still in TS" — I documented how the infra works but not where each kind of content lives. A Phase 2 contributor arrives needing the inverse map.
• loadSnippet silent passthrough — pre-existing behavior inherited from the generation loader; I didn't audit its error paths when promoting it. Now throws.
• Regex asymmetry — didn't put interpolateVariables's \w+ next to processSnippets's \w[\w-]*. Added a lint test so any kebab-case drift fails loud.

Leaving the peer-context.ts single-consumer note for Phase 2 as you suggested.

[cosarah]

tested locally, LGTM