Honor rooted HTML outputs
Authored by
mfwolffe <wolffemf@dukes.jmu.edu>
- SHA
1f4baadaf74b5a12e17ca0ff234a90cf599e235e- Parents
-
48742fd - Tree
26af0c8
1f4baad
1f4baadaf74b5a12e17ca0ff234a90cf599e235e48742fd
26af0c8| Status | File | + | - |
|---|---|---|---|
| M |
src/loader/runtime/dod.py
|
41 | 2 |
| M |
tests/test_dod.py
|
36 | 2 |
| M |
tests/test_tool_batches.py
|
125 | 0 |
src/loader/runtime/dod.pymodified@@ -788,6 +788,12 @@ def all_planned_artifact_outputs_exist( | ||
| 788 | 788 | for target, expect_directory in targets |
| 789 | 789 | ): |
| 790 | 790 | return False |
| 791 | + if _planned_html_outputs_declare_missing_files( | |
| 792 | + dod, | |
| 793 | + project_root=project_root, | |
| 794 | + targets=targets, | |
| 795 | + ): | |
| 796 | + return False | |
| 791 | 797 | if _substantive_multi_page_html_guide_is_incomplete( |
| 792 | 798 | dod, |
| 793 | 799 | project_root=project_root, |
@@ -869,8 +875,11 @@ def collect_missing_declared_html_output_files( | ||
| 869 | 875 | """Return missing HTML outputs already declared within the current artifact graph.""" |
| 870 | 876 | |
| 871 | 877 | normalized_target = target.resolve(strict=False) |
| 878 | + scope_target = normalized_target | |
| 879 | + if normalized_target.suffix.lower() in {".html", ".htm"}: | |
| 880 | + scope_target = normalized_target.parent | |
| 872 | 881 | artifact_root = _resolve_declared_html_artifact_root( |
| 873 | - normalized_target, | |
| 882 | + scope_target, | |
| 874 | 883 | project_root=project_root.resolve(strict=False), |
| 875 | 884 | ) |
| 876 | 885 | if artifact_root is None: |
@@ -894,7 +903,7 @@ def collect_missing_declared_html_output_files( | ||
| 894 | 903 | continue |
| 895 | 904 | try: |
| 896 | 905 | resolved_target.relative_to(artifact_root) |
| 897 | - resolved_target.relative_to(normalized_target) | |
| 906 | + resolved_target.relative_to(scope_target) | |
| 898 | 907 | except ValueError: |
| 899 | 908 | continue |
| 900 | 909 | key = str(resolved_target) |
@@ -905,6 +914,36 @@ def collect_missing_declared_html_output_files( | ||
| 905 | 914 | return tuple(missing_targets) |
| 906 | 915 | |
| 907 | 916 | |
| 917 | +def _planned_html_outputs_declare_missing_files( | |
| 918 | + dod: DefinitionOfDone, | |
| 919 | + *, | |
| 920 | + project_root: Path, | |
| 921 | + targets: list[tuple[Path, bool]], | |
| 922 | +) -> bool: | |
| 923 | + if not _requires_multiple_html_pages(dod, project_root=project_root): | |
| 924 | + return False | |
| 925 | + | |
| 926 | + seen_scopes: set[str] = set() | |
| 927 | + for target, expect_directory in targets: | |
| 928 | + if expect_directory: | |
| 929 | + scope_target = target | |
| 930 | + elif target.suffix.lower() in {".html", ".htm"}: | |
| 931 | + scope_target = target | |
| 932 | + else: | |
| 933 | + continue | |
| 934 | + | |
| 935 | + scope_key = str(scope_target.resolve(strict=False)) | |
| 936 | + if scope_key in seen_scopes: | |
| 937 | + continue | |
| 938 | + seen_scopes.add(scope_key) | |
| 939 | + if collect_missing_declared_html_output_files( | |
| 940 | + target=scope_target, | |
| 941 | + project_root=project_root, | |
| 942 | + ): | |
| 943 | + return True | |
| 944 | + return False | |
| 945 | + | |
| 946 | + | |
| 908 | 947 | def _infer_next_observed_output_file( |
| 909 | 948 | *, |
| 910 | 949 | target: Path, |
tests/test_dod.pymodified@@ -535,7 +535,7 @@ def test_all_planned_artifacts_exist_respects_nested_file_change_entries( | ||
| 535 | 535 | assert all_planned_artifacts_exist(dod, project_root=tmp_path) is True |
| 536 | 536 | |
| 537 | 537 | |
| 538 | -def test_all_planned_artifacts_exist_stays_false_while_touched_html_links_missing( | |
| 538 | +def test_all_planned_artifact_outputs_stay_false_while_root_declares_missing_html_outputs( | |
| 539 | 539 | tmp_path: Path, |
| 540 | 540 | ) -> None: |
| 541 | 541 | implementation_plan = tmp_path / "implementation.md" |
@@ -571,11 +571,45 @@ def test_all_planned_artifacts_exist_stays_false_while_touched_html_links_missin | ||
| 571 | 571 | dod.completed_items = ["Create chapter files with appropriate content"] |
| 572 | 572 | |
| 573 | 573 | assert all_planned_artifacts_exist(dod, project_root=tmp_path) is False |
| 574 | - assert all_planned_artifact_outputs_exist(dod, project_root=tmp_path) is True | |
| 574 | + assert all_planned_artifact_outputs_exist(dod, project_root=tmp_path) is False | |
| 575 | 575 | |
| 576 | 576 | (chapters / "02-setup.html").write_text("<h1>Setup</h1>\n") |
| 577 | 577 | |
| 578 | 578 | assert all_planned_artifacts_exist(dod, project_root=tmp_path) is True |
| 579 | + assert all_planned_artifact_outputs_exist(dod, project_root=tmp_path) is True | |
| 580 | + | |
| 581 | + | |
| 582 | +def test_collect_missing_declared_html_outputs_accepts_root_html_file_target( | |
| 583 | + tmp_path: Path, | |
| 584 | +) -> None: | |
| 585 | + implementation_plan = tmp_path / "implementation.md" | |
| 586 | + implementation_plan.write_text( | |
| 587 | + "\n".join( | |
| 588 | + [ | |
| 589 | + "# Implementation Plan", | |
| 590 | + "", | |
| 591 | + "## File Changes", | |
| 592 | + f"- `{tmp_path / 'guide' / 'index.html'}`", | |
| 593 | + f"- `{tmp_path / 'guide' / 'chapters'}/` (directory for chapter files)", | |
| 594 | + ] | |
| 595 | + ) | |
| 596 | + ) | |
| 597 | + | |
| 598 | + guide_root = tmp_path / "guide" | |
| 599 | + chapters = guide_root / "chapters" | |
| 600 | + chapters.mkdir(parents=True) | |
| 601 | + index = guide_root / "index.html" | |
| 602 | + index.write_text( | |
| 603 | + '<a href="chapters/01-introduction.html">Intro</a>\n' | |
| 604 | + '<a href="chapters/02-setup.html">Setup</a>\n' | |
| 605 | + ) | |
| 606 | + (chapters / "01-introduction.html").write_text("<h1>Intro</h1>\n") | |
| 607 | + | |
| 608 | + dod = create_definition_of_done("Create a multi-file guide with chapters.") | |
| 609 | + dod.implementation_plan = str(implementation_plan) | |
| 610 | + dod.touched_files = [str(index), str(chapters / "01-introduction.html")] | |
| 611 | + | |
| 612 | + assert all_planned_artifact_outputs_exist(dod, project_root=tmp_path) is False | |
| 579 | 613 | |
| 580 | 614 | |
| 581 | 615 | def test_build_verification_summary_keeps_concrete_missing_link_details() -> None: |
tests/test_tool_batches.pymodified@@ -701,6 +701,131 @@ async def test_tool_batch_runner_queues_duplicate_observation_nudge( | ||
| 701 | 701 | assert ephemeral_messages == [] |
| 702 | 702 | |
| 703 | 703 | |
| 704 | +@pytest.mark.asyncio | |
| 705 | +async def test_tool_batch_runner_duplicate_read_keeps_root_declared_missing_html_output_active( | |
| 706 | + temp_dir: Path, | |
| 707 | +) -> None: | |
| 708 | + async def assess_confidence( | |
| 709 | + tool_name: str, | |
| 710 | + tool_args: dict, | |
| 711 | + context: str, | |
| 712 | + ) -> ConfidenceAssessment: | |
| 713 | + raise AssertionError("Confidence scoring should not run for this scenario") | |
| 714 | + | |
| 715 | + async def verify_action( | |
| 716 | + tool_name: str, | |
| 717 | + tool_args: dict, | |
| 718 | + result: str, | |
| 719 | + expected: str = "", | |
| 720 | + ) -> ActionVerification: | |
| 721 | + raise AssertionError("Verification should not run for this scenario") | |
| 722 | + | |
| 723 | + guide_root = temp_dir / "guide" | |
| 724 | + chapters = guide_root / "chapters" | |
| 725 | + chapters.mkdir(parents=True) | |
| 726 | + index = guide_root / "index.html" | |
| 727 | + chapter_one = chapters / "01-introduction.html" | |
| 728 | + index.write_text( | |
| 729 | + '<a href="chapters/01-introduction.html">Intro</a>\n' | |
| 730 | + '<a href="chapters/02-installation.html">Install</a>\n' | |
| 731 | + ) | |
| 732 | + chapter_one.write_text("<h1>Intro</h1>\n") | |
| 733 | + | |
| 734 | + implementation_plan = temp_dir / "implementation.md" | |
| 735 | + implementation_plan.write_text( | |
| 736 | + "\n".join( | |
| 737 | + [ | |
| 738 | + "# Implementation Plan", | |
| 739 | + "", | |
| 740 | + "## File Changes", | |
| 741 | + f"- `{index}`", | |
| 742 | + f"- `{chapters}/` (directory for chapter files)", | |
| 743 | + ] | |
| 744 | + ) | |
| 745 | + ) | |
| 746 | + | |
| 747 | + messages = [ | |
| 748 | + Message( | |
| 749 | + role=Role.ASSISTANT, | |
| 750 | + content="I should keep building the guide.", | |
| 751 | + tool_calls=[ | |
| 752 | + ToolCall( | |
| 753 | + id="read-index", | |
| 754 | + name="read", | |
| 755 | + arguments={"file_path": str(index)}, | |
| 756 | + ) | |
| 757 | + ], | |
| 758 | + ), | |
| 759 | + ] | |
| 760 | + context = build_context( | |
| 761 | + temp_dir=temp_dir, | |
| 762 | + messages=messages, | |
| 763 | + safeguards=FakeSafeguards(), | |
| 764 | + assess_confidence=assess_confidence, | |
| 765 | + verify_action=verify_action, | |
| 766 | + auto_recover=False, | |
| 767 | + ) | |
| 768 | + context.session.current_task = f"Build the guide rooted at {index}." | |
| 769 | + persistent_messages: list[str] = [] | |
| 770 | + ephemeral_messages: list[str] = [] | |
| 771 | + context.queue_steering_message_callback = persistent_messages.append | |
| 772 | + context.queue_ephemeral_steering_message_callback = ephemeral_messages.append | |
| 773 | + runner = ToolBatchRunner(context, DefinitionOfDoneStore(temp_dir)) | |
| 774 | + tool_call = ToolCall( | |
| 775 | + id="read-dup-rooted", | |
| 776 | + name="read", | |
| 777 | + arguments={"file_path": str(index)}, | |
| 778 | + ) | |
| 779 | + duplicate_message = ( | |
| 780 | + "[Skipped - duplicate action: Already read " | |
| 781 | + f"{index} recently without any intervening changes; " | |
| 782 | + "reuse the earlier read result instead of rereading]" | |
| 783 | + ) | |
| 784 | + executor = FakeExecutor( | |
| 785 | + [ | |
| 786 | + ToolExecutionOutcome( | |
| 787 | + tool_call=tool_call, | |
| 788 | + state=ToolExecutionState.DUPLICATE, | |
| 789 | + message=Message.tool_result_message( | |
| 790 | + tool_call_id=tool_call.id, | |
| 791 | + display_content=duplicate_message, | |
| 792 | + result_content=duplicate_message, | |
| 793 | + ), | |
| 794 | + event_content=duplicate_message, | |
| 795 | + is_error=False, | |
| 796 | + result_output=duplicate_message, | |
| 797 | + ) | |
| 798 | + ] | |
| 799 | + ) | |
| 800 | + | |
| 801 | + summary = TurnSummary(final_response="") | |
| 802 | + dod = create_definition_of_done("Create a multi-file HTML guide with chapters.") | |
| 803 | + dod.implementation_plan = str(implementation_plan) | |
| 804 | + dod.touched_files = [str(index), str(chapter_one)] | |
| 805 | + dod.completed_items = ["Create chapter files with appropriate content"] | |
| 806 | + dod.pending_items.append("Create the remaining chapter files") | |
| 807 | + | |
| 808 | + await runner.execute_batch( | |
| 809 | + tool_calls=[tool_call], | |
| 810 | + tool_source="assistant", | |
| 811 | + pending_tool_calls_seen=set(), | |
| 812 | + emit=_noop_emit, | |
| 813 | + summary=summary, | |
| 814 | + dod=dod, | |
| 815 | + executor=executor, # type: ignore[arg-type] | |
| 816 | + on_confirmation=None, | |
| 817 | + on_user_question=None, | |
| 818 | + emit_confirmation=None, | |
| 819 | + consecutive_errors=0, | |
| 820 | + ) | |
| 821 | + | |
| 822 | + assert len(persistent_messages) == 1 | |
| 823 | + assert "Create the remaining chapter files" in persistent_messages[0] | |
| 824 | + assert "Resume by creating `02-installation.html` now." in persistent_messages[0] | |
| 825 | + assert "All explicitly planned artifacts already exist on disk." not in persistent_messages[0] | |
| 826 | + assert ephemeral_messages == [] | |
| 827 | + | |
| 828 | + | |
| 704 | 829 | @pytest.mark.asyncio |
| 705 | 830 | async def test_tool_batch_runner_todo_write_does_not_regress_completed_file_todo( |
| 706 | 831 | temp_dir: Path, |