Agentic fix: cwd-aware paths, verification controls, lint cleanup, and targeted tests

[beknobloch]

Summary

Make agentic fix path handling more robust: resolve llm_model.csv and error logs relative to the provided cwd, not process CWD.
Improve verification behavior in run_agentic_fix, including honoring PDD_AGENTIC_VERIFY=auto.
Fix stack-trace logging and clean up low-risk lint issues in agentic_fix.py.
Add/extend tests around cwd/path handling and LLM CSV discovery; temporarily skip the Google real-call test.

Test Results

• Unit tests: PASS
• Regression tests: PASS
• Sync regression: PASS
• PyLint result: 8.76 / 10

Manual Testing

N/A

Checklist

• All tests pass
• No temp files committed
• No merge conflicts

[Copilot]

Pull request overview

This PR improves the robustness of agentic_fix by making path resolution cwd-aware, adjusting verification/logging behavior, cleaning lint issues, and adding targeted tests for path discovery.

Changes:

• Resolve llm_model.csv and relative error-log paths against the provided cwd (not process CWD), with new tests.
• Add new agent-output “harvest/apply” parsing utilities (file blocks + optional TESTCMD) and extra diagnostics.
• Update real-call tests to skip the Google provider case when the expected key isn’t available.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 9 comments.

File	Description
tests/test_agentic_fix.py	Adds tests for cwd-aware error-log resolution and csv discovery; adjusts Google real-call gating.
pdd/agentic_fix.py	Implements cwd-aware csv/log paths, adds agent output harvesting utilities, changes provider detection and success/verification flow.

Comments suppressed due to low confidence (1)

pdd/agentic_fix.py:1

• The public function signature removed the protect_tests kwarg. If downstream callers (or CLI flags) still pass this, it will now raise TypeError. If this option is intentionally deprecated, consider keeping it as an ignored kwarg (or with the old default) for backward compatibility, or provide a clear deprecation path before removing it.

from __future__ import annotations

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[gltanaka]

Hey @beknobloch — thanks so much for putting this together! The bug identification here is sharp, and the diagnostic work around Issue #186 is genuinely appreciated. I'd love to get the core fixes landed, but I have some concerns about the overall scope before we merge. Here's the breakdown:

✅ Real bugs you found (great catches!)

1. find_llm_csv_path() uses process CWD instead of working_dir

Line 59 uses Path.cwd() (the process's current working directory), which means if run_agentic_fix() is called with an explicit cwd that differs from the shell's working directory, the project-level .pdd/llm_model.csv will never be found. Your base_dir parameter fix is exactly right.

2. Error log path not resolved against working_dir

Line 268 constructs Path(error_log_file) without resolving it against working_dir. When error_log_file is a relative path and the process CWD differs from the project CWD, the wrong log (or no log) gets read. The fix to resolve it against working_dir is correct.

These two bugs are worth fixing and the targeted tests you wrote for them are excellent.

⚠️ Architectural conflict with the January refactor

The main concern is that a significant portion of this PR resurrects code that was intentionally removed in commit 0ed88c72 ("refactor: Align agentic_fix.py with agentic_crash.py patterns", Jan 20). That refactor explicitly:

• Removed ~480 lines of provider-specific runners (_run_openai_variants, _run_anthropic_variants, _run_google_variants) in favor of the unified run_agentic_task() from agentic_common.py
• Removed the harvest fallback flow (_try_harvest_then_verify)
• Removed the hardcoded _AGENT_COST_PER_CALL = 0.02 constant (cost now comes from actual token usage)

This PR adds all of that back, which would grow agentic_fix.py from ~451 lines to ~1,200 lines and undo that architectural simplification. I'd want to understand if that's intentional before merging — if there's a reason those pieces need to come back, let's discuss it!

ℹ️ Other notes

• PID type validation in agentic_sync_runner.py: subprocess.Popen() always sets .pid to a valid int, so the defensive checks aren't strictly needed. Not harmful, just unnecessary complexity.
• _detect_suspicious_files(): This is a great diagnostic tool and I'm glad you re-added it — it was present in 4395951d and then dropped in the refactor. Happy to take this as a standalone addition if you want to split it out.
• Existing CWD tests: TestCwdHandling in tests/test_agentic_fix.py already has two tests covering cwd handling — worth checking your new tests don't overlap.

🙏 Suggested path forward

The cleanest approach would be a targeted PR that includes just:

1. find_llm_csv_path(base_dir=None) fix + call-site update
2. Error log path resolution against working_dir
3. Your two new targeted tests
4. Optionally: _detect_suspicious_files() re-addition

That gets the real fixes landed quickly without the architectural questions. If the provider-specific runners and harvest flow genuinely need to come back, that's worth a separate conversation and PR.

Really appreciate the bug sleuthing here — these are subtle path issues that matter!