Просмотр исходного кода

feat(planner): throw on LLM failure, require ChapterBrief in output

Remove runtime fallback to legacy rule-based planner. If the LLM
planner returns invalid JSON, the pipeline now throws immediately
instead of silently falling back to injectStructuralSkills.

Recovery strategy: git revert, not runtime fallback.

- PlanChapterOutput.brief is now required (was optional)
- tryPlanChapterBrief renamed to planChapterBrief, removes try/catch
- Remove !brief guard + legacy fallback block in planChapter
- Simplify composer: remove brief-null guards in deriveRetrievalHints
  and collectSelectedContext
- persisted-governed-plan: return null when brief section is missing
- Update planner tests: add default chat mock that echoes Goal Seed,
  update legacy-fallback test to verify throw behavior
Ma 7 месяцев назад
Родитель
Сommit
4bc0497c8f

+ 82 - 18
packages/core/src/__tests__/planner.test.ts

@@ -73,6 +73,73 @@ describe("PlannerAgent", () => {
     ]);
     ]);
   });
   });
 
 
+  /**
+   * Default chat mock installed on the prototype: echoes the derived goal
+   * from the planner user prompt as the brief's goal, so legacy
+   * goal-derivation tests keep working. Individual tests can override
+   * with their own vi.spyOn on a specific instance.
+   */
+  beforeEach(() => {
+    vi.spyOn(
+      PlannerAgent.prototype as unknown as { chat: (...args: unknown[]) => Promise<{ content: string }> },
+      "chat",
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    ).mockImplementation(async (...args: unknown[]) => {
+      const messages = args[0] as Array<{ role: string; content: string }>;
+      const userMsg = messages.find((m) => m.role === "user");
+      const content = userMsg?.content ?? "";
+
+      function isPlaceholder(line: string): boolean {
+        return /^\(.*\)$/.test(line) || /^\(文件尚未创建\)/.test(line);
+      }
+
+      // Extract first substantive line from a prompt section
+      function extractFirstContent(section: string): string | undefined {
+        const re = new RegExp(`## ${section}\\n([\\s\\S]*?)(?=\\n## |$)`);
+        const block = content.match(re)?.[1];
+        if (!block) return undefined;
+        const lines = block.split("\n").map((l) => l.trim()).filter(
+          (l) => l.length > 0 && !l.startsWith("#") && !isPlaceholder(l),
+        );
+        // Prefer non-bullet text, but fall back to first bullet stripped of "- "
+        const plain = lines.find((l) => !l.startsWith("-"));
+        if (plain) return plain;
+        // Join multiple bullet items for richer goal extraction (mimics deriveGoal combining)
+        return lines
+          .filter((l) => l.startsWith("- "))
+          .map((l) => l.slice(2))
+          .slice(0, 3)
+          .join(";") || undefined;
+      }
+
+      const goalSeed = extractFirstContent("Goal Seed");
+      const focus = extractFirstContent("Current Focus");
+      const authorIntent = extractFirstContent("Author Intent");
+      const goal = (goalSeed && goalSeed !== "(no matched outline slice)" ? goalSeed : undefined)
+        ?? focus
+        ?? authorIntent
+        ?? "advance the story";
+      const chapterMatch = content.match(/## Chapter\n(\d+)/);
+      const chapter = chapterMatch ? Number(chapterMatch[1]) : 3;
+
+      return {
+        content: JSON.stringify({
+          chapter,
+          goal,
+          chapterType: "推进",
+          isGoldenOpening: chapter <= 3,
+          beatOutline: [
+            { phase: "opening", instruction: "open scene" },
+            { phase: "development", instruction: "develop conflict" },
+            { phase: "hook", instruction: "end with hook" },
+          ],
+          hookPlan: [],
+          propsAndSetting: [],
+        }),
+      };
+    });
+  });
+
   afterEach(async () => {
   afterEach(async () => {
     await rm(root, { recursive: true, force: true });
     await rm(root, { recursive: true, force: true });
   });
   });
@@ -192,14 +259,14 @@ describe("PlannerAgent", () => {
     });
     });
 
 
     expect(result.intent.outlineNode).toContain("merchant guild's escape route");
     expect(result.intent.outlineNode).toContain("merchant guild's escape route");
-    expect(result.intent.goal).toContain("mentor debt confrontation");
-    expect(result.intent.goal).not.toContain("merchant guild's escape route");
+    // goal is now LLM-derived from brief; verify conflict detection still works
     expect(result.intent.conflicts).toEqual(expect.arrayContaining([
     expect(result.intent.conflicts).toEqual(expect.arrayContaining([
       expect.objectContaining({
       expect.objectContaining({
         type: "outline_vs_current_focus",
         type: "outline_vs_current_focus",
         resolution: "allow explicit current focus override",
         resolution: "allow explicit current focus override",
       }),
       }),
     ]));
     ]));
+    expect(result.brief).toBeDefined();
   });
   });
 
 
   it("keeps external context above both outline anchors and current focus", async () => {
   it("keeps external context above both outline anchors and current focus", async () => {
@@ -243,14 +310,14 @@ describe("PlannerAgent", () => {
       externalContext: "Ignore the canal pursuit for now and force the next chapter into the mentor debt confrontation.",
       externalContext: "Ignore the canal pursuit for now and force the next chapter into the mentor debt confrontation.",
     });
     });
 
 
-    expect(result.intent.goal).toContain("mentor debt confrontation");
-    expect(result.intent.goal).not.toContain("merchant guild's escape route");
+    // goal is now LLM-derived; verify conflict detection for external override
     expect(result.intent.conflicts).toEqual(expect.arrayContaining([
     expect(result.intent.conflicts).toEqual(expect.arrayContaining([
       expect.objectContaining({
       expect.objectContaining({
         type: "outline_vs_request",
         type: "outline_vs_request",
         resolution: "allow local outline deferral",
         resolution: "allow local outline deferral",
       }),
       }),
     ]));
     ]));
+    expect(result.brief).toBeDefined();
   });
   });
 
 
   it("emits structured directives when fallback planning, chapter type repetition, and title collapse stack up", async () => {
   it("emits structured directives when fallback planning, chapter type repetition, and title collapse stack up", async () => {
@@ -318,10 +385,11 @@ describe("PlannerAgent", () => {
       chapterNumber: 4,
       chapterNumber: 4,
     });
     });
 
 
-    expect(result.intent.arcDirective).toContain("fallback");
-    expect(result.intent.sceneDirective).toContain("investigation");
-    expect(result.intent.titleDirective?.toLowerCase()).toContain("ledger");
-    expect(result.intent.moodDirective).toBeUndefined();
+    // With LLM brief path, arc/scene directives are derived from the brief
+    expect(result.intent.arcDirective).toBeDefined();
+    expect(result.intent.sceneDirective).toBeDefined();
+    expect(result.brief).toBeDefined();
+    expect(result.brief.beatOutline.length).toBeGreaterThanOrEqual(1);
   });
   });
 
 
   it("emits a mood directive when recent chapters are all high-tension", async () => {
   it("emits a mood directive when recent chapters are all high-tension", async () => {
@@ -1099,8 +1167,8 @@ describe("PlannerAgent", () => {
       chapterNumber: 2,
       chapterNumber: 2,
     });
     });
 
 
-    expect(result.intent.goal).toContain("private confrontation");
-    expect(result.intent.goal).toContain("missing record");
+    // goal is now LLM-derived from brief; verify other code-derived fields
+    expect(result.brief).toBeDefined();
     expect(result.intent.mustAvoid).toEqual(expect.arrayContaining([
     expect(result.intent.mustAvoid).toEqual(expect.arrayContaining([
       "Do not turn this chapter into a citywide survey of every faction.",
       "Do not turn this chapter into a citywide survey of every faction.",
       "Do not use summary-heavy moralizing paragraphs.",
       "Do not use summary-heavy moralizing paragraphs.",
@@ -1518,14 +1586,14 @@ describe("PlannerAgent", () => {
       chapterNumber: 3,
       chapterNumber: 3,
     });
     });
 
 
-    expect(result.brief?.chapterType).toBe("confrontation");
+    expect(result.brief.chapterType).toBe("confrontation");
     expect(result.intent.goal).toContain("warehouse confrontation");
     expect(result.intent.goal).toContain("warehouse confrontation");
     expect(result.intent.arcDirective).toContain("opening: Open inside the warehouse argument.");
     expect(result.intent.arcDirective).toContain("opening: Open inside the warehouse argument.");
     expect(result.intent.sceneDirective).toContain("warehouse transfer sheet");
     expect(result.intent.sceneDirective).toContain("warehouse transfer sheet");
     await expect(readFile(result.runtimePath, "utf-8")).resolves.toContain("## Chapter Brief");
     await expect(readFile(result.runtimePath, "utf-8")).resolves.toContain("## Chapter Brief");
   });
   });
 
 
-  it("falls back to the legacy planner when the LLM chapter brief is invalid", async () => {
+  it("throws when the LLM chapter brief is invalid", async () => {
     const planner = new PlannerAgent({
     const planner = new PlannerAgent({
       client: {} as ConstructorParameters<typeof PlannerAgent>[0]["client"],
       client: {} as ConstructorParameters<typeof PlannerAgent>[0]["client"],
       model: "test-model",
       model: "test-model",
@@ -1538,14 +1606,10 @@ describe("PlannerAgent", () => {
         content: "{\"bad\":true}",
         content: "{\"bad\":true}",
       });
       });
 
 
-    const result = await planner.planChapter({
+    await expect(planner.planChapter({
       book,
       book,
       bookDir,
       bookDir,
       chapterNumber: 3,
       chapterNumber: 3,
-    });
-
-    expect(result.brief).toBeUndefined();
-    expect(result.intent.goal).toContain("merchant guild's escape route");
-    await expect(readFile(result.runtimePath, "utf-8")).resolves.not.toContain("## Chapter Brief");
+    })).rejects.toThrow("Planner LLM returned invalid ChapterBrief JSON");
   });
   });
 });
 });

+ 12 - 18
packages/core/src/agents/composer.ts

@@ -95,19 +95,17 @@ async function collectSelectedContext(
   language: "zh" | "en",
   language: "zh" | "en",
 ): Promise<ContextPackage["selectedContext"]> {
 ): Promise<ContextPackage["selectedContext"]> {
     const retrievalHints = deriveRetrievalHints(plan);
     const retrievalHints = deriveRetrievalHints(plan);
-    const chapterBriefEntry = plan.brief
-      ? [{
-          source: "runtime/chapter_brief",
-          reason: "Carry the planner's concrete chapter execution brief directly into governed writing.",
-          excerpt: [
-            `type=${plan.brief.chapterType}`,
-            ...plan.brief.beatOutline.map((beat) => `${beat.phase}: ${beat.instruction}`),
-            ...(plan.brief.propsAndSetting.length > 0
-              ? [`props=${plan.brief.propsAndSetting.join(", ")}`]
-              : []),
-          ].join(" | "),
-        }]
-      : [];
+    const chapterBriefEntry = [{
+      source: "runtime/chapter_brief",
+      reason: "Carry the planner's concrete chapter execution brief directly into governed writing.",
+      excerpt: [
+        `type=${plan.brief.chapterType}`,
+        ...plan.brief.beatOutline.map((beat) => `${beat.phase}: ${beat.instruction}`),
+        ...(plan.brief.propsAndSetting.length > 0
+          ? [`props=${plan.brief.propsAndSetting.join(", ")}`]
+          : []),
+      ].join(" | "),
+    }];
 
 
     const entries = await Promise.all([
     const entries = await Promise.all([
       maybeContextSource(storyDir, "current_focus.md", "Current task focus for this chapter."),
       maybeContextSource(storyDir, "current_focus.md", "Current task focus for this chapter."),
@@ -200,10 +198,6 @@ async function collectSelectedContext(
 }
 }
 
 
 function deriveRetrievalHints(plan: PlanChapterOutput): string[] {
 function deriveRetrievalHints(plan: PlanChapterOutput): string[] {
-  if (!plan.brief) {
-    return plan.intent.mustKeep;
-  }
-
   return [
   return [
     ...plan.brief.propsAndSetting,
     ...plan.brief.propsAndSetting,
     ...plan.brief.hookPlan.flatMap((entry) => [entry.hookId, entry.targetEffect]),
     ...plan.brief.hookPlan.flatMap((entry) => [entry.hookId, entry.targetEffect]),
@@ -319,7 +313,7 @@ async function buildHookDebtEntries(
 ): Promise<ContextPackage["selectedContext"]> {
 ): Promise<ContextPackage["selectedContext"]> {
     const targetHookIds = [
     const targetHookIds = [
       ...new Set([
       ...new Set([
-        ...(plan.brief?.hookPlan.map((entry) => entry.hookId) ?? []),
+        ...plan.brief.hookPlan.map((entry) => entry.hookId),
         ...plan.intent.hookAgenda.pressureMap.map((entry) => entry.hookId),
         ...plan.intent.hookAgenda.pressureMap.map((entry) => entry.hookId),
         ...plan.intent.hookAgenda.eligibleResolve,
         ...plan.intent.hookAgenda.eligibleResolve,
         ...plan.intent.hookAgenda.mustAdvance,
         ...plan.intent.hookAgenda.mustAdvance,

+ 38 - 71
packages/core/src/agents/planner.ts

@@ -36,7 +36,7 @@ export interface PlanChapterInput {
 
 
 export interface PlanChapterOutput {
 export interface PlanChapterOutput {
   readonly intent: ChapterIntent;
   readonly intent: ChapterIntent;
-  readonly brief?: ChapterBrief;
+  readonly brief: ChapterBrief;
   readonly intentMarkdown: string;
   readonly intentMarkdown: string;
   readonly plannerInputs: ReadonlyArray<string>;
   readonly plannerInputs: ReadonlyArray<string>;
   readonly runtimePath: string;
   readonly runtimePath: string;
@@ -98,49 +98,22 @@ export class PlannerAgent extends BaseAgent {
       chapterSummaries: materials.chapterSummariesRaw,
       chapterSummaries: materials.chapterSummariesRaw,
     });
     });
 
 
-    const brief = await this.tryPlanChapterBrief({
+    const brief = await this.planChapterBrief({
       input,
       input,
       outlineNode: planningAnchor,
       outlineNode: planningAnchor,
       materials,
       materials,
     });
     });
 
 
-    if (!brief) {
-      // TODO(v10): delete this skill-to-mustKeep bridge once the LLM planner lands.
-      this.injectStructuralSkills({
-        chapterNumber: input.chapterNumber,
-        language: input.book.language ?? "zh",
-        platform: input.book.platform,
-        mustKeep,
-        mustAvoid,
-        styleEmphasis,
-        hookAgenda,
-        cadence: analyzeChapterCadence({
-          language: this.isChineseLanguage(input.book.language) ? "zh" : "en",
-          rows: parseChapterSummariesMarkdown(materials.chapterSummariesRaw)
-            .filter((s) => s.chapter < input.chapterNumber)
-            .sort((a, b) => a.chapter - b.chapter)
-            .slice(-4)
-            .map((s) => ({ chapter: s.chapter, title: s.title, mood: s.mood, chapterType: s.chapterType })),
-        }),
-      });
-    }
-
     const intent = ChapterIntentSchema.parse({
     const intent = ChapterIntentSchema.parse({
       chapter: input.chapterNumber,
       chapter: input.chapterNumber,
-      goal: brief?.goal ?? goal,
+      goal: brief.goal,
       outlineNode,
       outlineNode,
       ...directives,
       ...directives,
       mustKeep,
       mustKeep,
       mustAvoid,
       mustAvoid,
-      styleEmphasis: brief
-        ? this.mergeBriefStyleEmphasis(styleEmphasis, brief)
-        : styleEmphasis,
-      sceneDirective: brief
-        ? this.buildSceneDirectiveFromBrief(brief)
-        : directives.sceneDirective,
-      arcDirective: brief
-        ? this.buildArcDirectiveFromBrief(brief)
-        : directives.arcDirective,
+      styleEmphasis: this.mergeBriefStyleEmphasis(styleEmphasis, brief),
+      sceneDirective: this.buildSceneDirectiveFromBrief(brief),
+      arcDirective: this.buildArcDirectiveFromBrief(brief),
       conflicts,
       conflicts,
       hookAgenda,
       hookAgenda,
     });
     });
@@ -148,7 +121,7 @@ export class PlannerAgent extends BaseAgent {
     const runtimePath = join(runtimeDir, `chapter-${String(input.chapterNumber).padStart(4, "0")}.intent.md`);
     const runtimePath = join(runtimeDir, `chapter-${String(input.chapterNumber).padStart(4, "0")}.intent.md`);
     const intentMarkdown = this.renderIntentMarkdown(
     const intentMarkdown = this.renderIntentMarkdown(
       intent,
       intent,
-      brief ?? undefined,
+      brief,
       input.book.language ?? "zh",
       input.book.language ?? "zh",
       renderHookSnapshot(memorySelection.hooks, input.book.language ?? "zh"),
       renderHookSnapshot(memorySelection.hooks, input.book.language ?? "zh"),
       renderSummarySnapshot(memorySelection.summaries, input.book.language ?? "zh"),
       renderSummarySnapshot(memorySelection.summaries, input.book.language ?? "zh"),
@@ -158,53 +131,47 @@ export class PlannerAgent extends BaseAgent {
 
 
     return {
     return {
       intent,
       intent,
-      brief: brief ?? undefined,
+      brief,
       intentMarkdown,
       intentMarkdown,
       plannerInputs: materials.plannerInputs,
       plannerInputs: materials.plannerInputs,
       runtimePath,
       runtimePath,
     };
     };
   }
   }
 
 
-  private async tryPlanChapterBrief(params: {
+  private async planChapterBrief(params: {
     readonly input: PlanChapterInput;
     readonly input: PlanChapterInput;
     readonly outlineNode: string | undefined;
     readonly outlineNode: string | undefined;
     readonly materials: PlanningMaterials;
     readonly materials: PlanningMaterials;
-  }): Promise<ChapterBrief | null> {
-    try {
-      const language = this.isChineseLanguage(params.input.book.language) ? "zh" : "en";
-      const response = await this.chat([
-        {
-          role: "system",
-          content: buildPlannerSystemPrompt(language),
-        },
-        {
-          role: "user",
-          content: buildPlannerUserPrompt({
-            chapterNumber: params.input.chapterNumber,
-            targetChapters: params.input.book.targetChapters,
-            genreName: params.input.book.genre,
-            language,
-            materials: {
-              ...params.materials,
-              outlineNode: params.outlineNode,
-            },
-          }),
-        },
-      ], {
-        temperature: 0.2,
-        maxTokens: 1600,
-      });
-
-      const parsed = this.tryParseChapterBrief(response.content);
-      if (!parsed) {
-        throw new Error("Planner brief output was not valid JSON.");
-      }
-      return parsed;
-    } catch (error) {
-      const detail = error instanceof Error ? error.message : String(error);
-      this.log?.warn(`[planner] Falling back to legacy planner: ${detail}`);
-      return null;
+  }): Promise<ChapterBrief> {
+    const language = this.isChineseLanguage(params.input.book.language) ? "zh" : "en";
+    const response = await this.chat([
+      {
+        role: "system",
+        content: buildPlannerSystemPrompt(language),
+      },
+      {
+        role: "user",
+        content: buildPlannerUserPrompt({
+          chapterNumber: params.input.chapterNumber,
+          targetChapters: params.input.book.targetChapters,
+          genreName: params.input.book.genre,
+          language,
+          materials: {
+            ...params.materials,
+            outlineNode: params.outlineNode,
+          },
+        }),
+      },
+    ], {
+      temperature: 0.2,
+      maxTokens: 1600,
+    });
+
+    const parsed = this.tryParseChapterBrief(response.content);
+    if (!parsed) {
+      throw new Error(`Planner LLM returned invalid ChapterBrief JSON. Raw output: ${response.content.slice(0, 200)}`);
     }
     }
+    return parsed;
   }
   }
 
 
   private tryParseChapterBrief(text: string): ChapterBrief | null {
   private tryParseChapterBrief(text: string): ChapterBrief | null {

+ 6 - 1
packages/core/src/pipeline/persisted-governed-plan.ts

@@ -36,6 +36,11 @@ export async function loadPersistedPlan(
       })
       })
       .filter((conflict): conflict is { type: string; resolution: string } => conflict !== null);
       .filter((conflict): conflict is { type: string; resolution: string } => conflict !== null);
 
 
+    const brief = parsePersistedChapterBrief(sections, chapterNumber);
+    if (!brief) {
+      return null;
+    }
+
     return {
     return {
       intent: ChapterIntentSchema.parse({
       intent: ChapterIntentSchema.parse({
         chapter: chapterNumber,
         chapter: chapterNumber,
@@ -46,7 +51,7 @@ export async function loadPersistedPlan(
         styleEmphasis: readIntentList(sections, "Style Emphasis"),
         styleEmphasis: readIntentList(sections, "Style Emphasis"),
         conflicts,
         conflicts,
       }),
       }),
-      brief: parsePersistedChapterBrief(sections, chapterNumber),
+      brief,
       intentMarkdown,
       intentMarkdown,
       plannerInputs: [runtimePath],
       plannerInputs: [runtimePath],
       runtimePath,
       runtimePath,