Require fuller multi-page guides
- SHA
3719f4431684609a1cc91e140601ea2f5bf5edb5- Parents
-
365993c - Tree
5759936
3719f44
3719f4431684609a1cc91e140601ea2f5bf5edb5365993c
5759936| Status | File | + | - |
|---|---|---|---|
| M |
src/loader/runtime/dod.py
|
104 | 12 |
| M |
tests/test_dod.py
|
76 | 0 |
| M |
tests/test_tool_batches.py
|
2 | 11 |
src/loader/runtime/dod.pymodified@@ -96,6 +96,7 @@ _MUTATING_FILE_CHANGE_HINTS = ( | ||
| 96 | 96 | "develop", |
| 97 | 97 | "developing", |
| 98 | 98 | ) |
| 99 | +_MINIMUM_SUBSTANTIVE_HTML_GUIDE_PAGES = 4 | |
| 99 | 100 | |
| 100 | 101 | |
| 101 | 102 | @dataclass |
@@ -629,11 +630,29 @@ def _derive_multi_page_html_quality_command( | ||
| 629 | 630 | project_root: Path, |
| 630 | 631 | task_statement: str, |
| 631 | 632 | ) -> str | None: |
| 632 | - html_paths = _multi_page_html_quality_paths(dod, project_root=project_root) | |
| 633 | - if len(html_paths) < 4: | |
| 634 | - return None | |
| 635 | 633 | if not _task_requires_substantive_html_guide_quality(task_statement): |
| 636 | 634 | return None |
| 635 | + html_paths = _multi_page_html_quality_paths(dod, project_root=project_root) | |
| 636 | + requires_multiple_pages = _requires_multiple_html_pages( | |
| 637 | + dod, | |
| 638 | + project_root=project_root, | |
| 639 | + ) | |
| 640 | + if requires_multiple_pages and len(html_paths) < _MINIMUM_SUBSTANTIVE_HTML_GUIDE_PAGES: | |
| 641 | + return "\n".join( | |
| 642 | + [ | |
| 643 | + "python3 - <<'PY'", | |
| 644 | + f"minimum_pages = {_MINIMUM_SUBSTANTIVE_HTML_GUIDE_PAGES}", | |
| 645 | + f"found_pages = {len(html_paths)}", | |
| 646 | + "print('HTML guide content quality issues:')", | |
| 647 | + "print(", | |
| 648 | + " f'insufficient HTML page count ({found_pages} files, expected at least {minimum_pages})'", | |
| 649 | + ")", | |
| 650 | + "raise SystemExit(1)", | |
| 651 | + "PY", | |
| 652 | + ] | |
| 653 | + ) | |
| 654 | + if len(html_paths) < _MINIMUM_SUBSTANTIVE_HTML_GUIDE_PAGES: | |
| 655 | + return None | |
| 637 | 656 | |
| 638 | 657 | path_literals = ", ".join(repr(str(path)) for path in html_paths) |
| 639 | 658 | return "\n".join( |
@@ -745,6 +764,11 @@ def all_planned_artifacts_exist( | ||
| 745 | 764 | for target, expect_directory in targets |
| 746 | 765 | ): |
| 747 | 766 | return False |
| 767 | + if _substantive_multi_page_html_guide_is_incomplete( | |
| 768 | + dod, | |
| 769 | + project_root=project_root, | |
| 770 | + ): | |
| 771 | + return False | |
| 748 | 772 | return not _planned_html_outputs_have_missing_local_links( |
| 749 | 773 | dod, |
| 750 | 774 | project_root=project_root, |
@@ -928,13 +952,34 @@ def _multi_page_html_quality_paths( | ||
| 928 | 952 | project_root=project_root, |
| 929 | 953 | max_paths=24, |
| 930 | 954 | ) |
| 931 | - planned_html = [ | |
| 932 | - target | |
| 933 | - for target, expect_directory in planned_targets | |
| 934 | - if not expect_directory and target.suffix.lower() in {".html", ".htm"} | |
| 935 | - ] | |
| 936 | - if planned_html: | |
| 937 | - return planned_html | |
| 955 | + paths: list[Path] = [] | |
| 956 | + seen: set[str] = set() | |
| 957 | + | |
| 958 | + for target, expect_directory in planned_targets: | |
| 959 | + if expect_directory: | |
| 960 | + if not target.exists(): | |
| 961 | + continue | |
| 962 | + try: | |
| 963 | + discovered = sorted(path for path in target.rglob("*.html") if path.is_file()) | |
| 964 | + except OSError: | |
| 965 | + continue | |
| 966 | + for path in discovered: | |
| 967 | + key = str(path) | |
| 968 | + if key in seen: | |
| 969 | + continue | |
| 970 | + seen.add(key) | |
| 971 | + paths.append(path) | |
| 972 | + continue | |
| 973 | + if target.suffix.lower() not in {".html", ".htm"} or not target.exists(): | |
| 974 | + continue | |
| 975 | + key = str(target) | |
| 976 | + if key in seen: | |
| 977 | + continue | |
| 978 | + seen.add(key) | |
| 979 | + paths.append(target) | |
| 980 | + | |
| 981 | + if paths: | |
| 982 | + return paths | |
| 938 | 983 | |
| 939 | 984 | touched_html = [] |
| 940 | 985 | for path_str in dod.touched_files: |
@@ -945,6 +990,52 @@ def _multi_page_html_quality_paths( | ||
| 945 | 990 | return list(dict.fromkeys(touched_html)) |
| 946 | 991 | |
| 947 | 992 | |
| 993 | +def _requires_multiple_html_pages( | |
| 994 | + dod: DefinitionOfDone, | |
| 995 | + *, | |
| 996 | + project_root: Path, | |
| 997 | +) -> bool: | |
| 998 | + planned_targets = collect_planned_artifact_targets( | |
| 999 | + dod, | |
| 1000 | + project_root=project_root, | |
| 1001 | + max_paths=24, | |
| 1002 | + ) | |
| 1003 | + if any( | |
| 1004 | + expect_directory | |
| 1005 | + and planned_directory_requires_generated_files( | |
| 1006 | + dod, | |
| 1007 | + target=target, | |
| 1008 | + project_root=project_root, | |
| 1009 | + ) | |
| 1010 | + for target, expect_directory in planned_targets | |
| 1011 | + ): | |
| 1012 | + return True | |
| 1013 | + | |
| 1014 | + planned_html = [ | |
| 1015 | + target | |
| 1016 | + for target, expect_directory in planned_targets | |
| 1017 | + if not expect_directory and target.suffix.lower() in {".html", ".htm"} | |
| 1018 | + ] | |
| 1019 | + if len(planned_html) > 1: | |
| 1020 | + return True | |
| 1021 | + | |
| 1022 | + lowered = dod.task_statement.lower() | |
| 1023 | + return "chapter" in lowered or "chapters" in lowered | |
| 1024 | + | |
| 1025 | + | |
| 1026 | +def _substantive_multi_page_html_guide_is_incomplete( | |
| 1027 | + dod: DefinitionOfDone, | |
| 1028 | + *, | |
| 1029 | + project_root: Path, | |
| 1030 | +) -> bool: | |
| 1031 | + if not _task_requires_substantive_html_guide_quality(dod.task_statement): | |
| 1032 | + return False | |
| 1033 | + if not _requires_multiple_html_pages(dod, project_root=project_root): | |
| 1034 | + return False | |
| 1035 | + html_paths = _multi_page_html_quality_paths(dod, project_root=project_root) | |
| 1036 | + return len(html_paths) < _MINIMUM_SUBSTANTIVE_HTML_GUIDE_PAGES | |
| 1037 | + | |
| 1038 | + | |
| 948 | 1039 | def _task_requires_substantive_html_guide_quality(task_statement: str) -> bool: |
| 949 | 1040 | lowered = task_statement.lower() |
| 950 | 1041 | if not any(token in lowered for token in ("guide", "tutorial", "documentation", "docs")): |
@@ -957,8 +1048,9 @@ def _task_requires_substantive_html_guide_quality(task_statement: str) -> bool: | ||
| 957 | 1048 | "detailed", |
| 958 | 1049 | "equally", |
| 959 | 1050 | "cadence", |
| 960 | - "chapter", | |
| 961 | - "chapters", | |
| 1051 | + "depth", | |
| 1052 | + "same structure", | |
| 1053 | + "same style", | |
| 962 | 1054 | ) |
| 963 | 1055 | ) |
| 964 | 1056 | |
tests/test_dod.pymodified@@ -261,6 +261,46 @@ def test_derive_verification_commands_adds_html_guide_quality_check_for_thorough | ||
| 261 | 261 | assert any("HTML guide content quality issues:" in command for command in commands) |
| 262 | 262 | |
| 263 | 263 | |
| 264 | +def test_derive_verification_commands_flags_insufficient_pages_for_broad_thorough_guide( | |
| 265 | + tmp_path: Path, | |
| 266 | +) -> None: | |
| 267 | + guide = tmp_path / "guide" | |
| 268 | + chapters = guide / "chapters" | |
| 269 | + chapters.mkdir(parents=True) | |
| 270 | + (guide / "index.html").write_text("<html></html>\n") | |
| 271 | + (chapters / "01-introduction.html").write_text("<h1>Intro</h1>\n") | |
| 272 | + | |
| 273 | + implementation_plan = tmp_path / "implementation.md" | |
| 274 | + implementation_plan.write_text( | |
| 275 | + "\n".join( | |
| 276 | + [ | |
| 277 | + "# Implementation Plan", | |
| 278 | + "", | |
| 279 | + "## File Changes", | |
| 280 | + f"- `{guide / 'index.html'}`", | |
| 281 | + f"- `{chapters}/` (directory for chapter files)", | |
| 282 | + "", | |
| 283 | + "## Execution Order", | |
| 284 | + "- Create chapter files with appropriate content", | |
| 285 | + ] | |
| 286 | + ) | |
| 287 | + ) | |
| 288 | + | |
| 289 | + dod = create_definition_of_done( | |
| 290 | + "Create an equally thorough multi-page HTML guide with chapter files." | |
| 291 | + ) | |
| 292 | + dod.implementation_plan = str(implementation_plan) | |
| 293 | + | |
| 294 | + commands = derive_verification_commands( | |
| 295 | + dod, | |
| 296 | + project_root=tmp_path, | |
| 297 | + task_statement=dod.task_statement, | |
| 298 | + supplement_existing=True, | |
| 299 | + ) | |
| 300 | + | |
| 301 | + assert any("insufficient HTML page count" in command for command in commands) | |
| 302 | + | |
| 303 | + | |
| 264 | 304 | def test_collect_planned_artifact_targets_ignores_prose_path_fragments_in_refreshed_plan( |
| 265 | 305 | tmp_path: Path, |
| 266 | 306 | ) -> None: |
@@ -390,6 +430,42 @@ def test_all_planned_artifacts_exist_requires_file_contents_for_planned_output_d | ||
| 390 | 430 | assert all_planned_artifacts_exist(dod, project_root=tmp_path) is False |
| 391 | 431 | |
| 392 | 432 | |
| 433 | +def test_all_planned_artifacts_exist_stays_false_for_substantive_guide_with_only_one_chapter( | |
| 434 | + tmp_path: Path, | |
| 435 | +) -> None: | |
| 436 | + implementation_plan = tmp_path / "implementation.md" | |
| 437 | + implementation_plan.write_text( | |
| 438 | + "\n".join( | |
| 439 | + [ | |
| 440 | + "# Implementation Plan", | |
| 441 | + "", | |
| 442 | + "## File Changes", | |
| 443 | + f"- `{tmp_path / 'guide' / 'index.html'}`", | |
| 444 | + f"- `{tmp_path / 'guide' / 'chapters'}/` (directory for chapter files)", | |
| 445 | + "", | |
| 446 | + "## Execution Order", | |
| 447 | + "- Create chapter files with appropriate content", | |
| 448 | + ] | |
| 449 | + ) | |
| 450 | + ) | |
| 451 | + | |
| 452 | + guide_root = tmp_path / "guide" | |
| 453 | + chapters = guide_root / "chapters" | |
| 454 | + chapters.mkdir(parents=True) | |
| 455 | + (guide_root / "index.html").write_text("<html></html>\n") | |
| 456 | + (chapters / "01-introduction.html").write_text("<h1>Intro</h1>\n") | |
| 457 | + | |
| 458 | + dod = create_definition_of_done("Create an equally thorough guide with chapters.") | |
| 459 | + dod.implementation_plan = str(implementation_plan) | |
| 460 | + dod.completed_items = ["Create chapter files with appropriate content"] | |
| 461 | + dod.touched_files = [ | |
| 462 | + str(guide_root / "index.html"), | |
| 463 | + str(chapters / "01-introduction.html"), | |
| 464 | + ] | |
| 465 | + | |
| 466 | + assert all_planned_artifacts_exist(dod, project_root=tmp_path) is False | |
| 467 | + | |
| 468 | + | |
| 393 | 469 | def test_all_planned_artifacts_exist_respects_nested_file_change_entries( |
| 394 | 470 | tmp_path: Path, |
| 395 | 471 | ) -> None: |
tests/test_tool_batches.pymodified@@ -3607,7 +3607,7 @@ async def test_tool_batch_runner_working_note_prefers_declared_output_gap_over_s | ||
| 3607 | 3607 | |
| 3608 | 3608 | |
| 3609 | 3609 | @pytest.mark.asyncio |
| 3610 | -async def test_tool_batch_runner_glob_handoff_stays_compact_before_first_output_write( | |
| 3610 | +async def test_tool_batch_runner_shallow_glob_does_not_handoff_before_content_read( | |
| 3611 | 3611 | temp_dir: Path, |
| 3612 | 3612 | ) -> None: |
| 3613 | 3613 | async def assess_confidence( |
@@ -3694,16 +3694,7 @@ async def test_tool_batch_runner_glob_handoff_stays_compact_before_first_output_ | ||
| 3694 | 3694 | consecutive_errors=0, |
| 3695 | 3695 | ) |
| 3696 | 3696 | |
| 3697 | - assert queued_messages | |
| 3698 | - message = queued_messages[-1] | |
| 3699 | - assert "Confirmed progress:" in message | |
| 3700 | - assert "Next step: create `index.html`." in message | |
| 3701 | - assert ( | |
| 3702 | - f"Prefer one `write` call for `{temp_dir / 'Loader' / 'guides' / 'nginx' / 'index.html'}` now." | |
| 3703 | - in message | |
| 3704 | - ) | |
| 3705 | - assert "One declared output artifact is still missing." not in message | |
| 3706 | - assert "Do not reread reference material or spend the next turn on bookkeeping." in message | |
| 3697 | + assert queued_messages == [] | |
| 3707 | 3698 | |
| 3708 | 3699 | |
| 3709 | 3700 | @pytest.mark.asyncio |