mirror of
https://github.com/khodges42/nightShift.git
synced 2026-06-14 18:18:36 +00:00
Story generator fixes again
This commit is contained in:
parent
c4d88fced5
commit
33b9de5441
|
|
@ -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.
|
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
|
## P1: Add A Writing-Mode Validator
|
||||||
|
|
||||||
Add deterministic checks for prose workflows:
|
Add deterministic checks for prose workflows:
|
||||||
|
|
|
||||||
|
|
@ -354,7 +354,9 @@ def build_prompt_bundle(
|
||||||
task_context: str = "",
|
task_context: str = "",
|
||||||
retry_context: str | None = None,
|
retry_context: str | None = None,
|
||||||
) -> str:
|
) -> 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())
|
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)
|
retries = "\n".join(f"- {note}" for note in retry_notes)
|
||||||
|
|
||||||
|
|
@ -373,7 +375,7 @@ def build_prompt_bundle(
|
||||||
"",
|
"",
|
||||||
"## Task",
|
"## Task",
|
||||||
"",
|
"",
|
||||||
task.raw_markdown.strip(),
|
task_markdown.strip(),
|
||||||
"",
|
"",
|
||||||
"## Acceptance Criteria",
|
"## Acceptance Criteria",
|
||||||
"",
|
"",
|
||||||
|
|
@ -521,6 +523,94 @@ def output_contract_for(stage: StageConfig) -> str:
|
||||||
return "Write the requested stage output in concise markdown."
|
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:
|
def _file_writer_block_contract(stage: StageConfig) -> str:
|
||||||
normalized = tuple(path.replace("\\", "/").rstrip("/") for path in stage.allowed_paths)
|
normalized = tuple(path.replace("\\", "/").rstrip("/") for path in stage.allowed_paths)
|
||||||
if normalized == ("story/chapters",):
|
if normalized == ("story/chapters",):
|
||||||
|
|
|
||||||
|
|
@ -767,6 +767,7 @@ class PipelineRunner:
|
||||||
invalid_rerun_done = False
|
invalid_rerun_done = False
|
||||||
candidate_index_path: Path | None = None
|
candidate_index_path: Path | None = None
|
||||||
while True:
|
while True:
|
||||||
|
updates: tuple[FileUpdate, ...] = ()
|
||||||
try:
|
try:
|
||||||
updates = parse_file_updates(stdout)
|
updates = parse_file_updates(stdout)
|
||||||
candidate_index_path = self._write_file_writer_candidates(
|
candidate_index_path = self._write_file_writer_candidates(
|
||||||
|
|
@ -787,6 +788,28 @@ class PipelineRunner:
|
||||||
break
|
break
|
||||||
except PipelineError as exc:
|
except PipelineError as exc:
|
||||||
reason = _file_writer_error_reason(stage, str(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 (
|
if (
|
||||||
"no file blocks found" in str(exc)
|
"no file blocks found" in str(exc)
|
||||||
and "diff --git " not in stdout
|
and "diff --git " not in stdout
|
||||||
|
|
@ -1814,6 +1837,18 @@ def _file_writer_stage_guidance(stage: StageConfig) -> str:
|
||||||
return ""
|
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:
|
def _file_writer_repair_format_note(stage: StageConfig) -> str:
|
||||||
if _is_state_update_stage(stage):
|
if _is_state_update_stage(stage):
|
||||||
return (
|
return (
|
||||||
|
|
|
||||||
|
|
@ -98,6 +98,55 @@ class AgentExecutorTests(unittest.TestCase):
|
||||||
self.assertIn("---END---", prompt)
|
self.assertIn("---END---", prompt)
|
||||||
self.assertIn("Do not use markdown code fences", 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:
|
def test_command_agent_writes_output_and_returns_pass(self) -> None:
|
||||||
with tempfile.TemporaryDirectory() as directory:
|
with tempfile.TemporaryDirectory() as directory:
|
||||||
root = Path(directory)
|
root = Path(directory)
|
||||||
|
|
|
||||||
|
|
@ -772,7 +772,7 @@ Acceptance Criteria:
|
||||||
self.assertTrue(candidate_index.exists())
|
self.assertTrue(candidate_index.exists())
|
||||||
self.assertIn("app.py", candidate_index.read_text(encoding="utf-8"))
|
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:
|
with tempfile.TemporaryDirectory() as directory:
|
||||||
root = Path(directory)
|
root = Path(directory)
|
||||||
_write_common_files(root)
|
_write_common_files(root)
|
||||||
|
|
@ -811,9 +811,14 @@ Acceptance Criteria:
|
||||||
|
|
||||||
task_dir = root / ".nightshift" / "runs" / "test-run" / "tasks" / "TASK-001"
|
task_dir = root / ".nightshift" / "runs" / "test-run" / "tasks" / "TASK-001"
|
||||||
candidate = task_dir / "candidate-files" / "draft_scene" / "001-story_chapters_scene.md"
|
candidate = task_dir / "candidate-files" / "draft_scene" / "001-story_chapters_scene.md"
|
||||||
self.assertEqual(result.status, "failed")
|
rejected_candidate = task_dir / "candidate-files" / "draft_scene" / "002-story_plot-state.md"
|
||||||
self.assertIn("This is the drafting stage", result.reason)
|
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(candidate.exists())
|
||||||
|
self.assertTrue(rejected_candidate.exists())
|
||||||
self.assertEqual(candidate.read_text(encoding="utf-8"), "scene prose\n")
|
self.assertEqual(candidate.read_text(encoding="utf-8"), "scene prose\n")
|
||||||
|
|
||||||
def test_file_writer_accepts_unified_diff_fallback(self) -> None:
|
def test_file_writer_accepts_unified_diff_fallback(self) -> None:
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue
Block a user