`6eb402b`

Recover blocked mutation paths

Authored by mfwolffe <wolffemf@dukes.jmu.edu> 2 weeks ago

SHA: 6eb402b466d0b9fb0f17abe309280aa051d1b933
Parents: 1fafa49
Tree: e675a78

6 changed files

Status	File	+	-
M	`src/loader/runtime/recovery.py`	66	2
M	`src/loader/runtime/repair.py`	65	4
M	`src/loader/runtime/tool_batch_recovery.py`	34	0
M	`src/loader/runtime/tool_batches.py`	160	0
M	`tests/test_repair.py`	65	0
M	`tests/test_tool_batches.py`	107	0

src/loader/runtime/recovery.pymodified

      args: dict[str, Any] | None,
      error: str,
  ) -> dict[str, Any] | None:
 -    """Detect metadata-only mutation calls missing their real text payload."""
 +    """Detect invalid mutation calls missing their real payload or target path."""
      arguments = dict(args or {})
      error_lower = error.lower()
              "missing required",
              "empty content",
              "validation warning",
 +            "empty file path",
 +            "valid file path",
 +            "missing file path",
+         ]
      ):
          return None
      file_path = str(arguments.get("file_path") or arguments.get("path") or "").strip()
 +    missing_target = tool_name in {"write", "edit", "patch"} and (
 +        "empty file path" in error_lower
 +        or "valid file path" in error_lower
 +        or "missing file path" in error_lower
 +        or (
 +            any(
 +                token in error_lower
 +                for token in [
 +                    "required positional argument",
 +                    "missing 1 required",
 +                    "missing required",
 +                ]
 +            )
 +            and "file_path" in error_lower
 +        )
 +    )
++
 +    if missing_target:
 +        return {
 +            "kind": "missing_target",
 +            "required_fields": ["file_path"],
 +            "invalid_fields": [],
 +            "file_path": file_path,
 +        }
      if tool_name == "write":
          invalid_fields = [
+         ]
          if "content" not in arguments and invalid_fields:
              return {
 +                "kind": "missing_payload",
                  "required_fields": ["content"],
                  "invalid_fields": invalid_fields,
                  "file_path": file_path,
+         ]
          if missing_fields and invalid_fields:
              return {
 +                "kind": "missing_payload",
                  "required_fields": missing_fields,
                  "invalid_fields": invalid_fields,
                  "file_path": file_path,
          invalid_fields = [field for field in ("hunk_count",) if field in arguments]
          if "patch" not in arguments and "hunks" not in arguments and invalid_fields:
              return {
 +                "kind": "missing_payload",
                  "required_fields": ["patch or hunks"],
                  "invalid_fields": invalid_fields,
                  "file_path": file_path,
          required = ", ".join(payload_fix["required_fields"])
          invalid = ", ".join(payload_fix["invalid_fields"])
          target = payload_fix["file_path"]
 -        if tool_name == "write":
 +        if payload_fix.get("kind") == "missing_target":
 +            if tool_name == "write":
 +                category_hints = [
 +                    (
 +                        f"Resend the mutation as `write(file_path=..., content='...')` "
 +                        f"for `{target}` with a real file path"
 +                        if target
 +                        else "Resend the mutation as `write(file_path=..., content='...')` with a real file path"
 +                    ),
 +                    "Do not leave `file_path` empty or pointed at an unknown target",
 +                    "Do not reread reference files first unless one specific fact still blocks the write target",
 +                ]
 +            elif tool_name == "edit":
 +                category_hints = [
 +                    (
 +                        f"Resend the mutation as `edit(file_path=..., old_string='...', new_string='...')` "
 +                        f"for `{target}` with a real file path"
 +                        if target
 +                        else "Resend the mutation as `edit(file_path=..., old_string='...', new_string='...')` with a real file path"
 +                    ),
 +                    "Do not leave `file_path` empty or pointed at an unknown target",
 +                    "Do not reread reference files first unless one specific exact replacement span is still unknown",
 +                ]
 +            elif tool_name == "patch":
 +                category_hints = [
 +                    (
 +                        f"Resend the mutation as `patch(file_path=..., patch='...')` "
 +                        f"for `{target}` with a real file path"
 +                        if target
 +                        else "Resend the mutation as `patch(file_path=..., patch='...')` with a real file path"
 +                    ),
 +                    "Do not leave `file_path` empty or pointed at an unknown target",
 +                    "Do not reread reference files first unless one specific edit span is still unknown",
 +                ]
 +        elif tool_name == "write":
              category_hints = [
+                 (
                      f"Resend the mutation as `write(file_path=..., content='...')` "

src/loader/runtime/repair.pymodified

          if dod is not None and self._should_compact_empty_retry_message(dod):
              compact_lines: list[str] = []
              compact_lines.extend(self._planned_artifact_progress_lines(dod)[:2])
 -            compact_lines.extend(self._payload_retry_lines())
 +            compact_lines.extend(self._payload_retry_lines(dod))
              compact_lines.extend(
                  self._next_step_resume_lines(
                      dod,
              planned_lines = self._planned_artifact_progress_lines(dod)
              progress_lines.extend(planned_lines)
 -            progress_lines.extend(self._payload_retry_lines())
 +            progress_lines.extend(self._payload_retry_lines(dod))
              progress_lines.extend(
                  self._next_step_resume_lines(
                      dod,
+             ]
+         )
 -    def _payload_retry_lines(self) -> list[str]:
 +    def _payload_retry_lines(self, dod: DefinitionOfDone | None) -> list[str]:
          recovery_context = self.context.recovery_context
          if recovery_context is None or not recovery_context.attempts:
              return []
          if fix is None:
              return []
 -        target = fix["file_path"]
 +        target = fix["file_path"] or self._preferred_retry_target(dod)
          invalid = ", ".join(f"`{field}`" for field in fix["invalid_fields"])
 +        if fix.get("kind") == "missing_target":
 +            if attempt.tool_name == "write":
 +                target_line = (
 +                    f"Last tool failure: resend `write` for `{target}` with a valid `file_path` and real `content`."
 +                    if target
 +                    else "Last tool failure: resend `write` with a valid `file_path` and real `content`."
 +                )
 +                return [
 +                    target_line,
 +                    "Do not leave `file_path` empty; point it at the concrete next output file.",
 +                ]
 +            if attempt.tool_name == "edit":
 +                target_line = (
 +                    f"Last tool failure: resend `edit` for `{target}` with a valid `file_path` plus real `old_string`/`new_string`."
 +                    if target
 +                    else "Last tool failure: resend `edit` with a valid `file_path` plus real `old_string`/`new_string`."
 +                )
 +                return [
 +                    target_line,
 +                    "Do not leave `file_path` empty; point it at the concrete file you already know needs the edit.",
 +                ]
 +            if attempt.tool_name == "patch":
 +                target_line = (
 +                    f"Last tool failure: resend `patch` for `{target}` with a valid `file_path` and real patch text or `hunks`."
 +                    if target
 +                    else "Last tool failure: resend `patch` with a valid `file_path` and real patch text or `hunks`."
 +                )
 +                return [
 +                    target_line,
 +                    "Do not leave `file_path` empty; point it at the concrete file you already know needs the patch.",
 +                ]
          if attempt.tool_name == "write":
              target_line = (
                  f"Last tool failure: resend `write` for `{target}` with real `content`, not just summary fields."
                  return normalized_target, False
          return first_missing
 +    def _preferred_retry_target(self, dod: DefinitionOfDone | None) -> str:
 +        if dod is None:
 +            return ""
++
 +        missing_artifact = self._preferred_resume_missing_artifact(dod)
 +        next_pending = self._preferred_resume_pending_item(
 +            dod,
 +            missing_artifact=missing_artifact,
 +        )
 +        if next_pending:
 +            pending_target = self._infer_pending_item_output_target(dod, next_pending)
 +            if pending_target is not None and not pending_target.exists():
 +                return str(pending_target)
++
 +        if missing_artifact is None:
 +            return ""
++
 +        target, expect_directory = missing_artifact
 +        if not expect_directory:
 +            return str(target)
++
 +        next_output_file, _ = infer_next_output_file(
 +            target=target,
 +            project_root=self.context.project_root,
 +            messages=list(getattr(self.context.session, "messages", []) or []),
 +        )
 +        if next_output_file is not None:
 +            return str(next_output_file)
 +        return str(target)
++
      def _concretize_directory_missing_artifact(
          self,
          dod: DefinitionOfDone,

src/loader/runtime/tool_batch_recovery.pymodified

          target = fix["file_path"]
          invalid_fields = ", ".join(f"`{field}`" for field in fix["invalid_fields"])
          required_fields = "`, `".join(fix["required_fields"])
 +        if fix.get("kind") == "missing_target":
 +            if tool_call.name == "write":
 +                target_line = (
 +                    f"- The failed call for `{target}` omitted a valid `file_path`."
 +                    if target
 +                    else "- The failed call omitted a valid `file_path`."
 +                )
 +                return [
 +                    target_line,
 +                    "- Resend one concrete `write(file_path=..., content='...')` call now instead of rereading more files.",
 +                ]
++
 +            if tool_call.name == "edit":
 +                target_line = (
 +                    f"- The failed call for `{target}` omitted a valid `file_path`."
 +                    if target
 +                    else "- The failed call omitted a valid `file_path`."
 +                )
 +                return [
 +                    target_line,
 +                    "- Resend one concrete `edit(file_path=..., old_string='...', new_string='...')` call now instead of rereading more files.",
 +                ]
++
 +            if tool_call.name == "patch":
 +                target_line = (
 +                    f"- The failed call for `{target}` omitted a valid `file_path`."
 +                    if target
 +                    else "- The failed call omitted a valid `file_path`."
 +                )
 +                return [
 +                    target_line,
 +                    "- Resend one concrete `patch(file_path=..., patch='...')` or `patch(..., hunks=[...])` call now instead of rereading more files.",
 +                ]
++
          if tool_call.name == "write":
              target_line = (
                  f"- The failed call for `{target}` omitted the required `content` payload."

src/loader/runtime/tool_batches.pymodified

  from .executor import ToolExecutionState, ToolExecutor
  from .logging import get_runtime_logger
  from .policy_timeline import append_verification_timeline_entry
 +from .recovery import RecoveryContext, detect_missing_mutation_payload
  from .repair_focus import extract_active_repair_context
  from .safeguard_services import extract_shell_text_rewrite_target
  from .tool_batch_checks import ToolBatchConfidenceGate, ToolBatchVerificationGate
              if outcome.state == ToolExecutionState.DUPLICATE:
                  self._queue_duplicate_observation_nudge(tool_call, dod=dod)
              elif outcome.state == ToolExecutionState.BLOCKED:
 +                self._queue_blocked_invalid_mutation_nudge(
 +                    tool_call,
 +                    outcome.event_content,
 +                    dod=dod,
 +                )
                  self._queue_blocked_active_repair_nudge(outcome.event_content)
                  self._queue_blocked_active_repair_mutation_nudge(outcome.event_content)
                  self._queue_blocked_completed_artifact_scope_nudge(
              "Do not reopen unrelated reference materials while this concrete repair target is unresolved."
+         )
 +    def _queue_blocked_invalid_mutation_nudge(
 +        self,
 +        tool_call: ToolCall,
 +        event_content: str,
 +        *,
 +        dod: DefinitionOfDone,
 +    ) -> None:
 +        """Recover blocked mutations that omitted a real target path or text payload."""
++
 +        fix = detect_missing_mutation_payload(
 +            tool_call.name,
 +            tool_call.arguments,
 +            event_content,
 +        )
 +        if fix is None:
 +            return
++
 +        self._record_blocked_invalid_mutation_attempt(tool_call, event_content)
++
 +        messages = list(getattr(self.context.session, "messages", []) or [])
 +        missing_artifact = _next_missing_planned_artifact(
 +            dod,
 +            project_root=self.context.project_root,
 +            messages=messages,
 +        )
 +        next_pending = preferred_pending_todo_item(
 +            dod,
 +            project_root=self.context.project_root,
 +            missing_artifact=missing_artifact,
 +        )
 +        missing_artifact = _prefer_missing_artifact_for_pending_item(
 +            dod,
 +            missing_artifact=missing_artifact,
 +            next_pending=next_pending,
 +            project_root=self.context.project_root,
 +        )
 +        resume_target = _preferred_resume_target_path(
 +            dod,
 +            next_pending=next_pending,
 +            missing_artifact=missing_artifact,
 +            project_root=self.context.project_root,
 +            messages=messages,
 +        )
 +        resume_suffix = _pending_item_resume_suffix(
 +            dod,
 +            next_pending=next_pending,
 +            missing_artifact=missing_artifact,
 +            project_root=self.context.project_root,
 +            messages=messages,
 +        )
 +        target_label = f"`{resume_target.name or str(resume_target)}`" if resume_target else ""
++
 +        if fix.get("kind") == "missing_target":
 +            prefix = f"That `{tool_call.name}` call did not provide a valid `file_path`."
 +            if target_label:
 +                prefix += f" Stay on {target_label}."
 +            self.context.queue_steering_message(
 +                prefix
 +                + resume_suffix
 +                + " Resend one concrete "
 +                + _invalid_mutation_call_shape(tool_call.name)
 +                + " now instead of another working note, reread, or empty response."
 +            )
 +            return
++
 +        invalid_fields = ", ".join(f"`{field}`" for field in fix["invalid_fields"])
 +        prefix = f"That `{tool_call.name}` call omitted the real text payload."
 +        if invalid_fields:
 +            prefix += f" {invalid_fields} are summary fields, not valid mutation inputs."
 +        if target_label:
 +            prefix += f" Stay on {target_label}."
 +        self.context.queue_steering_message(
 +            prefix
 +            + resume_suffix
 +            + " Resend one concrete "
 +            + _invalid_mutation_call_shape(tool_call.name)
 +            + " now instead of rereading more files."
 +        )
++
 +    def _record_blocked_invalid_mutation_attempt(
 +        self,
 +        tool_call: ToolCall,
 +        error: str,
 +    ) -> None:
 +        """Seed recovery state from blocked malformed mutations for later retry guidance."""
++
 +        recovery_context = self.context.recovery_context
 +        if recovery_context is None or not recovery_context.is_related_failure(
 +            tool_call.name,
 +            tool_call.arguments,
 +            error,
 +        ):
 +            recovery_context = RecoveryContext(
 +                original_tool=tool_call.name,
 +                original_args=tool_call.arguments,
 +                max_retries=self.context.config.max_recovery_attempts,
 +            )
 +            self.context.recovery_context = recovery_context
++
 +        if not recovery_context.is_similar_attempt(
 +            tool_call.name,
 +            tool_call.arguments,
 +        ):
 +            recovery_context.add_attempt(
 +                tool_call.name,
 +                tool_call.arguments,
 +                error,
 +            )
++
      async def _record_successful_execution(
          self,
          *,
+     )
 +def _preferred_resume_target_path(
 +    dod: DefinitionOfDone,
 +    *,
 +    next_pending: str | None,
 +    missing_artifact: tuple[Path, bool] | None,
 +    project_root: Path,
 +    messages: list[Any] | None = None,
 +) -> Path | None:
 +    if next_pending:
 +        pending_target = infer_pending_todo_output_target(
 +            dod,
 +            next_pending,
 +            project_root=project_root,
 +        )
 +        if pending_target is not None and not pending_target.exists():
 +            return pending_target.expanduser().resolve(strict=False)
++
 +    if missing_artifact is None:
 +        return None
++
 +    target, expect_directory = missing_artifact
 +    normalized_target = target.expanduser().resolve(strict=False)
 +    if not expect_directory:
 +        return normalized_target
++
 +    next_output_file, _ = infer_next_output_file(
 +        target=normalized_target,
 +        project_root=project_root,
 +        messages=list(messages or []),
 +    )
 +    if next_output_file is not None:
 +        return next_output_file.expanduser().resolve(strict=False)
 +    return normalized_target
++
++
 +def _invalid_mutation_call_shape(tool_name: str) -> str:
 +    if tool_name == "write":
 +        return "`write(file_path=..., content=...)`"
 +    if tool_name == "edit":
 +        return "`edit(file_path=..., old_string=..., new_string=...)`"
 +    if tool_name == "patch":
 +        return "`patch(file_path=..., patch='...')` or `patch(..., hunks=[...])`"
 +    return f"`{tool_name}(...)`"
++
++
  def _resume_suffix_for_target(
      target: Path,
      *,

tests/test_repair.pymodified

+     )
 +def test_empty_response_retry_recovers_blocked_empty_file_path_to_concrete_target(
 +    temp_dir: Path,
 +) -> None:
 +    context = build_context(
 +        temp_dir=temp_dir,
 +        use_react=False,
 +    )
 +    repairer = ResponseRepairer(context)
++
 +    guide_root = temp_dir / "guides" / "nginx"
 +    chapters = guide_root / "chapters"
 +    chapters.mkdir(parents=True)
 +    index_path = guide_root / "index.html"
 +    first_chapter = chapters / "01-introduction.html"
 +    second_chapter = chapters / "02-installation.html"
 +    index_path.write_text("<html></html>\n")
 +    first_chapter.write_text("<h1>Intro</h1>\n")
++
 +    implementation_plan = temp_dir / "implementation.md"
 +    implementation_plan.write_text(
 +        "\n".join(
 +            [
 +                "# Implementation Plan",
 +                "",
 +                "## File Changes",
 +                f"- `{index_path}`",
 +                f"- `{first_chapter}`",
 +                f"- `{second_chapter}`",
 +                "",
 +            ]
 +        )
 +    )
++
 +    dod = create_definition_of_done("Create a multi-file nginx guide.")
 +    dod.implementation_plan = str(implementation_plan)
 +    dod.touched_files.extend([str(index_path), str(first_chapter)])
 +    dod.pending_items.append("Creating Chapter 2: Installation and Setup")
++
 +    context.recovery_context = RecoveryContext(
 +        original_tool="write",
 +        original_args={"file_path": "", "content": "<html></html>\n"},
 +    )
 +    context.recovery_context.add_attempt(
 +        "write",
 +        {"file_path": "", "content": "<html></html>\n"},
 +        "Empty file path",
 +    )
++
 +    decision = repairer.handle_empty_response(
 +        task="Create a multi-file nginx guide.",
 +        original_task=None,
 +        empty_retry_count=1,
 +        max_empty_retries=2,
 +        dod=dod,
 +    )
++
 +    assert decision.should_continue is True
 +    assert decision.retry_message is not None
 +    assert (
 +        f"Last tool failure: resend `write` for `{second_chapter}` with a valid `file_path` and real `content`."
 +        in decision.retry_message
 +    )
 +    assert "Do not leave `file_path` empty" in decision.retry_message
++
++
  def test_empty_response_retry_respects_discovery_first_pending_step(
      temp_dir: Path,
  ) -> None:

tests/test_tool_batches.pymodified

      assert "All explicitly planned artifacts already exist." in queued[0]
      assert "Verify all guide files are linked and complete" in queued[0]
      assert "Do not reopen earlier reference materials." in queued[0]
++
++
 +@pytest.mark.asyncio
 +async def test_tool_batch_runner_blocked_empty_file_path_nudges_concrete_next_artifact(
 +    temp_dir: Path,
 +) -> None:
 +    async def assess_confidence(
 +        tool_name: str,
 +        tool_args: dict,
 +        context: str,
 +    ) -> ConfidenceAssessment:
 +        raise AssertionError("Confidence scoring should be disabled in this scenario")
++
 +    async def verify_action(
 +        tool_name: str,
 +        tool_args: dict,
 +        result: str,
 +        expected: str = "",
 +    ) -> ActionVerification:
 +        raise AssertionError("Verification should not run in this scenario")
++
 +    guide_root = temp_dir / "guides" / "nginx"
 +    chapters = guide_root / "chapters"
 +    chapters.mkdir(parents=True)
 +    index_path = guide_root / "index.html"
 +    chapter_one = chapters / "01-introduction.html"
 +    chapter_two = chapters / "02-installation.html"
 +    index_path.write_text("<html></html>\n")
 +    chapter_one.write_text("<h1>Intro</h1>\n")
++
 +    implementation_plan = temp_dir / "implementation.md"
 +    implementation_plan.write_text(
 +        "\n".join(
 +            [
 +                "# Implementation Plan",
 +                "",
 +                "## File Changes",
 +                f"- `{index_path}`",
 +                f"- `{chapter_one}`",
 +                f"- `{chapter_two}`",
 +                "",
 +            ]
 +        )
 +    )
++
 +    context = build_context(
 +        temp_dir=temp_dir,
 +        messages=[],
 +        safeguards=FakeSafeguards(),
 +        assess_confidence=assess_confidence,
 +        verify_action=verify_action,
 +        auto_recover=False,
 +    )
 +    queued: list[str] = []
 +    context.queue_steering_message_callback = queued.append
 +    runner = ToolBatchRunner(context, DefinitionOfDoneStore(temp_dir))
 +    tool_call = ToolCall(
 +        id="write-2",
 +        name="write",
 +        arguments={"file_path": "", "content": "<html></html>\n"},
 +    )
 +    blocked_message = "[Blocked - Empty file path] Suggestion: Provide a valid file path"
 +    executor = FakeExecutor(
 +        [
 +            ToolExecutionOutcome(
 +                tool_call=tool_call,
 +                state=ToolExecutionState.BLOCKED,
 +                message=Message.tool_result_message(
 +                    tool_call_id=tool_call.id,
 +                    display_content=blocked_message,
 +                    result_content=blocked_message,
 +                    is_error=True,
 +                ),
 +                event_content=blocked_message,
 +                is_error=True,
 +                result_output=blocked_message,
 +            )
 +        ]
 +    )
 +    dod = create_definition_of_done("Create a multi-file nginx guide.")
 +    dod.implementation_plan = str(implementation_plan)
 +    dod.touched_files.extend([str(index_path), str(chapter_one)])
 +    dod.pending_items.append("Creating Chapter 2: Installation and Setup")
++
 +    await runner.execute_batch(
 +        tool_calls=[tool_call],
 +        tool_source="assistant",
 +        pending_tool_calls_seen=set(),
 +        emit=_noop_emit,
 +        summary=TurnSummary(final_response=""),
 +        dod=dod,
 +        executor=executor,  # type: ignore[arg-type]
 +        on_confirmation=None,
 +        on_user_question=None,
 +        emit_confirmation=None,
 +        consecutive_errors=0,
 +    )
++
 +    assert queued
 +    assert "did not provide a valid `file_path`" in queued[0]
 +    assert "Resume by creating `02-installation.html` now." in queued[0]
 +    assert (
 +        f"Prefer one `write` call for `{chapter_two}` instead of more rereads."
 +        in queued[0]
 +    )
 +    assert context.recovery_context is not None
 +    assert context.recovery_context.attempts[-1].error == blocked_message