CI: Make Codex complexity classification non-fatal

[mszeszko-meta]

Previously, Classify PR complexity (Codex) ran under bash -e, so any codex exec failure aborted the entire Codex review before the real review step could run. The classifier only selects the review budget, so on failure we now log the classifier output tail, default to the complex review budget, and continue. This keeps actual Codex review failures visible through the existing review exit-code/log handling while preventing the auxiliary classifier from blocking review generation.

[github-actions]

✅ clang-tidy: No findings on changed lines

Completed in 0.0s.

[github-actions]

✅ Claude Code Review

Auto-triggered after CI passed — reviewing commit f4ea128

---

Summary

Clean, well-structured defensive change that makes the Codex complexity classifier non-fatal in two parallel CI job locations. The fallback-to-complex approach is safe because the downstream "Pick thinking budget" steps already default to complex when the file is missing or unparseable. The two modified blocks are consistent with each other.

High-severity findings (0):
No high-severity findings.

Full review (click to expand)

Findings

🔴 HIGH

None.

🟡 MEDIUM

M1. First block: early-exit on missing API key now writes file but still exits before set +e — ai-review-analysis.yml:554

• Issue: In the first block (line ~554 in the pre-patch file), the exit 0 on missing OPENAI_API_KEY was already present before this PR. The PR correctly adds printf 'complex\n' > codex-complexity-output.txt before that exit 0, ensuring the downstream step has a file to read. This is fine.
• However, consider: if the workflow's default shell has set -e (GitHub Actions default), the exit 0 is a clean exit so the step passes. No issue here — this is actually handled correctly. Downgraded to informational.

M2. Second block: missing API key guard was absent before this PR — ai-review-analysis.yml:1143

• Issue: The second "Classify PR complexity (Codex)" block (the manual-trigger job) previously had no OPENAI_API_KEY check at all. The PR adds one, which is a good catch beyond the stated scope. This is a correctness improvement.
• Assessment: Positive change, no issue.

🟢 LOW / NIT

L1. Duplicated shell logic across two jobs

• Issue: The two "Classify PR complexity (Codex)" steps (lines ~551 and ~1142) now have near-identical error-handling logic. If future changes are needed, both must be updated in sync. Consider extracting to a composite action or reusable script.
• Impact: Maintenance burden only; no runtime risk.

L2. tail -100 output goes to stdout in a CI step

• Issue: The tail -100 codex-complexity.log output is printed to the step's stdout. For very noisy failures, this could make the Actions log hard to read, but 100 lines is a reasonable cap.
• Impact: Cosmetic only.

L3. The set +e / set -e scope is minimal and correct

• Assessment: set +e is issued immediately before codex exec and set -e is restored right after capturing $?. This is the correct pattern — the error-suppression window is as narrow as possible.

Cross-Component Analysis

Component	Impact	Assessment
"Pick thinking budget (Codex)" step (both jobs)	Reads codex-complexity-output.txt	Already has a try/catch that defaults to complex on read failure. The PR's explicit printf 'complex\n' makes the fallback path more deterministic but was already safe.
Downstream Codex review step	Uses effort output from budget step	Unaffected — receives xhigh (complex default) on classifier failure, which is the conservative choice.
Claude provider path	Separate if guard on inputs.provider	Completely unaffected by this change.

Positive Observations

• The fallback to complex (which maps to xhigh reasoning effort) is the conservative/safe default — it allocates more thinking budget rather than less.
• Adding the OPENAI_API_KEY guard to the second block fixes a pre-existing gap where the manual-trigger job would fail hard if the secret was missing.
• The set +e / set -e window is correctly scoped to just the codex exec call.
• Writing codex-complexity-output.txt on all failure paths (including missing API key) provides defense-in-depth even though the downstream JS already handles missing files.

---

ℹ️ About this response

Generated by Claude Code.
Review methodology: claude_md/code_review.md

Limitations:

• Claude may miss context from files not in the diff
• Large PRs may be truncated
• Always apply human judgment to AI suggestions

Commands:

• /claude-review [context] — Request a code review
• /claude-query <question> — Ask about the PR or codebase

[meta-codesync]

@mszeszko-meta has imported this pull request. If you are a Meta employee, you can view this in D104326081.

[meta-codesync]

@mszeszko-meta merged this pull request in 330962b.