Skip to content

Rectify: Anomaly Detection Observed-Model Self-Comparison & Scanner Budget Exhaustion — PART A ONLY#3365

Merged
Trecek merged 4 commits into
developfrom
pipeline-health-scanner-exhausts-agent-tool-call-budget-on-m/3333
May 31, 2026
Merged

Rectify: Anomaly Detection Observed-Model Self-Comparison & Scanner Budget Exhaustion — PART A ONLY#3365
Trecek merged 4 commits into
developfrom
pipeline-health-scanner-exhausts-agent-tool-call-budget-on-m/3333

Conversation

@Trecek
Copy link
Copy Markdown
Collaborator

@Trecek Trecek commented May 31, 2026

Summary

Commit a5a68f49 introduced a regression in session_log.py where the _observed model variable is set to model_identifier (the configured model) when model_identifier is non-empty. This causes detect_model_drift(model_identifier, model_identifier) — comparing the configured model against itself — which permanently disables MODEL_DRIFT detection for all sessions with a configured model. The existing test test_flush_session_log_no_false_drift_with_configured_model validates this broken behavior as correct, and the AST guard in test_model_identity_contract.py only covers detect_model_drift internals, not the call site in session_log.py.

The architectural weakness is that _observed is computed at the call site with no structural enforcement that it must come from a different data source than configured_model. The detection function detect_model_drift(configured, observed) has a two-value comparison contract, but the call site collapsed both values to the same source. No test asserts the positive case (real drift fires through flush_session_log), and no AST guard prevents future regressions at the call site.

Part A fixes the _observed computation bug plus adds the tests and AST guard that make this class of regression structurally impossible. Part B will cover scanner-level improvements.

Requirements

Conflict Resolution Decisions

The following files had merge conflicts that were automatically resolved.

Implementation Plan

Plan file: /home/talon/projects/autoskillit-runs/remediation-20260530-171650-060527/.autoskillit/temp/rectify/rectify_anomaly_detection_observed_model_self_comparison_2026-05-30_172500.md

🤖 Generated with Claude Code via AutoSkillit

Token Usage Summary

Step Model count uncached output cache_read peak_ctx turns cache_write time
rectify* opus[1m] 1 2.9k 23.7k 3.7M 132.2k 339 210.3k 20m 18s
review_approach* sonnet 1 9.9k 7.8k 198.5k 53.9k 112 42.9k 6m 16s
dry_walkthrough* opus 1 39 7.0k 571.8k 62.5k 76 46.3k 3m 48s
implement* sonnet 1 1.1k 12.8k 1.1M 58.6k 76 42.7k 5m 1s
audit_impl* sonnet 1 70 8.7k 235.1k 36.5k 27 28.9k 3m 5s
prepare_pr* sonnet 1 99.5k 4.1k 247.6k 30.9k 25 43.8k 1m 55s
compose_pr* sonnet 1 39.4k 1.6k 182.8k 30.9k 14 15.6k 48s
review_pr* sonnet 1 214 28.6k 1.3M 76.4k 64 60.9k 6m 39s
resolve_review* opus 1 35 5.4k 904.2k 67.3k 37 72.0k 6m 46s
Total 153.1k 99.7k 8.4M 132.2k 563.2k 54m 41s

* Step used a non-Anthropic provider; caching behavior may differ.

Token Efficiency

Step LoC Changed cache_read/LoC cache_write/LoC output/LoC
rectify 0
review_approach 0
dry_walkthrough 0
implement 109 9832.7 391.6 117.1
audit_impl 0
prepare_pr 0
compose_pr 0
review_pr 0
resolve_review 60 15070.6 1200.2 89.8
Total 169 49809.7 3332.6 589.9

Model Usage Breakdown

Model steps uncached output cache_read cache_write time
opus[1m] 1 2.9k 23.7k 3.7M 210.3k 20m 18s
sonnet 6 150.1k 63.6k 3.3M 234.6k 23m 47s
opus 2 74 12.3k 1.5M 118.3k 10m 35s

Trecek and others added 4 commits May 30, 2026 18:23
Adds two tests that expose the a5a68f4 regression in flush_session_log
where _observed = model_identifier causes detect_model_drift(X, X):

- test_flush_session_log_genuine_drift_with_configured_model: positive
  integration test through flush_session_log; asserts MODEL_DRIFT fires
  when configured=opus but API-observed dominant model=sonnet.

- test_drift_call_site_uses_independent_observed_source: AST guard that
  walks flush_session_log, finds the detect_model_drift call site, and
  asserts _observed is assigned from _primary_model_identifier() with no
  ternary branch that collapses it back to model_identifier.

Both tests fail against the current code and will pass after the fix.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ifier

Regression introduced in a5a68f4: when model_identifier was set, _observed
was assigned model_identifier, causing detect_model_drift(X, X) — comparing
the configured model against itself, which permanently disables MODEL_DRIFT
detection for any session with a configured model.

Fix: _observed is now always derived from _primary_model_identifier(token_usage)
(the API-observed dominant model). When model_identifier and the argmax model
genuinely agree, detect_model_drift correctly returns no anomaly via _models_match
alias handling. When they differ, drift is correctly emitted.

effective_model_id is unchanged — it retains model_identifier-or-argmax fallback
semantics for session metadata, which is a distinct use case from drift detection.

Also updates:
- test_flush_session_log_no_false_drift_with_configured_model: corrects the test
  scenario (opus configured, opus dominates breakdown) and docstring (previously
  encoded the broken behavior as expected).
- test_primary_model_identifier_argmax_returns_subagent_when_dominant: docstring
  now accurately describes flush_session_log's actual behavior after the fix.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The AST loop checking _observed assignment only matched ast.Assign.
If production code refactored to an annotated assignment (_observed: str = ...),
the guard would silently fall through to pytest.fail with a misleading message.
Now also matches ast.AnnAssign for structural robustness.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… message

Parse JSONL lines once then filter, instead of calling json.loads twice per
line. Also adds a descriptive failure message to the drift_entries length
assertion for easier diagnosis.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Trecek Trecek force-pushed the pipeline-health-scanner-exhausts-agent-tool-call-budget-on-m/3333 branch from c63c7a0 to a8993a1 Compare May 31, 2026 01:23
@Trecek Trecek added this pull request to the merge queue May 31, 2026
Merged via the queue into develop with commit b2f1c33 May 31, 2026
3 checks passed
@Trecek Trecek deleted the pipeline-health-scanner-exhausts-agent-tool-call-budget-on-m/3333 branch May 31, 2026 01:36
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.

1 participant