fix: classify Modal sandbox / tests-dir loss as transient_infra#53
fix: classify Modal sandbox / tests-dir loss as transient_infra#53shehabyasser-scale wants to merge 3 commits into
Conversation
| r"sandbox|streamterminated|" | ||
| r"failed.?to.?add.?tests.?directory|addtestsdirerror|" | ||
| r"loading.?container|fetchspec", | ||
| re.IGNORECASE, | ||
| ), |
There was a problem hiding this comment.
Bare
sandbox term over-classifies candidate errors as infra
The alternation opens with the unanchored word sandbox, which matches any error string containing that substring — e.g. "Stripe sandbox is not enabled for this account", "Reached rate limit on Sandbox plan", or any SWE-bench task whose ground-truth fix touches sandbox-related code and whose candidate crashes with a message that happens to include "sandbox". Those cases would be silently reclassified from TASK_FAILURE (informative, scored) to TRANSIENT_INFRA (excluded, counted toward invalidity), producing a falsely elevated invalidity fraction and suppressing real signal. A more precise anchor — e.g. modal.*sandbox|sandbox.*(unavailable|not.?found|shut.?down) — targets the three actual Modal lifecycle messages from the PR description without catching unrelated uses of the word.
Prompt To Fix With AI
This is a comment left during a code review.
Path: vero/src/vero/evaluation/error_taxonomy.py
Line: 170-174
Comment:
**Bare `sandbox` term over-classifies candidate errors as infra**
The alternation opens with the unanchored word `sandbox`, which matches any error string containing that substring — e.g. `"Stripe sandbox is not enabled for this account"`, `"Reached rate limit on Sandbox plan"`, or any SWE-bench task whose ground-truth fix touches sandbox-related code and whose candidate crashes with a message that happens to include "sandbox". Those cases would be silently reclassified from `TASK_FAILURE` (informative, scored) to `TRANSIENT_INFRA` (excluded, counted toward invalidity), producing a falsely elevated invalidity fraction and suppressing real signal. A more precise anchor — e.g. `modal.*sandbox|sandbox.*(unavailable|not.?found|shut.?down)` — targets the three actual Modal lifecycle messages from the PR description without catching unrelated uses of the word.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
#53 stopped a dead sandbox being scored as an informative zero. The same masking survives one layer up, for a cause that is even easier to fix. In the 2026-07-25 swe-atlas-qna run, 145 of 469 cases ended on: NotFoundError: Error code: 404 - {'error': {'type': 'invalid_request_error', 'code': 'DeploymentNotFound', 'message': 'The API deployment for this resource does not exist. ...'}} The configured eval model is not provisioned on the Azure resource. The agent's every call 404s, it writes no answer, and `classify_case` — finding nothing in the signal it recognises — returns TASK_FAILURE, whose policy is `is_informative_sample=True`. So each case was recorded as a genuine score of 0.0 and the harness reported that the candidate failed the task. Add UPSTREAM_MODEL_UNAVAILABLE: never an informative sample, never retried, terminating (every remaining case fails identically, so there is nothing left to measure), and counted toward invalidity so the aggregate still comes out invalid if the terminating path is ever bypassed. It is ranked above TRANSIENT_INFRA, so a case that saw both a dead sandbox and a missing deployment reports the deterministic, operator-fixable cause. The pattern deliberately matches the provider error codes and the exact Azure sentence rather than a bare "does not exist": 102 cases in that same run failed with `FetchSpec failed: loading container: file does not exist`, which is genuine infrastructure and must stay TRANSIENT_INFRA. There is a test pinning both sides. Refs #51. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
#53 stopped a dead sandbox being scored as an informative zero. The same masking survives one layer up, for a cause that is even easier to fix. In the 2026-07-25 swe-atlas-qna run, 145 of 469 cases ended on: NotFoundError: Error code: 404 - {'error': {'type': 'invalid_request_error', 'code': 'DeploymentNotFound', 'message': 'The API deployment for this resource does not exist. ...'}} The configured eval model is not provisioned on the Azure resource. The agent's every call 404s, it writes no answer, and `classify_case` — finding nothing in the signal it recognises — returns TASK_FAILURE, whose policy is `is_informative_sample=True`. So each case was recorded as a genuine score of 0.0 and the harness reported that the candidate failed the task. Add UPSTREAM_MODEL_UNAVAILABLE: never an informative sample, never retried, terminating (every remaining case fails identically, so there is nothing left to measure), and counted toward invalidity so the aggregate still comes out invalid if the terminating path is ever bypassed. It is ranked above TRANSIENT_INFRA, so a case that saw both a dead sandbox and a missing deployment reports the deterministic, operator-fixable cause. The pattern deliberately matches the provider error codes and the exact Azure sentence rather than a bare "does not exist": 102 cases in that same run failed with `FetchSpec failed: loading container: file does not exist`, which is genuine infrastructure and must stay TRANSIENT_INFRA. There is a test pinning both sides. Refs #51. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The harbor backend's error taxonomy records any unrecognized case failure as TASK_FAILURE: an informative, scoreable sample at the failure value with status SUCCESS. That is right for a candidate that crashes on its own, but Modal sandbox lifecycle failures and a missing held-out tests fixture are infrastructure the candidate did not cause. Seen on a swe-atlas-qna run (2026-07-24): all 396 cases returned status=success, error_category=task_failure, score=0.0. Terminal exceptions were AddTestsDirError x144, Modal sandbox NotFound x125, RuntimeError x67, BadRequestError x60. Classified as task failures, the aggregate reported error_rate 0.0 and objective feasible at 0.0, so a fully broken environment looked like a real, shippable zero and the optimizer ran with no gradient (baseline and every candidate 0.0). Add the harness/Modal environment-loss signals (sandbox lifecycle, AddTestsDirError / "failed to add tests directory", container fetch failures, StreamTerminatedError) to the TRANSIENT_INFRA patterns. Those cases are now excluded from the aggregate and counted toward invalidity, so a collapsed environment surfaces as an invalid run instead of a silent zero. A candidate's own bug (ValueError, KeyError, no answer) is still a task failure, unchanged. Covered by two new tests; existing behavior asserted unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ccc7d07 to
ec708e9
Compare
ff0dff8 to
988aa83
Compare
|
Restacked onto the new One interaction worth surfacing, because it partly undercuts this PR's stated motivation. def _attempt_is_infra(self, attempt, *, trusted: bool) -> bool:
"""...For a competitive (agent) evaluation a candidate-controlled trial's own
transient-infra exception is not trusted as infrastructure - the candidate
could emit a timeout/connection error to have its dead attempt excluded
from the mean. Only trusted evaluations honor that signal."""Every pattern this PR adds maps to I think the gate's reasoning is right and I am not proposing to weaken it. But the two signal classes are not equivalent:
So there may be room for a narrow exemption: honour harness-origin sandbox-lifecycle signals even when untrusted, keep the gate for the generic network ones. That is a design call on your gate rather than a fix to mine, so I have left the code alone and am raising it here. If you would rather keep it uniform, this PR is still worth having for the trusted finalization path and for correct diagnostics. Full suite matches the base branch: 11 pre-existing failures, none new. |
fix: a missing upstream deployment is not a task failure
What
The harbor error taxonomy records any unrecognized case failure as
TASK_FAILURE: an informative, scoreable sample at the failure value withstatus=SUCCESS. That is correct for a candidate that crashes on its own, but Modal sandbox lifecycle failures and a missing held-out tests fixture are infrastructure the candidate did not cause, and today they are silently scored 0.Why
On a swe-atlas-qna optimizer run (2026-07-24), all 396 evaluation cases came back
status=success,error_category=task_failure,score=0.0. The terminal exceptions:AddTestsDirError("failed to add tests directory")NotFoundError/ "Sandbox unavailable"RuntimeErrorBadRequestError(gpt-4oreasoning.effort400)Because these were classified as task failures, the aggregate reported
error_rate 0.0andobjective.feasible=trueat0.0: a fully broken environment masqueraded as a real, shippable zero, and the optimizer ran with no gradient (baseline and every candidate 0.0).Change
Add the harness/Modal environment-loss signals (sandbox lifecycle,
AddTestsDirError/ "failed to add tests directory", container fetch failures,StreamTerminatedError) to theTRANSIENT_INFRApatterns inerror_taxonomy.py. Those cases are now excluded from the aggregate and counted toward invalidity, so a collapsed environment surfaces as an invalid run instead of a silent zero.A candidate's own bug (
ValueError,KeyError, no answer) is still a task failure, unchanged.Tests
Two new tests in
test_v05_error_taxonomy.pycover the new signals and assert existing classification behavior is unchanged. Verified locally by exercising the classifier directly (new signals map toTRANSIENT_INFRA, all prior cases unchanged).Related
feat/swe-bench-pro-baseline-scaffold(stacked on the current integration tip).🤖 Generated with Claude Code
Greptile Summary
This PR fixes a critical masking bug: Modal sandbox lifecycle failures and missing held-out test fixtures were classified as
TASK_FAILURE(scored as an informative 0.0) instead ofTRANSIENT_INFRA(excluded from the aggregate), causing a fully broken environment to masquerade as a real zero-score result. It also introduces a newUPSTREAM_MODEL_UNAVAILABLEcategory for cases where the configured model is not provisioned, which similarly should not be attributed to the candidate.UPSTREAM_MODEL_UNAVAILABLEenum and policy (terminating, non-retryable, counts toward invalidity but never scored), with a new regex forDeploymentNotFound/model_not_found/Azure deployment-not-found phrases.TRANSIENT_INFRAregex block covering Modal sandbox lifecycle signals,AddTestsDirError,FetchSpec/loading container, andStreamTerminatedError.classify_caseprecedence updated to insertUPSTREAM_MODEL_UNAVAILABLEbetweenAUTH_FAILUREandTRANSIENT_INFRA, with four new regression tests covering the actual production error strings.Confidence Score: 5/5
Safe to merge — the change only widens the set of signals reclassified from task failure to infrastructure, backed by exact production error strings and covered by new regression tests.
The regex additions are anchored tightly to provider-specific error codes and harness-specific exception names. The new UPSTREAM_MODEL_UNAVAILABLE category is wired correctly into both the policy table and the classify_case precedence chain. The four new tests exercise the actual terminal exceptions from the production incident.
Files Needing Attention: No files require special attention beyond the stale classify_case docstring in error_taxonomy.py.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A["classify_case(signals)"] --> B["classify_signal per signal"] B --> C{Pattern matched?} C -- "budget/quota" --> D["INFERENCE_BUDGET_EXHAUSTED"] C -- "auth/permission" --> E["AUTH_FAILURE"] C -- "deploymentnotfound / model_not_found" --> F["UPSTREAM_MODEL_UNAVAILABLE"] C -- "rate-limit / timeout / connection / 5xx" --> G["TRANSIENT_INFRA (existing)"] C -- "sandbox / addtestsdirerror / streamterminated / loading container / fetchspec" --> H["TRANSIENT_INFRA (new)"] C -- no match --> I["None to TASK_FAILURE"] D & E & F & G & H & I --> J["Precedence resolution"] J --> K{INFERENCE_BUDGET_EXHAUSTED?} K -- yes --> L["terminating, not scored"] K -- no --> M{AUTH_FAILURE?} M -- yes --> N["terminating, not scored"] M -- no --> O{UPSTREAM_MODEL_UNAVAILABLE NEW} O -- yes --> P["terminating, counts toward invalidity, not scored"] O -- no --> Q{TRANSIENT_INFRA?} Q -- yes --> R["retryable, counts toward invalidity, not scored"] Q -- no --> S["TASK_FAILURE: informative score at failure value"]Reviews (3): Last reviewed commit: "Merge pull request #60 from scaleapi/fix..." | Re-trigger Greptile