Strengthen declared target recovery
Authored by
mfwolffe <wolffemf@dukes.jmu.edu>
- SHA
62a6597e6f5e0cf8105d2a3331031b0e18644cb5- Parents
-
4bffedd - Tree
98e456c
62a6597
62a6597e6f5e0cf8105d2a3331031b0e18644cb54bffedd
98e456c| Status | File | + | - |
|---|---|---|---|
| M |
src/loader/runtime/safeguard_services.py
|
17 | 10 |
| M |
src/loader/runtime/tool_batches.py
|
27 | 3 |
| M |
tests/test_safeguard_services.py
|
33 | 0 |
| M |
tests/test_tool_batches.py
|
55 | 0 |
src/loader/runtime/safeguard_services.pymodified@@ -939,14 +939,25 @@ class PreActionValidator: | ||
| 939 | 939 | if current_relative in declared_targets: |
| 940 | 940 | return ValidationResult(valid=True) |
| 941 | 941 | |
| 942 | + declared_suggestions = self._suggest_declared_html_targets( | |
| 943 | + declared_targets, | |
| 944 | + [current_relative], | |
| 945 | + ) | |
| 942 | 946 | declared_preview = ", ".join(sorted(declared_targets)[:3]) |
| 943 | 947 | if authoritative_root_graph: |
| 944 | - root_index = (root / "index.html").resolve(strict=False) | |
| 945 | - suggestion = ( | |
| 946 | - "Keep new non-root HTML files within the root-declared artifact set and " | |
| 947 | - f"update the guide root `{root_index}` before creating undeclared sibling pages, " | |
| 948 | - f"for example: {current_relative}" | |
| 949 | - ) | |
| 948 | + if declared_suggestions: | |
| 949 | + suggestion = ( | |
| 950 | + "Keep new non-root HTML files within the root-declared artifact set. " | |
| 951 | + f"Do not create undeclared sibling page `{current_relative}`; " | |
| 952 | + "use the closest declared local target instead" | |
| 953 | + ) | |
| 954 | + else: | |
| 955 | + root_index = (root / "index.html").resolve(strict=False) | |
| 956 | + suggestion = ( | |
| 957 | + "Keep new non-root HTML files within the root-declared artifact set and " | |
| 958 | + f"update the guide root `{root_index}` before creating undeclared sibling pages, " | |
| 959 | + f"for example: {current_relative}" | |
| 960 | + ) | |
| 950 | 961 | else: |
| 951 | 962 | suggestion = ( |
| 952 | 963 | "Keep new non-root HTML files within the current declared artifact set and " |
@@ -954,10 +965,6 @@ class PreActionValidator: | ||
| 954 | 965 | ) |
| 955 | 966 | if declared_preview: |
| 956 | 967 | suggestion += f". Already-declared local targets include: {declared_preview}" |
| 957 | - declared_suggestions = self._suggest_declared_html_targets( | |
| 958 | - declared_targets, | |
| 959 | - [current_relative], | |
| 960 | - ) | |
| 961 | 968 | if declared_suggestions: |
| 962 | 969 | suggestion += ( |
| 963 | 970 | ". Closest declared local targets include: " |
src/loader/runtime/tool_batches.pymodified@@ -1093,6 +1093,14 @@ class ToolBatchRunner: | ||
| 1093 | 1093 | relative_target = target_path.relative_to(root_index.parent).as_posix() |
| 1094 | 1094 | except ValueError: |
| 1095 | 1095 | relative_target = target_path.name |
| 1096 | + closest_targets = _extract_blocked_html_target_list( | |
| 1097 | + event_content, | |
| 1098 | + "Closest declared local targets include:", | |
| 1099 | + ) | |
| 1100 | + declared_targets = _extract_blocked_html_target_list( | |
| 1101 | + event_content, | |
| 1102 | + "Already-declared local targets include:", | |
| 1103 | + ) | |
| 1096 | 1104 | |
| 1097 | 1105 | if all_planned_artifact_outputs_exist(dod, project_root=self.context.project_root): |
| 1098 | 1106 | verification_commands = dod.verification_commands or derive_verification_commands( |
@@ -1116,10 +1124,26 @@ class ToolBatchRunner: | ||
| 1116 | 1124 | |
| 1117 | 1125 | guidance = ( |
| 1118 | 1126 | "That new HTML file is outside the current root-declared artifact set. " |
| 1119 | - f"Before creating `{relative_target}`, update `{root_index}` so the guide root " | |
| 1120 | - "explicitly links to that page, then retry the file creation. " | |
| 1121 | - "Stay on the active guide files; do not reopen the earlier reference guide first." | |
| 1127 | + f"Do not create `{relative_target}`." | |
| 1122 | 1128 | ) |
| 1129 | + if closest_targets: | |
| 1130 | + guidance += ( | |
| 1131 | + " Stay within the declared set and continue with the closest declared target instead: " | |
| 1132 | + + ", ".join(f"`{candidate}`" for candidate in closest_targets[:3]) | |
| 1133 | + + "." | |
| 1134 | + ) | |
| 1135 | + else: | |
| 1136 | + guidance += ( | |
| 1137 | + f" Before creating `{relative_target}`, update `{root_index}` so the guide root " | |
| 1138 | + "explicitly links to that page, then retry the file creation." | |
| 1139 | + ) | |
| 1140 | + if declared_targets: | |
| 1141 | + guidance += ( | |
| 1142 | + " Already-declared local targets include: " | |
| 1143 | + + ", ".join(f"`{candidate}`" for candidate in declared_targets[:3]) | |
| 1144 | + + "." | |
| 1145 | + ) | |
| 1146 | + guidance += " Stay on the active guide files; do not reopen the earlier reference guide first." | |
| 1123 | 1147 | self.context.queue_steering_message(guidance) |
| 1124 | 1148 | |
| 1125 | 1149 | def _queue_blocked_html_missing_target_nudge( |
tests/test_safeguard_services.pymodified@@ -691,6 +691,39 @@ def test_pre_action_validator_allows_declared_missing_chapter_file_creation( | ||
| 691 | 691 | assert result.valid is True |
| 692 | 692 | |
| 693 | 693 | |
| 694 | +def test_pre_action_validator_blocks_undeclared_file_creation_with_closest_declared_target( | |
| 695 | + tmp_path: Path, | |
| 696 | +) -> None: | |
| 697 | + validator = PreActionValidator() | |
| 698 | + guide = tmp_path / "guide" | |
| 699 | + chapters = guide / "chapters" | |
| 700 | + chapters.mkdir(parents=True) | |
| 701 | + (guide / "index.html").write_text( | |
| 702 | + "\n".join( | |
| 703 | + [ | |
| 704 | + '<a href="chapters/01-introduction.html">Introduction</a>', | |
| 705 | + '<a href="chapters/02-installation.html">Installation</a>', | |
| 706 | + '<a href="chapters/03-configuration.html">Configuration</a>', | |
| 707 | + "", | |
| 708 | + ] | |
| 709 | + ) | |
| 710 | + ) | |
| 711 | + (chapters / "01-introduction.html").write_text("<html></html>\n") | |
| 712 | + | |
| 713 | + result = validator.validate( | |
| 714 | + "write", | |
| 715 | + { | |
| 716 | + "file_path": str(chapters / "02-basics.html"), | |
| 717 | + "content": "<html></html>\n", | |
| 718 | + }, | |
| 719 | + ) | |
| 720 | + | |
| 721 | + assert result.valid is False | |
| 722 | + assert result.reason == "HTML file creation falls outside the current declared artifact set" | |
| 723 | + assert "Do not create undeclared sibling page `chapters/02-basics.html`" in result.suggestion | |
| 724 | + assert "Closest declared local targets include: chapters/02-installation.html" in result.suggestion | |
| 725 | + | |
| 726 | + | |
| 694 | 727 | def test_pre_action_validator_blocks_chapter_write_with_undeclared_missing_sibling( |
| 695 | 728 | tmp_path: Path, |
| 696 | 729 | ) -> None: |
tests/test_tool_batches.pymodified@@ -7378,6 +7378,61 @@ def test_tool_batch_runner_blocked_html_declared_file_creation_after_outputs_exi | ||
| 7378 | 7378 | assert "update the guide root" not in queued[0] |
| 7379 | 7379 | |
| 7380 | 7380 | |
| 7381 | +def test_tool_batch_runner_blocked_html_declared_file_creation_prefers_closest_target( | |
| 7382 | + temp_dir: Path, | |
| 7383 | +) -> None: | |
| 7384 | + async def assess_confidence( | |
| 7385 | + tool_name: str, | |
| 7386 | + tool_args: dict, | |
| 7387 | + context: str, | |
| 7388 | + ) -> ConfidenceAssessment: | |
| 7389 | + raise AssertionError("Confidence scoring should not run in this scenario") | |
| 7390 | + | |
| 7391 | + async def verify_action( | |
| 7392 | + tool_name: str, | |
| 7393 | + tool_args: dict, | |
| 7394 | + result: str, | |
| 7395 | + expected: str = "", | |
| 7396 | + ) -> ActionVerification: | |
| 7397 | + raise AssertionError("Verification should not run in this scenario") | |
| 7398 | + | |
| 7399 | + context = build_context( | |
| 7400 | + temp_dir=temp_dir, | |
| 7401 | + messages=[], | |
| 7402 | + safeguards=FakeSafeguards(), | |
| 7403 | + assess_confidence=assess_confidence, | |
| 7404 | + verify_action=verify_action, | |
| 7405 | + ) | |
| 7406 | + queued: list[str] = [] | |
| 7407 | + context.queue_steering_message_callback = queued.append | |
| 7408 | + runner = ToolBatchRunner(context, DefinitionOfDoneStore(temp_dir)) | |
| 7409 | + dod = create_definition_of_done("Create a guide.") | |
| 7410 | + | |
| 7411 | + target = temp_dir / "guide" / "chapters" / "02-basics.html" | |
| 7412 | + runner._queue_blocked_html_declared_file_creation_nudge( | |
| 7413 | + ToolCall( | |
| 7414 | + id="write-basics", | |
| 7415 | + name="write", | |
| 7416 | + arguments={"file_path": str(target)}, | |
| 7417 | + ), | |
| 7418 | + ( | |
| 7419 | + "[Blocked - HTML file creation falls outside the current declared artifact set] " | |
| 7420 | + "Suggestion: Keep new non-root HTML files within the root-declared artifact set. " | |
| 7421 | + "Do not create undeclared sibling page `chapters/02-basics.html`; use the closest declared local target instead. " | |
| 7422 | + "Already-declared local targets include: chapters/01-introduction.html, " | |
| 7423 | + "chapters/02-installation.html, chapters/03-basic-configuration.html. " | |
| 7424 | + "Closest declared local targets include: chapters/02-installation.html" | |
| 7425 | + ), | |
| 7426 | + dod=dod, | |
| 7427 | + ) | |
| 7428 | + | |
| 7429 | + assert queued | |
| 7430 | + assert "Do not create `chapters/02-basics.html`." in queued[0] | |
| 7431 | + assert "closest declared target instead: `chapters/02-installation.html`" in queued[0] | |
| 7432 | + assert "Already-declared local targets include:" in queued[0] | |
| 7433 | + assert "update the guide root" not in queued[0] | |
| 7434 | + | |
| 7435 | + | |
| 7381 | 7436 | def test_tool_batch_runner_blocked_html_missing_target_after_outputs_exist_prefers_verify( |
| 7382 | 7437 | temp_dir: Path, |
| 7383 | 7438 | ) -> None: |