Skip to content

fix(learn): surface Codex analysis failures - #3016

Open
JerrettDavis wants to merge 2 commits into
mainfrom
fix/learn-codex-cli-failure
Open

fix(learn): surface Codex analysis failures#3016
JerrettDavis wants to merge 2 commits into
mainfrom
fix/learn-codex-cli-failure

Conversation

@JerrettDavis

@JerrettDavis JerrettDavis commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

Description

headroom learn could invoke Codex CLI from a non-Git working directory without Codex’s required bypass flag. The resulting backend error was then swallowed by the analyzer and rendered as “No actionable patterns found” with exit code 0. This fixes both coupled defects so Codex can run from discovered project locations and genuine analysis failures remain visible and machine-detectable.

Closes #3008

Type of Change

  • 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

  • Added --skip-git-repo-check to the Codex CLI analysis backend command.
  • Added an explicit analysis_error result field instead of conflating backend failure with an empty recommendation set.
  • Kept multi-project analysis best-effort, while returning exit code 1 after any project analysis fails.
  • Prevented failed analysis from printing a misleading no-pattern success message.
  • Added analyzer and CLI regression coverage for the command and failure-propagation contracts.

Testing

  • Unit tests pass (pytest)
  • Linting passes (ruff check .)
  • Type checking passes (mypy headroom)
  • New tests added for new functionality
  • Manual testing performed

Test Output

uv run pytest -q tests/test_learn/test_analyzer.py tests/test_cli_learn.py
102 passed in 2.34s

uv run pytest -q tests/test_learn tests/test_cli_learn.py
257 passed, 7 skipped in 3.11s

uv run mypy headroom
Success: no issues found in 520 source files

uv run ruff check <changed files>
All checks passed!
uv run ruff format --check <changed files>
5 files already formatted

uv run pytest tests scripts/tests --splits 4 --group N --tb=short -q
shard 1: 2766 passed, 140 skipped in 174.08s
shard 2: 2699 passed, 207 skipped in 60.00s
shard 3: 2822 passed, 84 skipped in 76.10s
shard 4: 2734 passed, 172 skipped in 80.29s

Real Behavior Proof

  • Environment: macOS arm64, Python 3.13, Codex CLI 0.147.0-compatible command surface, current main including fix(ci): prevent native detector from hanging test shards #2996.
  • Exact command / steps: verified codex exec --help; exercised _call_cli_llm with a captured subprocess command; invoked the Click command with a simulated Codex nonzero backend result.
  • Observed result: the subprocess command is codex exec --skip-git-repo-check; backend failure text is printed as Analysis failed, the misleading no-pattern message is absent, and the CLI exits 1.
  • Not tested: live paid Codex analysis against production account credentials; subprocess and CLI behavior are covered deterministically.

Runtime Rollout Safety

  • Rollout-managed feature(s): none; this is CLI-only failure handling.
  • Minimum rollout channel: normal patch release after exact-head CI is entirely green.
  • Stable/default behavior changed: failed LLM analysis now exits nonzero instead of reporting success; successful and genuinely empty analyses are unchanged.
  • Kill switch / disable path: select another backend with HEADROOM_LEARN_CLI or --model if Codex CLI is unavailable.
  • Unsafe override required: none.
  • Qualification impact: all four Python CI shards, static checks, security checks, and command-level regression tests must pass.
  • Rollback path: fix forward through a human-reviewed corrective PR; no persisted data or migration is involved.

Review Readiness

  • I have performed a self-review
  • This PR is ready for human review

Checklist

  • My code follows the project’s style guidelines
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation — inline result-contract documentation; no separate user guide change is required
  • My changes generate no new warnings
  • 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
  • I did not edit CHANGELOG.md — it is generated by release-please from my Conventional Commit PR title (a CI guard enforces this)

Screenshots (if applicable)

Not applicable; command-line backend and exit semantics only.

Additional Notes

Human review only. No merge or auto-merge is configured. This corrects the root failure and exit semantics without extending any timeout.

@codecov-commenter

codecov-commenter commented Aug 14, 2026

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 20.00000% with 8 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
headroom/cli/learn.py 0.00% 8 Missing ⚠️

📢 Thoughts on this report? Let us know!

@github-actions

github-actions Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

PR governance

This PR follows the template and is marked ready for human review.

@JerrettDavis
JerrettDavis marked this pull request as ready for review August 14, 2026 04:52
@github-actions github-actions Bot added status: needs author action Pull request body or readiness checklist still needs author updates status: ready for review Pull request body is complete and the author marked it ready for human review and removed status: needs author action Pull request body or readiness checklist still needs author updates labels Aug 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: ready for review Pull request body is complete and the author marked it ready for human review

Projects

None yet

2 participants