From 33b9de5441bbe11508a3a6ce8f0aaf72c7024639 Mon Sep 17 00:00:00 2001 From: "K. Hodges" Date: Fri, 22 May 2026 19:37:03 -0700 Subject: [PATCH] Story generator fixes again --- docs/ideas.md | 18 ++++++++ nightshift/agents.py | 94 +++++++++++++++++++++++++++++++++++++++++- nightshift/pipeline.py | 35 ++++++++++++++++ tests/test_agents.py | 49 ++++++++++++++++++++++ tests/test_pipeline.py | 11 +++-- 5 files changed, 202 insertions(+), 5 deletions(-) diff --git a/docs/ideas.md b/docs/ideas.md index 15c916a..f770c67 100644 --- a/docs/ideas.md +++ b/docs/ideas.md @@ -111,6 +111,24 @@ Candidate classes: Route `local_edit` to the editor, `redraft` to the drafter, and `escalate` to a clear user-facing failure. Keep original draft and edited draft artifacts side by side for comparison. +## P1: Separate Expressive Drafting From Corrective Editing + +If writing quality feels lower after adding stricter correctness constraints, test a two-pass creative workflow: + +```text +free_draft -> corrective_edit -> review +``` + +The drafter would receive fewer mechanical constraints and focus on voice, scene energy, imagery, and character presence. A later editor pass would enforce: + +- output format +- canonical pronouns +- continuity details +- acceptance criteria +- minor cleanup + +This may recover stronger prose while keeping correctness guarantees. Treat it as an experiment, not a default, because weaker local models may use the looser draft prompt as permission to ignore task boundaries. + ## P1: Add A Writing-Mode Validator Add deterministic checks for prose workflows: diff --git a/nightshift/agents.py b/nightshift/agents.py index ea82a44..3f0dd8f 100644 --- a/nightshift/agents.py +++ b/nightshift/agents.py @@ -354,7 +354,9 @@ def build_prompt_bundle( task_context: str = "", retry_context: str | None = None, ) -> str: - acceptance = "\n".join(f"- {item}" for item in task.acceptance_criteria) + task_markdown = _task_markdown_for_stage(stage, task) + task_context = _task_context_for_stage(stage, task_context) + acceptance = "\n".join(f"- {item}" for item in _acceptance_for_stage(stage, task)) prior = "\n\n".join(f"## {stage_id}\n\n{content}" for stage_id, content in previous_outputs.items()) retries = "\n".join(f"- {note}" for note in retry_notes) @@ -373,7 +375,7 @@ def build_prompt_bundle( "", "## Task", "", - task.raw_markdown.strip(), + task_markdown.strip(), "", "## Acceptance Criteria", "", @@ -521,6 +523,94 @@ def output_contract_for(stage: StageConfig) -> str: return "Write the requested stage output in concise markdown." +def _task_markdown_for_stage(stage: StageConfig, task: Task) -> str: + if not _is_scene_drafting_stage(stage): + return task.raw_markdown + return _remove_update_bullets(_remove_task_section(task.raw_markdown, "Updates")) + + +def _acceptance_for_stage(stage: StageConfig, task: Task) -> tuple[str, ...]: + if not _is_scene_drafting_stage(stage): + return task.acceptance_criteria + filtered: list[str] = [] + skipping_update_paths = False + for item in task.acceptance_criteria: + normalized = item.strip() + lower = normalized.lower() + if lower == "updates:": + skipping_update_paths = True + continue + pathish = normalized.strip("`") + if skipping_update_paths and pathish.startswith("story/") and "chapters/" not in pathish: + continue + skipping_update_paths = False + filtered.append(item) + return tuple(filtered) + + +def _task_context_for_stage(stage: StageConfig, task_context: str) -> str: + if not _is_scene_drafting_stage(stage): + return task_context + return _remove_update_bullets_from_acceptance(task_context) + + +def _is_scene_drafting_stage(stage: StageConfig) -> bool: + allowed = {path.replace("\\", "/").rstrip("/") for path in stage.allowed_paths} + return stage.type == "file_writer" and "story/chapters" in allowed + + +def _remove_task_section(markdown: str, section_name: str) -> str: + lines = markdown.splitlines() + output: list[str] = [] + index = 0 + section_header = f"{section_name}:" + while index < len(lines): + line = lines[index] + if line.strip() == section_header: + index += 1 + while index < len(lines): + candidate = lines[index] + if re.match(r"^[A-Za-z][A-Za-z ]+:\s*$", candidate.strip()): + break + if re.match(r"^\s*---\s*$", candidate): + break + index += 1 + continue + output.append(line) + index += 1 + return "\n".join(output).strip() + "\n" + + +def _remove_update_bullets_from_acceptance(markdown: str) -> str: + return _remove_update_bullets(markdown, only_inside_acceptance=True) + + +def _remove_update_bullets(markdown: str, *, only_inside_acceptance: bool = False) -> str: + lines = markdown.splitlines() + output: list[str] = [] + in_acceptance = not only_inside_acceptance + skipping_update_paths = False + for line in lines: + stripped = line.strip() + if only_inside_acceptance and stripped in {"## Acceptance Criteria", "Acceptance Criteria:"}: + in_acceptance = True + output.append(line) + continue + if only_inside_acceptance and in_acceptance and line.startswith("## "): + in_acceptance = False + skipping_update_paths = False + if in_acceptance: + if stripped == "- Updates:": + skipping_update_paths = True + continue + pathish = stripped.removeprefix("- ").strip("`") + if skipping_update_paths and pathish.startswith("story/") and "chapters/" not in pathish: + continue + skipping_update_paths = False + output.append(line) + return "\n".join(output) + + def _file_writer_block_contract(stage: StageConfig) -> str: normalized = tuple(path.replace("\\", "/").rstrip("/") for path in stage.allowed_paths) if normalized == ("story/chapters",): diff --git a/nightshift/pipeline.py b/nightshift/pipeline.py index 9a3fc64..62bb3ed 100644 --- a/nightshift/pipeline.py +++ b/nightshift/pipeline.py @@ -767,6 +767,7 @@ class PipelineRunner: invalid_rerun_done = False candidate_index_path: Path | None = None while True: + updates: tuple[FileUpdate, ...] = () try: updates = parse_file_updates(stdout) candidate_index_path = self._write_file_writer_candidates( @@ -787,6 +788,28 @@ class PipelineRunner: break except PipelineError as exc: reason = _file_writer_error_reason(stage, str(exc)) + allowed_updates = _filter_allowed_file_updates(updates, stage) + if ( + allowed_updates + and len(allowed_updates) < len(updates) + and "not allowed for this stage" in str(exc) + ): + patch = generate_patch_from_file_updates( + allowed_updates, + self.config.project.root, + self.config.safety, + allowed_paths=stage.allowed_paths, + forbidden_paths=stage.forbidden_paths or DEFAULT_FORBIDDEN_PATHS, + ) + patch_reason = "Deterministic patch written from allowed file blocks; disallowed file blocks were ignored." + log_message = "Wrote deterministic patch from allowed file blocks" + self.logger.event( + "file_writer.disallowed_blocks_ignored", + "Ignored disallowed file blocks from file writer output", + stage_id=stage.id, + task_id=task.id, + ) + break if ( "no file blocks found" in str(exc) and "diff --git " not in stdout @@ -1814,6 +1837,18 @@ def _file_writer_stage_guidance(stage: StageConfig) -> str: return "" +def _filter_allowed_file_updates(updates: tuple[FileUpdate, ...], stage: StageConfig) -> tuple[FileUpdate, ...]: + if not updates or not stage.allowed_paths: + return () + allowed = tuple(path.replace("\\", "/").strip().strip("/") for path in stage.allowed_paths) + kept: list[FileUpdate] = [] + for update in updates: + path = update.path.replace("\\", "/").strip().strip("/") + if any(path == root or path.startswith(root.rstrip("/") + "/") for root in allowed): + kept.append(update) + return tuple(kept) + + def _file_writer_repair_format_note(stage: StageConfig) -> str: if _is_state_update_stage(stage): return ( diff --git a/tests/test_agents.py b/tests/test_agents.py index 04d7f4f..7b24024 100644 --- a/tests/test_agents.py +++ b/tests/test_agents.py @@ -98,6 +98,55 @@ class AgentExecutorTests(unittest.TestCase): self.assertIn("---END---", prompt) self.assertIn("Do not use markdown code fences", prompt) + def test_scene_file_writer_prompt_filters_state_updates_from_task_view(self) -> None: + task = parse_tasks( + """# Tasks + +- [ ] SCENE-001: Draft scene + +Description: +Write the opening scene. + +Acceptance Criteria: +- Writes: +- `story/chapters/chapter-001/scene-001.md` +- Updates: +- `story/plot-state.md` +- `story/unresolved-threads.md` +""" + )[0] + + prompt = build_prompt_bundle( + system_prompt="System rules", + stage=StageConfig( + id="draft_scene", + type="file_writer", + agent="drafter", + allowed_paths=("story/chapters",), + ), + project_context="Project context", + task_context="\n".join( + [ + "# Task Context", + "", + "## Acceptance Criteria", + "", + "- Writes:", + "- `story/chapters/chapter-001/scene-001.md`", + "- Updates:", + "- `story/plot-state.md`", + ] + ), + task=task, + previous_outputs={}, + retry_notes=[], + ) + + self.assertIn("story/chapters/chapter-001/scene-001.md", prompt) + self.assertNotIn("Updates:", prompt) + self.assertNotIn("story/plot-state.md", prompt) + self.assertNotIn("story/unresolved-threads.md", prompt) + def test_command_agent_writes_output_and_returns_pass(self) -> None: with tempfile.TemporaryDirectory() as directory: root = Path(directory) diff --git a/tests/test_pipeline.py b/tests/test_pipeline.py index f472bdf..c50f51a 100644 --- a/tests/test_pipeline.py +++ b/tests/test_pipeline.py @@ -772,7 +772,7 @@ Acceptance Criteria: self.assertTrue(candidate_index.exists()) self.assertIn("app.py", candidate_index.read_text(encoding="utf-8")) - def test_file_writer_preserves_candidates_when_stage_paths_reject_extra_files(self) -> None: + def test_file_writer_ignores_disallowed_blocks_when_allowed_candidate_exists(self) -> None: with tempfile.TemporaryDirectory() as directory: root = Path(directory) _write_common_files(root) @@ -811,9 +811,14 @@ Acceptance Criteria: task_dir = root / ".nightshift" / "runs" / "test-run" / "tasks" / "TASK-001" candidate = task_dir / "candidate-files" / "draft_scene" / "001-story_chapters_scene.md" - self.assertEqual(result.status, "failed") - self.assertIn("This is the drafting stage", result.reason) + rejected_candidate = task_dir / "candidate-files" / "draft_scene" / "002-story_plot-state.md" + patch = task_dir / "proposed.patch" + self.assertEqual(result.status, "complete") + self.assertTrue(patch.exists()) + self.assertIn("story/chapters/scene.md", patch.read_text(encoding="utf-8")) + self.assertNotIn("story/plot-state.md", patch.read_text(encoding="utf-8")) self.assertTrue(candidate.exists()) + self.assertTrue(rejected_candidate.exists()) self.assertEqual(candidate.read_text(encoding="utf-8"), "scene prose\n") def test_file_writer_accepts_unified_diff_fallback(self) -> None: