Skip to content

fix(enhance): a completed paid classification survives a post-result assembly failure — the finish validator enforces include_functions element types (#614) - #638

Merged
gadievron merged 3 commits into
masterfrom
fix/issue614-finish-payload-validation
Sep 17, 2026
Merged

gadievron merged 3 commits into
masterfrom
fix/issue614-finish-payload-validation

Conversation

@gadievron

@gadievron gadievron commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator

Fixes #614.

What was wrong

The agentic enhance loop stored a completed, paid classification and then overwrote it with an error dict when post-result assembly failed:

  • agent.py assigns the finished, billed classification to the unit's agent_context.
  • The assembly loop then dereferenced the model's raw include_functions finish input — and the finish validator checked field presence only, never element types. A bare-string item raised AttributeError: 'str' object has no attribute 'get'.
  • The handler then caught the raise and replaced the stored context with the error dict — destroying the completed classification and its metadata.

The referenced run's receipt: all 12 raise-class enhance errors are exactly this AttributeError — the paid classification existed, was stored, and was then silently discarded.

The fix (three layers)

  1. The validator: include_functions must be a list of objects with string ids — enforced at the finish schema boundary (the model self-corrects on rejection). (The repo's own degenerate-exit test fixture carried the bare-string shape — presence-only validation had let it through; the fixture is corrected to the documented contract.)
  2. Preserve on failure: agent.py's assembly block is wrapped — a raise marks assembly_error inside the stored context (the classification, reasoning, confidence, and recorded usage all survive; the additional code is simply not inlined). Never overwrite paid work with an error dict. The origin metadata computes into locals before the commit (a metadata raise no longer leaves inlined code with deps_inlined unset). The wrap does NOT re-raise — the preservation is delivered by the marker alone.
  3. The handler distinguishes the two error shapes: an assembly_error-marked context is preserved and re-classified from it (the completed verdict exists — the summary/stats keying stays consistent); a genuine agent-loop failure (an LLM/parse raise with no completed context) still takes the error dict. Reachability, disclosed: the wrap does not re-raise, so the branch is defense-in-depth for a future raise-after-store.

The agentic stats no longer count units_with_context/functions_added for assembly-error units (delivery that failed); the verdict bucket still counts them (a completed analysis whose inlining failed is still a verdict).

Evidence

  • RED (pristine master, this file copied in): 4 failed + 2 passed (the 6-item set — the two validator rows, the preserve, the handler; the valid-elements and the then-vacuous control are the green rows).
  • GREEN: 9/9 on the file (pytest -q tests/test_issue614_finish_payload.py9 passed) — the bare-string / non-list / non-string-id validator rows; the preserve receipt driving the real enhance_unit_with_agent with a poisoned index (the completed neutral classification + reasoning survive, error is None, the marker carries the exception class); the defense-in-depth receipt_finish monkeypatched to the regressed presence-only shape, the historical bare-string payload driven through the real entry point (the wrap catches at the historical AttributeError site; the classification survives; the handler never fires); the behavioral handler receipt — the real enhance_dataset_agentic closure driven with a store-then-raise fake (the context preserved, the classification re-read from it, never the error dict); the stats-gate receipt (an assembly-error unit: no units_with_context claim; the verdict still bucketed). The full suite locally: pytest -q2 failed, 4208 passed, 34 skipped — the 2 are test_llm_sdk_contract_floor SDK-pin host drift, identical on master; CI is the authoritative suite. Ruff clean; Semgrep 0 findings.
  • The fixture correction re-routes three existing tests to their intended paths (the bare string previously never reached the assembly — analyze_unit does not run it; with the new validator the old fixture would have broken test_complete_finish_still_accepted and The agentic enhance give-up classes are conflated under one 'incomplete' classification — expose exit_kind diagnostics (the iteration-limit budget exhaustion differs in kind and remedy) #615's finish_truncated receipt).

The de-balancing check (the intent trace)

serves: I-enh; de-balances-checked: the genuine-failure error dict is byte-identical to master (the preserve only widens); every == "incomplete" consumer unchanged (#615's exit_kind stamping untouched — the wrap is disjoint).

Notes for the reviewer

  • The error_summary grouping follow-up from the issue stands: the preserved-context path reduces the error cohort to genuine failures, where a finer grouping by exception_class becomes meaningful.
  • The assembly's remaining raise sites (a non-dict index return, a non-str language) are all inside the wrap — they preserve the classification by construction; the validator closes the front door for the historical shape.

…assembly failure — the finish validator enforces include_functions element types (#614)

The defect chain: agent.py stores the finished, billed
classification; the assembly loop then dereferenced the model's RAW
include_functions finish input — and the finish validator checked
field PRESENCE but never element TYPES, so a bare string raised
AttributeError ('str' object has no attribute 'get'); and
context_enhancer's handler then REPLACED the stored context with an
error dict — destroying the paid classification and its metadata (the
referenced run's 12 raise-class errors are exactly this shape).

Three fixes:
1. THE VALIDATOR: include_functions must be a list of OBJECTS —
   checked at the finish schema boundary (the root; the degenerate-exit
   fixture itself carried the bare-string shape and is corrected).
2. PRESERVE ON FAILURE: agent.py's assembly is wrapped — a raise marks
   assembly_error INSIDE the stored context (the classification, the
   reasoning, the confidence, and the recorded usage all survive; the
   additional code is simply not inlined).
3. THE HANDLER distinguishes the two error shapes: an
   assembly_error-marked context is PRESERVED (a completed verdict
   exists); a genuine agent-loop failure (an LLM/parse raise with no
   completed context) still takes the error dict.

serves: I-enh, I-out. de-balances-checked: the degenerate-exit
classification semantics unchanged (the corrected fixture still
asserts security_control accepted); the error-dict path preserved for
the genuine failure shape.

Refs #614
…e stats honest, the validator id-checked, the wrap's true site pinned (#614)

The combined 2d+DU seat's findings, all dispositioned:

- the handler's preserve branch re-READS the classification from the
  preserved context (a stale 'error' classification against a completed
  context broke the summary/stats identity); the reachability disclosed
  (the wrap does not re-raise — the branch is defense-in-depth for a
  future raise-after-store).
- _compute_agentic_stats no longer counts units_with_context /
  functions_added for assembly-error units (delivery that failed).
- the validator enforces the schema's id: string (an unhashable id
  reached the index lookup and raised — swallowed by the wrap, but the
  model should self-correct instead).
- the vacuous 'healthy assembly' test replaced by the DEFENSE-IN-DEPTH
  receipt: _finish monkeypatched to the regressed presence-only shape,
  the REAL enhance_unit_with_agent driven with the historical
  bare-string payload — the wrap catches at the historical site
  (AttributeError) and the classification survives; the false
  error_summary-regrouping docstring claim removed; the false
  index.adapter comment corrected.
- the assembly try's 2-space indentation normalized; the CHANGELOG
  entry added (the repo's stated convention).

6/6 + the sibling suites green (32 passed across the 4 files); ruff
clean. RED at base: 4 failed + 2 passed (the 6-item set, receipt
red-receipt-638641.log).
…, the stats gate + the id validator pinned, the partial-mutation reordered, the artifacts cleaned (#614)

The panel + deep-refute findings, all dispositioned:

- the duplicated #615 CHANGELOG heading (an insertion artifact) deleted.
- the handler's source-string pin RETIRED: the behavioral receipt drives
  the REAL enhance_dataset_agentic closure with a fake agent entry that
  stores a completed assembly_error-marked context then raises (the
  future raise-after-store shape) — the context preserved, the
  classification re-read (never the error dict). The fake's signature
  bug (5 positional args at the call site) caught by direct
  reproduction.
- the stats gate pinned (an assembly_error unit: units_with_context /
  functions_added NOT counted; the verdict bucket still counted); the
  validator's string-id half pinned.
- the assembly's partial-mutation reordered: the origin metadata
  computes into locals FIRST; primary_code + primary_origin commit LAST
  (a raise in the metadata no longer leaves inlined code with
  deps_inlined unset).
- the CHANGELOG '5 files' corrected (6: the degenerate-exit fixture is
  the sixth); the test's agent.py:585 cite corrected (:600).

9/9 on the file + the sibling suites; ruff clean.
@gadievron
gadievron force-pushed the fix/issue614-finish-payload-validation branch from fa199b4 to 7f879f9 Compare September 17, 2026 12:29
context and THEN raises (the future raise-after-store shape the
handler branch guards) — the context is preserved and the unit's
classification is the PRESERVED one (never the error dict)."""
import utilities.context_enhancer as ce_mod
@gadievron
gadievron merged commit fc95049 into master Sep 17, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant