Skip to content

fix(agents): restore protected theme statements after a drifting retry - #12209

Merged
MarkusNeusinger merged 3 commits into
mainfrom
fix/agent-theme-drift-restore
Oct 16, 2026
Merged

MarkusNeusinger merged 3 commits into
mainfrom
fix/agent-theme-drift-restore

Conversation

@MarkusNeusinger

Copy link
Copy Markdown
Owner

Summary

  • Restore instead of ending the round. When an adapter answer fails its check only because it drifted the protected theme statements (drift:theme, drift:theme_token, drift:savefig), and the round's check retry is already spent, edits.restore_protected splices the base's statement text back over the drifted statements. The result then goes through the normal _check, render and review. Owner decision 2026-10-16, live case "heat3" (round 8): no third round, no new forced-repair ids.
  • Guards. A rebind (drift:rebind) is never restored. Nothing is restored when a protected statement was removed, when it shares its lines with another statement (x = 1; INK = ...), or when the splice does not parse or still drifts. AppliedPlan.drifted carries the plan's result when drift alone refused it, and apply_plan itself is unchanged in behavior.
  • Observability. pipeline_check logs outcome drift_restored with the restored drift kinds (one line per attempt; rejected if the restored code then fails the validator). The pipeline_run telemetry line gains a drift_restored bool, which is added to docs/reference/agent-telemetry.md and to the pipeline_runs view in agents/telemetry/views/02_event_views.sql. The view needs a reload in BigQuery (it selects fields explicitly).

Decisions made in this PR

  • A drifted statement is found in the plan's code by name (the module-level assignment of THEME or of the token, the final module-level savefig), not by region detection. A full file that turns PAGE_BG into a plain PAGE_BG = "#FFFFFF" is no longer a theme token there, so the region finder would miss it.
  • When the base's statement ends the file without a line break and code follows it in the plan, the splice adds one, so plt.close() after the savefig survives.
  • With AGENT_CHECK_RETRY=false there is no retry to spend, so a restorable drift is restored on the first miss. The restore also applies to round 1's retry (attempt 2 may send a full file) and does not require the first answer of the round to have drifted too. The rule is "no retry left and this answer is drift-only".

Plan

N/A (owner decision 2026-10-16, relayed in the delegate brief)

Test plan

  • tests/unit/agents/code/test_edits.py::TestRestoreProtected: restore for each allowed kind (THEME value, THEME no longer getenv, token value, token no longer a theme conditional, savefig, all three at once), keeping a plot change; end-of-file savefig; no restore on rebind, removal, a shared line, an unparseable splice (monkeypatched _splice), no drift or an unparseable plan.
  • tests/unit/agents/runtime/test_service_flow.py::test_a_repair_that_drifts_the_theme_twice_renders_with_the_theme_restored: round 1 is rejected, round 2's full-file answer and its check retry both drift THEME and PAGE_BG, and the restored code (with the repair's marker change) renders, passes review and ships. pipeline_check outcomes are ok, edits_failed and drift_restored; pipeline_run.drift_restored is true. The shipped attempt is 3 (the retry), which is shipped round 2.
  • ruff check, ruff format --check, mypy api core agents, pytest tests/unit (12,135 passed), changelog check.
  • After merge: reload agents/telemetry/views/02_event_views.sql in BigQuery so pipeline_runs.drift_restored exists.

🤖 Generated with Claude Code

https://claude.ai/code/session_01GdMSqLR5ww4ji74EUSmk9M

…k retry

A plan whose only failure is drift of the protected THEME assignment, theme
tokens or final savefig gets those statements restored from the code before
the plan once the round's check retry is spent (owner decision 2026-10-16),
and goes on through check, render and review instead of ending the round.
A rebind is never restored. pipeline_check logs outcome drift_restored and
pipeline_run telemetry gains a drift_restored flag.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GdMSqLR5ww4ji74EUSmk9M
Copilot AI balanced review requested due to automatic review settings October 16, 2026 20:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Restoration can copy unrelated statements from shared base lines and silently undo plot repairs.

1 open finding
What changed in this PR

Restores protected theme statements after an adapter exhausts its check retry, allowing plot repairs to continue through validation, rendering, and review.

Changes:

  • Adds guarded restoration and pipeline integration.
  • Adds restoration telemetry and a BigQuery view field.
  • Adds regression coverage and updates documentation.
File Description
tests/​unit/​agents/​runtime/​test_service_flow.py Tests restoration through the pipeline.
tests/​unit/​agents/​code/​test_edits.py Tests restoration behavior and guards.
docs/​reference/​agent-telemetry.md Documents the restoration flag.
docs/​concepts/​agent-network.md Explains restoration and retry behavior.
changelog.d/​agent-theme-drift-restore.md Records the fix.
agents/​telemetry/​views/​02_event_views.sql Exposes the restoration flag.
agents/​README.md Updates retry documentation.
agents/​anyplot/​telemetry.py Adds the telemetry field.
agents/​anyplot/​settings.py Clarifies retry behavior in documentation.
agents/​anyplot/​pipeline.py Integrates restoration and logging.
agents/​anyplot/​code/​edits.py Implements protected-statement restoration.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread agents/anyplot/code/edits.py Outdated
…lines

Splicing the base's whole lines back would also restore another statement on
the same line and undo a repair's change to it (Copilot review of #12209).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GdMSqLR5ww4ji74EUSmk9M

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

Restoration retains downstream validation, regression coverage includes the resolved shared-line issue, and no blocking findings remain.

0 open findings

1 resolved since last review

🧠 Review effort: Balanced

@MarkusNeusinger
MarkusNeusinger merged commit d2ef494 into main Oct 16, 2026
16 checks passed
@MarkusNeusinger
MarkusNeusinger deleted the fix/agent-theme-drift-restore branch October 16, 2026 20:14
@codecov

codecov Bot commented Oct 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.86047% with 7 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
agents/anyplot/code/edits.py 90.14% 7 Missing ⚠️

📢 Thoughts on this report? Let us know!

MarkusNeusinger added a commit that referenced this pull request Oct 16, 2026
## Summary
- Cut the 3.5.0 section with `uv run python -m tools.changelog release
3.5.0 --title "Bring your own data"`. It folds all 99 fragments in
`changelog.d/` (agent network window: #12099 to #12209), deletes them,
repoints the compare links, and bumps `pyproject.toml`, `uv.lock` and
`app/package.json` to 3.5.0.
- Add the hand-written parts: a section intro (precedent from 3.4.0) and
the italic *Catalog* line. No catalogue PRs merged since v3.4.0, and
coverage stays at 315 of 325 specs. No **Dependencies:** bullet, because
no Dependabot PR merged in the window. The five grouped bumps #12160 to
#12164 are still open.
- Fix the fragment `api-agent-secret-binding.md`, which had no PR
reference, by adding `(#12128)` before the fold.

The release is the admin-only "Use with my data" agent chat. The section
does not suggest that it is publicly available, because that is reserved
for v4.0.0.

## Plan
`agentic/commands/release.md`. Tag `v3.5.0` and the condensed GitHub
release follow after the merge.

## Test plan
- [x] `uv run python -m tools.changelog check --base origin/main` passes
- [x] `uv lock --check` passes
- [x] `tests/unit/test_version_sync.py` and `tests/unit/tools` pass
- [x] The diff touches only `CHANGELOG.md`, `pyproject.toml`, `uv.lock`,
`app/package.json` and the 99 deleted fragments
- [ ] CI green

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01GdMSqLR5ww4ji74EUSmk9M

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants