Recover blocked HTML structure edits
Authored by
mfwolffe <wolffemf@dukes.jmu.edu>
- SHA
e4b7f3b86f48f48c8dc4ba49c71a9ea95b5492f3- Parents
-
6925565 - Tree
4d94db3
e4b7f3b
e4b7f3b86f48f48c8dc4ba49c71a9ea95b5492f36925565
4d94db3| Status | File | + | - |
|---|---|---|---|
| M |
src/loader/runtime/tool_batches.py
|
51 | 0 |
| M |
tests/test_tool_batches.py
|
102 | 0 |
src/loader/runtime/tool_batches.pymodified@@ -370,6 +370,10 @@ class ToolBatchRunner: | |||
| 370 | tool_call, | 370 | tool_call, |
| 371 | outcome.event_content, | 371 | outcome.event_content, |
| 372 | ) | 372 | ) |
| 373 | + self._queue_blocked_html_structure_nudge( | ||
| 374 | + tool_call, | ||
| 375 | + outcome.event_content, | ||
| 376 | + ) | ||
| 373 | self._queue_blocked_html_content_quality_nudge( | 377 | self._queue_blocked_html_content_quality_nudge( |
| 374 | tool_call, | 378 | tool_call, |
| 375 | outcome.event_content, | 379 | outcome.event_content, |
@@ -1604,6 +1608,52 @@ class ToolBatchRunner: | |||
| 1604 | "mutation succeeds." | 1608 | "mutation succeeds." |
| 1605 | ) | 1609 | ) |
| 1606 | 1610 | ||
| 1611 | + def _queue_blocked_html_structure_nudge( | ||
| 1612 | + self, | ||
| 1613 | + tool_call: ToolCall, | ||
| 1614 | + event_content: str, | ||
| 1615 | + ) -> None: | ||
| 1616 | + """Recover blocked HTML structure edits without implying the file changed.""" | ||
| 1617 | + | ||
| 1618 | + if tool_call.name not in {"write", "edit", "patch"}: | ||
| 1619 | + return | ||
| 1620 | + if "HTML document structure would be invalid" not in event_content: | ||
| 1621 | + return | ||
| 1622 | + | ||
| 1623 | + target = str( | ||
| 1624 | + tool_call.arguments.get("file_path") | ||
| 1625 | + or tool_call.arguments.get("path") | ||
| 1626 | + or "" | ||
| 1627 | + ).strip() | ||
| 1628 | + if not target: | ||
| 1629 | + return | ||
| 1630 | + | ||
| 1631 | + anchor = html_quality_repair_insertion_anchor(target) | ||
| 1632 | + if anchor: | ||
| 1633 | + self.context.queue_steering_message( | ||
| 1634 | + f"The last HTML mutation for `{target}` was blocked before it " | ||
| 1635 | + "changed the file. Do not assume the on-disk file is malformed. " | ||
| 1636 | + "Stay on this same file and insert the new body content before " | ||
| 1637 | + "the existing closing document tail. Use one concrete `edit` or " | ||
| 1638 | + "`patch` that preserves exactly one closing tail; do not add a " | ||
| 1639 | + "second `</body>` or `</html>`. Exact current closing-tail anchor:\n" | ||
| 1640 | + f"```html\n{anchor}\n```\n" | ||
| 1641 | + "If using `edit`, make that anchor the `old_string`, and make " | ||
| 1642 | + "`new_string` the added substantive sections followed by the same " | ||
| 1643 | + "anchor. Do not reread or list unrelated files before retrying." | ||
| 1644 | + ) | ||
| 1645 | + return | ||
| 1646 | + | ||
| 1647 | + self.context.queue_steering_message( | ||
| 1648 | + f"The last HTML mutation for `{target}` was blocked before it changed " | ||
| 1649 | + "the file because the proposed result would have malformed document " | ||
| 1650 | + "structure. Retry the same target with one complete HTML document that " | ||
| 1651 | + "has exactly one `<html>`, one `<body>`, one `</body>`, one `</html>`, " | ||
| 1652 | + "and no content after `</html>`. Do not assume the blocked mutation " | ||
| 1653 | + "was applied, do not switch files, and do not claim completion until " | ||
| 1654 | + "the mutation succeeds." | ||
| 1655 | + ) | ||
| 1656 | + | ||
| 1607 | def _queue_blocked_invalid_mutation_nudge( | 1657 | def _queue_blocked_invalid_mutation_nudge( |
| 1608 | self, | 1658 | self, |
| 1609 | tool_call: ToolCall, | 1659 | tool_call: ToolCall, |
@@ -3527,6 +3577,7 @@ def _is_recoverable_guidance_block(event_content: str) -> bool: | |||
| 3527 | "[Blocked - HTML file creation falls outside the current declared artifact set]", | 3577 | "[Blocked - HTML file creation falls outside the current declared artifact set]", |
| 3528 | "[Blocked - HTML page introduces new local targets outside the current declared artifact set]", | 3578 | "[Blocked - HTML page introduces new local targets outside the current declared artifact set]", |
| 3529 | "[Blocked - HTML local asset references do not exist]", | 3579 | "[Blocked - HTML local asset references do not exist]", |
| 3580 | + "[Blocked - HTML document structure would be invalid]", | ||
| 3530 | "[Blocked - HTML content contains placeholder or stub text]", | 3581 | "[Blocked - HTML content contains placeholder or stub text]", |
| 3531 | "[Blocked - HTML guide chapter content is too thin]", | 3582 | "[Blocked - HTML guide chapter content is too thin]", |
| 3532 | "[Blocked - Edited HTML links point to files that do not exist]", | 3583 | "[Blocked - Edited HTML links point to files that do not exist]", |
tests/test_tool_batches.pymodified@@ -8280,6 +8280,108 @@ async def test_tool_batch_runner_blocked_html_quality_guidance_does_not_halt( | |||
| 8280 | assert "not a scaffold or outline" in queued[-1] | 8280 | assert "not a scaffold or outline" in queued[-1] |
| 8281 | 8281 | ||
| 8282 | 8282 | ||
| 8283 | +@pytest.mark.asyncio | ||
| 8284 | +async def test_tool_batch_runner_blocked_html_structure_guidance_does_not_halt( | ||
| 8285 | + temp_dir: Path, | ||
| 8286 | +) -> None: | ||
| 8287 | + async def assess_confidence( | ||
| 8288 | + tool_name: str, | ||
| 8289 | + tool_args: dict, | ||
| 8290 | + context: str, | ||
| 8291 | + ) -> ConfidenceAssessment: | ||
| 8292 | + raise AssertionError("Confidence scoring should not run in this scenario") | ||
| 8293 | + | ||
| 8294 | + async def verify_action( | ||
| 8295 | + tool_name: str, | ||
| 8296 | + tool_args: dict, | ||
| 8297 | + result: str, | ||
| 8298 | + expected: str = "", | ||
| 8299 | + ) -> ActionVerification: | ||
| 8300 | + raise AssertionError("Verification should not run in this scenario") | ||
| 8301 | + | ||
| 8302 | + target = temp_dir / "guide" / "chapters" / "08-monitoring.html" | ||
| 8303 | + target.parent.mkdir(parents=True) | ||
| 8304 | + target.write_text( | ||
| 8305 | + "\n".join( | ||
| 8306 | + [ | ||
| 8307 | + "<!DOCTYPE html>", | ||
| 8308 | + '<html lang="en">', | ||
| 8309 | + "<body>", | ||
| 8310 | + "<h1>Monitoring</h1>", | ||
| 8311 | + "<p>Existing content.</p>", | ||
| 8312 | + "</body>", | ||
| 8313 | + "</html>", | ||
| 8314 | + "", | ||
| 8315 | + ] | ||
| 8316 | + ) | ||
| 8317 | + ) | ||
| 8318 | + | ||
| 8319 | + context = build_context( | ||
| 8320 | + temp_dir=temp_dir, | ||
| 8321 | + messages=[], | ||
| 8322 | + safeguards=FakeSafeguards(), | ||
| 8323 | + assess_confidence=assess_confidence, | ||
| 8324 | + verify_action=verify_action, | ||
| 8325 | + ) | ||
| 8326 | + queued: list[str] = [] | ||
| 8327 | + context.queue_steering_message_callback = queued.append | ||
| 8328 | + runner = ToolBatchRunner(context, DefinitionOfDoneStore(temp_dir)) | ||
| 8329 | + dod = create_definition_of_done("Expand a guide chapter.") | ||
| 8330 | + | ||
| 8331 | + tool_calls = [ | ||
| 8332 | + ToolCall( | ||
| 8333 | + id=f"patch-structure-{index}", | ||
| 8334 | + name="patch", | ||
| 8335 | + arguments={"file_path": str(target), "patch": "@@ malformed"}, | ||
| 8336 | + ) | ||
| 8337 | + for index in range(3) | ||
| 8338 | + ] | ||
| 8339 | + blocked_message = ( | ||
| 8340 | + "[Blocked - HTML document structure would be invalid] Suggestion: " | ||
| 8341 | + "expected exactly one closing </html> tag (found 2); expected exactly " | ||
| 8342 | + "one closing </body> tag (found 2). Keep the existing closing document " | ||
| 8343 | + "tail intact." | ||
| 8344 | + ) | ||
| 8345 | + executor = FakeExecutor( | ||
| 8346 | + [ | ||
| 8347 | + tool_outcome( | ||
| 8348 | + tool_call=tool_call, | ||
| 8349 | + output=blocked_message, | ||
| 8350 | + is_error=True, | ||
| 8351 | + state=ToolExecutionState.BLOCKED, | ||
| 8352 | + ) | ||
| 8353 | + for tool_call in tool_calls | ||
| 8354 | + ] | ||
| 8355 | + ) | ||
| 8356 | + events: list[AgentEvent] = [] | ||
| 8357 | + | ||
| 8358 | + async def emit(event: AgentEvent) -> None: | ||
| 8359 | + events.append(event) | ||
| 8360 | + | ||
| 8361 | + result = await runner.execute_batch( | ||
| 8362 | + tool_calls=tool_calls, | ||
| 8363 | + tool_source="native", | ||
| 8364 | + pending_tool_calls_seen=set(), | ||
| 8365 | + emit=emit, | ||
| 8366 | + summary=TurnSummary(final_response=""), | ||
| 8367 | + dod=dod, | ||
| 8368 | + executor=executor, | ||
| 8369 | + on_confirmation=None, | ||
| 8370 | + on_user_question=None, | ||
| 8371 | + emit_confirmation=None, | ||
| 8372 | + consecutive_errors=0, | ||
| 8373 | + ) | ||
| 8374 | + | ||
| 8375 | + assert result.halted is False | ||
| 8376 | + assert result.consecutive_errors == 0 | ||
| 8377 | + assert queued | ||
| 8378 | + assert str(target) in queued[-1] | ||
| 8379 | + assert "blocked before it changed the file" in queued[-1] | ||
| 8380 | + assert "Do not assume the on-disk file is malformed" in queued[-1] | ||
| 8381 | + assert "```html\n</body>\n</html>\n```" in queued[-1] | ||
| 8382 | + assert "do not add a second `</body>` or `</html>`" in queued[-1] | ||
| 8383 | + | ||
| 8384 | + | ||
| 8283 | def test_tool_batch_runner_blocked_html_missing_target_after_outputs_exist_prefers_verify( | 8385 | def test_tool_batch_runner_blocked_html_missing_target_after_outputs_exist_prefers_verify( |
| 8284 | temp_dir: Path, | 8386 | temp_dir: Path, |
| 8285 | ) -> None: | 8387 | ) -> None: |