mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
a24fe7dcbf
## Description
`is_error_content` classifies successful shell commands as errors,
inflating the failure stats that `headroom learn` reports.
The heuristic flags a tool result as an error when it contains any of a
list of substrings, one of which is the bare `"exit code"`:
```python
indicators = [
..., "timed out", "exit code", "FileNotFoundError",
]
return any(ind in snippet for ind in indicators)
```
But agent harnesses (Codex, Grok, opencode, ...) append `exit code 0` to
the output of every **successful** shell command. `"exit code" in
snippet` is `True` for `exit code 0`, so those successes are counted as
failures.
That is not cosmetic: `is_error_content` sets `ToolCall.is_error`, which
feeds:
- the per-project failure rate the digest shows the LLM
(`_build_digest`: "N failures (X%)"), and
- loop classification (`detect_loops` treats a group as an *error loop*
when ≥ half its calls are errors),
so a project where most shell commands succeed can read as one riddled
with failures, biasing the learned recommendations.
## Fix
Match a **nonzero** exit code instead of the bare substring:
```python
_NONZERO_EXIT_RE = re.compile(r"exit code:?\s*(?!0\b)\d", re.IGNORECASE)
...
if any(ind in snippet for ind in indicators):
return True
return bool(_NONZERO_EXIT_RE.search(snippet))
```
`exit code 0` no longer matches. A nonzero code still does — and, as a
small bonus, the case-insensitive regex now also catches `Exit code: 1`
(colon + capitalized), which the old case-sensitive lowercase substring
missed.
Closes #
## Type of Change
- [x] Bug fix (non-breaking change that fixes an issue)
- [ ] New feature (non-breaking change that adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update
- [ ] Performance improvement
- [ ] Code refactoring (no functional changes)
## Changes Made
- `headroom/learn/_shared.py`: replace the `"exit code"` substring
indicator with a nonzero-exit-code regex (`_NONZERO_EXIT_RE`) checked
after the other indicators.
- `tests/test_learn/test_integration.py`: new tests that `exit code 0`
is not an error and a nonzero code (any casing / with a colon) still is.
- `CHANGELOG.md`: Bug Fixes entry.
## Testing
- [ ] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check .`)
- [x] Type checking passes (`mypy headroom`)
- [x] New tests added for new functionality
- [ ] Manual testing performed
### Test Output
```text
$ uvx ruff@0.15.17 check headroom/learn/_shared.py tests/test_learn/test_integration.py
All checks passed!
$ uvx mypy@1.20.2 --ignore-missing-imports headroom/learn/_shared.py
Success: no issues found in 1 source file
```
## Real Behavior Proof
- Environment: Windows 11, Python 3.12, `uvx ruff@0.15.17` / `uvx
mypy@1.20.2`. A full `pytest` OOM-kills this box (ML stack import), so I
reproduced the classifier with a dependency-free script and left the
full pytest to CI.
- Exact command / steps: ran a successful output ending `Process
finished with exit code 0`, plus several nonzero-code failures (`exit
code 1`, `Exit code: 127`, `exit code 137`) and control strings, through
the OLD substring form and the NEW regex form.
- Observed result: OLD flags `exit code 0` as an error; NEW returns
`False` for it, still returns `True` for every nonzero code (including
the colon/capitalized form the old lowercase substring missed), and
leaves the other indicators unchanged.
- Not tested: a full `learn` run over a real history; full local
`pytest` deferred to CI (OOM).
## Review Readiness
- [x] I have performed a self-review
- [x] This PR is ready for human review
## Checklist
- [x] My code follows the project's style guidelines
- [x] I have performed a self-review of my code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works
- [ ] New and existing unit tests pass locally with my changes
- [x] I have updated the CHANGELOG.md if applicable
## Additional Notes
The "unit tests pass locally" box is unchecked because the full suite
imports the ML stack, which I can't run here. The new tests live
alongside the existing `is_error_content` false-positive/true-positive
tests in `tests/test_learn/test_integration.py`, so they run under the
normal CI pytest job; behaviour is additionally verified by the
standalone proof above.
---------
Co-authored-by: JerrettDavis <mxjerrett@gmail.com>
|
||
|---|---|---|
| .. | ||
| __init__.py | ||
| test_analyzer.py | ||
| test_claude_config_dir.py | ||
| test_error_classification.py | ||
| test_gemini_scanner.py | ||
| test_integration.py | ||
| test_loop_weighting.py | ||
| test_opencode_scanner.py | ||
| test_plugin_encoding.py | ||
| test_registry.py | ||
| test_scanner.py | ||
| test_subagent_scanning.py | ||
| test_writer.py | ||