fix(sync): make outer consecutive-fix breaker progress-sensitive (refs #1203)

[prompt-driven-github]

Summary

Complements the already-merged inner-loop fix for #1203 by closing a gap one layer up.

#1230 (merged) made the inner fix_error_loop progress-sensitive via an
assertion/logic stagnation streak. But a single outer fix operation runs that
whole inner loop (fix_main(..., loop=True, max_attempts=5)). When a fix
operation makes partial progress yet exhausts its inner max_attempts without
fully converging, the sync orchestrator re-selects fix, and the outer
consecutive-fix breaker in sync_orchestration.py is still progress-blind — it
trips at 5 consecutive operations even when the failing-test count is strictly
decreasing across them. This is the exact symptom #1203 reported
("Detected 5 consecutive fix operations").

Refs #1203. Does not close it — the parent is tracked by #1300 (combining
#1230 + #1231). This is a complementary hardening of the outer breaker only.

Changes

• pdd/sync_orchestration.py: the consecutive-fix breaker now fires only when
  consecutive_fixes >= MAX_CONSECUTIVE_FIXES (5, now a named constant matching
  the sibling breakers) and the failing-test count did not strictly decrease
  versus the prior fix operation. Failure count is read via
  read_run_report(basename, language, paths=pdd_files) so the correct
  subproject .pdd/meta is honored (pdd fix from a parent dir writes orphan files to run cwd instead of the subproject #1211).
• tests/test_issue_1203_breaker.py: subproject-meta path-resolution regression
  plus parametrized progress/stagnation cases.

Scope notes

• No prompt-template changes — that's Update test-generation prompt to use pattern-based pytest.raises(match=) instead of exact strings #1231/feat: use pattern-based pytest.raises(match=) in test-generation prompts (#1231) #1235, already merged.
• No hard caps — the existing budget guard backstops the outer loop.

[gltanaka]

Do not merge as-is. The parent issue is real, and a progress-sensitive outer sync fix may still be useful, but this PR needs changes before merge.

Required changes:

1. Pass the resolved path hints into the new progress check. The new pdd/sync_orchestration.py call uses read_run_report(basename, language) without paths=pdd_files, while the surrounding sync code now consistently passes paths=pdd_files for issue #1211. In a parent-CWD/subproject setup, the pathless call reads the wrong .pdd/meta and returns None, so the new failure history records 0 instead of the actual failing-test count. That can make the progress-sensitive breaker behave incorrectly exactly in the subproject workflow we recently fixed. Required fix: use read_run_report(basename, language, paths=pdd_files) and add a regression where the run report is under a subproject .pdd/meta while sync is invoked from the parent directory.

2. Reconcile this PR with the narrower open PRs for the split work (#1233 and #1235). This PR overlaps #1235's prompt guidance, but with a broader absolute "NEVER exact-string" rule and without the same context/docs updates or carve-outs for contractual/structured message text. Either remove the prompt-template changes from this PR and let #1235 carry that work, or rebase and align the wording/docs so we do not merge duplicate or conflicting prompt guidance.

3. Document or remove the new hard caps. The code adds a hard 30-operation limit and a hard 15-minute wall-clock limit, but sync_orchestration_python.prompt only documents the consecutive-fix breaker change. If these caps are intentional, they need to be part of the prompt contract and should be justified/tested as user-visible sync behavior; otherwise they should be left out of this fix.

I would not merge #1245 until these are addressed.

[sohni-tagirisa]

Reframed and rescoped this per your review. It's no longer trying to resolve #1203 — #1230 (inner loop) and #1231 (#1235, prompts) already cover the parent via #1300. This PR now does one thing: hardens the outer consecutive-fix breaker in sync_orchestration.py, which #1230 doesn't reach.

The gap: a single outer fix operation runs the whole inner fix_error_loop (fix_main(..., loop=True, max_attempts=5)). When a fix operation makes partial progress but exhausts its inner max_attempts without fully converging, the orchestrator re-selects fix, and the outer breaker trips at 5 consecutive operations even while the failing-test count is strictly decreasing across them — the exact symptom #1203 reported. This is the "progress-sensitive outer sync fix may still be useful" case you noted.

Addressing your three points:

1. Path hints — the new failure-count read uses read_run_report(basename, language, paths=pdd_files), matching the rest of sync (pdd fix from a parent dir writes orphan files to run cwd instead of the subproject #1211). Added tests/test_issue_1203_breaker.py::test_run_report_resolves_subproject_meta_from_parent, which puts the run report under a subproject .pdd/meta, invokes from the parent dir, and asserts the pathful read finds it while a pathless read does not.

2. Prompt-template overlap with chore: PDD sync for #1230 #1233/feat: use pattern-based pytest.raises(match=) in test-generation prompts (#1231) #1235 — removed entirely. This PR touches no prompts.

3. Hard caps — removed. The existing budget guard backstops the loop; the only behavioral change is making the outer consecutive-fix breaker progress-sensitive.

Changed the closing keyword to Refs #1203 so this doesn't auto-close the parent ahead of #1300. Happy to close this instead if you'd rather fold the outer-breaker change into the #1300 combined branch — your call on where it lands.

[gltanaka]

Still do not merge as-is. The substantive direction is now much cleaner, and the earlier blockers are mostly addressed: the path-aware read_run_report(..., paths=pdd_files) fix is present, #1233/#1235 are merged, the prompt-template overlap was removed, and the hard caps were removed.

Required changes before merge:

1. Update the source prompt contract. pdd/sync_orchestration.py now implements a progress-sensitive outer consecutive-fix breaker, but pdd/prompts/sync_orchestration_python.prompt still says Consecutive fix operations: Break after 5. In this prompt-driven repo, that leaves the generated module behavior out of sync with its source prompt and risks a future pdd sync sync_orchestration regenerating the old progress-blind breaker. Add the progress-sensitive rule to the prompt contract, including the paths=pdd_files run-report lookup requirement for #1211.

2. Fix the whitespace / generated artifact churn. git diff --check origin/main...origin/pr/1245 currently fails on trailing whitespace in context/sync_orchestration_example.py lines 34, 37, 40, 47, 49, 53, 69, and 74. Also please review whether the .pdd/meta/sync_orchestration_python.json reset to null hashes and deletion of .pdd/meta/sync_orchestration_python_run.json are intentional; those changes look like auto-heal artifact churn rather than part of the fix.

Verification I ran locally in a clean merge worktree:

• conda run -n pdd pytest tests/test_issue_1203_breaker.py -q -> 4 passed
• conda run -n pdd pytest tests/test_sync_orchestration.py::test_budget_exceeded tests/test_sync_orchestration.py::test_dry_run_mode -q -> 2 passed
• python -m compileall -q pdd/sync_orchestration.py tests/test_issue_1203_breaker.py context/sync_orchestration_example.py -> passed
• git diff --check origin/main...origin/pr/1245 -> failed as above

Once the prompt contract and diff hygiene are fixed, this should be close to mergeable.

[sohni-tagirisa]

Addressed the latest review blockers in 7c5ada4. The sync orchestration prompt now documents the progress-sensitive outer consecutive-fix breaker, including the path-aware read_run_report(basename, language, paths=pdd_files) requirement for subproject metadata (#1211). I also removed the generated artifact/example churn from the PR diff; the branch diff against origin/main is now limited to pdd/sync_orchestration.py, pdd/prompts/sync_orchestration_python.prompt, and tests/test_issue_1203_breaker.py, and git diff --check origin/main...HEAD is clean locally. Local validation passed: python -m pytest tests/test_issue_1203_breaker.py -q; python -m pytest tests/test_sync_orchestration.py::test_budget_exceeded tests/test_sync_orchestration.py::test_dry_run_mode -q; python -m compileall -q pdd/sync_orchestration.py tests/test_issue_1203_breaker.py context/sync_orchestration_example.py.

[gltanaka]

Looks good to merge from code review.

The previous blockers are addressed:

• sync_orchestration_python.prompt now documents the progress-sensitive outer consecutive-fix breaker and the required read_run_report(..., paths=pdd_files) lookup.
• The generated artifact churn and trailing whitespace are gone; the diff is down to the prompt contract, sync_orchestration.py, and the focused regression test.
• The PR still avoids the earlier prompt-template overlap and hard-cap additions.

Local verification against a clean merge into current origin/main:

• conda run -n pdd pytest tests/test_issue_1203_breaker.py -q -> 4 passed
• conda run -n pdd pytest tests/test_sync_orchestration.py::test_budget_exceeded tests/test_sync_orchestration.py::test_dry_run_mode -q -> 2 passed
• python -m compileall -q pdd/sync_orchestration.py tests/test_issue_1203_breaker.py -> passed
• git diff --cached --check in the merge worktree -> passed

GitHub currently marks the branch as behind, but the local merge onto current origin/main applied cleanly.

[sohni-tagirisa]

Verified necessity against the real sync_orchestration loop + added a regression test

Pushed 304b58c: a new integration test, test_consecutive_fix_breaker_stays_open_while_failures_decrease in tests/test_sync_orchestration.py, that exercises the actual production loop (not a re-implementation).

Why this test

The PR's existing test_progress_sensitive_breaker_condition re-implements the breaker decision inline, so it would pass even if the production breaker were never changed. The new test drives the real sync_orchestration() on a real on-disk project (prompt/code/test/example + a real .pdd/meta run report), with only the LLM-calling leaf ops stubbed. The run report's failing-test count strictly decreases across consecutive fix operations (5→4→3→2→1→0), and the real breaker reads it via the real read_run_report(..., paths=...).

Result (proves the change is necessary)

Same test, both code versions:

• This PR → PASS: the breaker stays open past the cap of 5 and the loop converges to all_synced.
• origin/main → FAIL, with the exact Issue Fix-loop circuit breaker doesn't account for multi-failure regenerations; test generator writes exact-string assertions that subsequent regenerations don't preserve #1203 symptom:

  LogicalFailure: "Detected 5 consecutive fix operations. Breaking infinite fix loop.", consecutive: 5, operations_completed: ['fix','fix','fix','fix']

So on a loop that is converging one failure per iteration, main's unconditional consecutive_fixes >= 5 aborts after 4 completed fixes; this PR lets it finish. On a genuinely stuck loop (no run report / non-decreasing failures) both versions still break, so the infinite-loop protection is preserved (covered by the pre-existing test_sync_orchestration_records_logical_failure_in_core_dump_errors).

How this PR differs from the other #1203 children (for reviewers)

#1203 is fixed across three layers; this PR is only the outermost one:

• feat: use pattern-based pytest.raises(match=) in test-generation prompts (#1231) #1235 (closes Update test-generation prompt to use pattern-based pytest.raises(match=) instead of exact strings #1231, merged) — test-generation prompt: pattern-based pytest.raises(match=) instead of exact strings (removes the root cause of N-simultaneous failures).
• chore: PDD sync for #1230 #1233 (syncs Make fix-loop circuit breaker progress-sensitive instead of raw-iteration-count-based #1230, merged) — inner fix_error_loop breaker made progress-sensitive (iterations within one fix_main(loop=True, max_attempts=5) call).
• fix(sync): make outer consecutive-fix breaker progress-sensitive (refs #1203) #1245 (this PR) — outer sync_orchestration consecutive-fix breaker made progress-sensitive (how many times the orchestrator re-selects fix). A single outer fix runs the whole inner loop, so even with chore: PDD sync for #1230 #1233 the outer breaker was still progress-blind. Refs Fix-loop circuit breaker doesn't account for multi-failure regenerations; test generator writes exact-string assertions that subsequent regenerations don't preserve #1203, does not close it; parent verification is tracked by Fix-loop circuit breaker doesn't account for multi-failure regenerations; test generator writes exact-string assertions that subsequent regenerations don't preserve (#1203) #1300.

🤖 Generated with Claude Code

[gltanaka]

Still good to merge after the latest update.

The new real-loop regression test is a useful addition: it exercises the production sync_orchestration() loop and demonstrates the breaker stays open past the old cap while failures strictly decrease.

Local verification against a clean merge onto current origin/main:

• conda run -n pdd pytest tests/test_issue_1203_breaker.py tests/test_sync_orchestration.py::test_consecutive_fix_breaker_stays_open_while_failures_decrease -q -> 5 passed
• conda run -n pdd pytest tests/test_sync_orchestration.py::test_budget_exceeded tests/test_sync_orchestration.py::test_dry_run_mode -q -> 2 passed
• python -m compileall -q pdd/sync_orchestration.py tests/test_issue_1203_breaker.py tests/test_sync_orchestration.py -> passed
• git diff --cached --check in the merge worktree -> passed

GitHub still reports the branch as behind, but it remains mergeable and the local merge onto current origin/main applied cleanly.

[gltanaka]

Re-approved after updating the branch from main. The diff remains scoped to the same four files, and is clean locally. Waiting on the fresh required checks before merge.

[gltanaka]

Re-approved current head after the latest branch update from main. Diff remains unchanged in scope and is clean.

[gltanaka]

Re-approved current head after branch update. Diff remains scoped to four files and diff check is clean; waiting on fresh required checks before merge.

[gltanaka]

Re-approved after updating the branch to current main. Revalidated that the refreshed diff is unchanged in scope (same 4 files, 211 insertions/4 deletions) and passes git diff --check. Waiting on the rerun required checks before merge.