Report analysis diagnostics, and fail with a specific comment when the LLM quota runs out - #124
Svilen-Stefanov wants to merge 6 commits into
Conversation
… them A run that exits zero is not a run that finished cleanly. Core records every degradation it survived — a language server that never started, a language nothing indexed under, naming that stopped answering — in the analysis it writes. Until now this action ignored that field, so a baseline missing a whole language was committed green and a review comment showed a diagram with nothing to say it was short. Read metadata.run_diagnostics back and put it where the reader already is: an annotation per entry on the run page, the list in the sync job summary, and the same list at the top of the review comment, above the diagram rather than under it — a caveat printed below a picture is read after the picture is believed. Entries carry their own remedy. Where nothing on the reader's side would have changed the outcome the remedy is empty, and the block links Discord instead of inventing an instruction nobody can follow. Diagnostics never fail the run: a degraded analysis is still worth having, and the point is that its reader learns it is degraded. An analysis written by an engine that predates the field, or one that never got written at all, reads as silence rather than an error. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BSnQMqJKj7zbdfjy7KHF9x
The same title/detail/remedy strings render in the webview, which enforces the house rule with a test, so the prose the action wraps them in should read the same way rather than switching voice between surfaces. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BSnQMqJKj7zbdfjy7KHF9x
CodeBoarding reviewStatus: 1 changed component See the full change in CodeBoarding. graph LR
n_action_scripts["action_scripts"]
classDef added fill:#1f883d,stroke:#0b5d23,color:#ffffff;
classDef modified fill:#bf8700,stroke:#7d4e00,color:#ffffff;
classDef deleted fill:#cf222e,stroke:#82071e,color:#ffffff,stroke-dasharray:5 3;
class n_action_scripts modified;
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c66dd8664
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| # An analysis that finished is not an analysis that finished cleanly. The engine | ||
| # records every degradation it survived; without this the run is green and the | ||
| # committed diagram is short of a language with nothing to say so. | ||
| - name: Read analysis diagnostics |
There was a problem hiding this comment.
feat:
If this commit is merged or rebased with its current subject, Report analysis diagnostics in workflow runs is not a Conventional Commit, so release-please will skip this adopter-facing feature and will not propose the release that moves the v1 tag. Use a feat: subject for the commit and PR title so consumers actually receive the new workflow behavior.
AGENTS.md reference: AGENTS.md:L88-L93
Useful? React with 👍 / 👎.
| for entry in entries: | ||
| level = "warning" if entry.get("severity") == "degraded" else "notice" | ||
| remedy = entry.get("remedy") or f"Nothing on your side causes this; please report it: {DISCORD_URL}" | ||
| lines.append(f"::{level}::{entry.get('title', 'Analysis diagnostic')} {entry.get('detail', '')} {remedy}") |
There was a problem hiding this comment.
When an engine diagnostic contains a newline in its title, detail, or remedy, this emits multiple physical log lines, so only the first is part of the intended annotation and a later line beginning with workflow-command syntax can create a spurious annotation. Multiline language-server or model failure details are a realistic input, and tests/test_llm_contract.py::test_the_annotation_stays_one_line_however_rich_the_remedy already records the repository's one-line requirement; normalize or workflow-command-escape these fields before printing.
Useful? React with 👍 / 👎.
| continue-on-error: true | ||
| shell: bash | ||
| env: | ||
| ANALYSIS_PATH: ${{ steps.review_analyze.outputs.analysis_path }} |
There was a problem hiding this comment.
When the merge-base analysis is degraded but the subsequent head analysis completes cleanly, the rendered review still compares against an incomplete base and can report structures missing from that base as PR additions, yet this step reads only the head analysis path. Because review_analyze produces a separate base_analysis_path, its run diagnostics are not reliably represented by the head's per-run metadata; read and deduplicate diagnostics from both analyses before building the review comment.
Useful? React with 👍 / 👎.
…ebview-errors-x3e0sw # Conflicts: # action.yml
The engine now stops on a mid-run quota refusal (exit 3, kind llm_quota_exhausted) or rejected credentials (exit 2, kind llm_auth) instead of writing a folder-named map. analyze_repository.py keeps that verdict in $RUNNER_TEMP/codeboarding-engine-error.json, exits with the engine's code and annotates the specific reason. A new pair of steps turns the record into the job summary and, in review mode, the review's sticky comment, and the generic "Post review failure" stands down when it exists. Delivery and the base publishes already skip on a failed analysis; tests now pin that. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Svilen-Stefanov
changed the title
`llm: Hosted` passes the credential check, which lower-cases it, but the quota comment compared it raw and gave the own-key remedy instead of the free-tier reset. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Once the paywall stack sits on this, a walled run ends green; an ::error annotation on it would contradict the check. Nothing writes wall.json on main, so no change here. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5d1b13564a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| fi | ||
| # analyze_repository.py records an engine refusal here for the steps that report it; one | ||
| # left by an earlier use of the action in this job is not this run's. | ||
| rm -f "$RUNNER_TEMP/codeboarding-engine-error.json" |
There was a problem hiding this comment.
When a job invokes the action twice and lets the first quota-aborted invocation continue, a second review with an invalid depth_cap exits at the validation above this cleanup. The old codeboarding-engine-error.json is therefore still present, so the subsequent Read engine failure step treats the second analysis failure as a quota exhaustion and replaces the PR's sticky comment with the wrong remediation. Remove the stale record before any early validation can exit.
Useful? React with 👍 / 👎.
2 participants
Summary
Two outcomes of an analysis, reported the way each deserves.
metadata.run_diagnostics, andscripts/analysis_diagnostics.pyturns them into a::warning::per entry, a block in the sync job summary, and a block at the top of the review comment, above the diagram. Diagnostics never fail the run.kind: llm_quota_exhausted) instead of writing a folder-named "deterministic" map. The Action keeps that verdict, the run goes red, and the pull request is told why and what to change. Today the same situation goes green: sync commits the degraded baseline and review posts a normal-looking comment (seen on CodeBoarding/eshop PR 6 and the CodeBoarding/opencode sync).This changes the PR comment people see in two ways: the diagnostics block above the diagram, and the new "stopped: LLM quota used up" comment in place of the diagram (or of the generic "see the workflow logs") when the quota runs out.
What changes
scripts/analyze_repository.py: on a non-zero engine exit, parse the stdout JSON; ifkindisllm_quota_exhaustedorllm_auth, write it (plusexitCode) to$RUNNER_TEMP/codeboarding-engine-error.json, print::error title=CodeBoarding LLM quota exhausted::<engine message>(or... LLM credentials rejected) and exit with the engine's own code. Every other failure keeps the generic "Command failed" path.scripts/action/analyze.sh: clears a stale record at the start.scripts/action/engine_failure.py(new): renders the record into the job summary (both modes) and a comment body (review).action.yml: two new steps,Read engine failure(runs onfailure()when an analyze step failed,continue-on-error) andReport engine failure(sticky comment, sameheaderas the review comment, so it replaces "analyzing…").Post review failuregains&& steps.engine_failure.outputs.reason == ''so it does not overwrite it. No otherif:changed; Deliver baseline and the base/warm-start publishes already skip on a failed analysis, and tests now pin that.build-review-comment.sh's shape withfailure=<kind>in place of the counts:<!-- codeboarding: platform_url=… head=… failure=llm_quota_exhausted -->.origin/main(17 commits). One conflict, theBuild review commentenv block: kept main'sHEAD_SHAandANALYSED_FILES_CHANGEDand this branch'sDIAGNOSTICS_MD.What it looks like
Review mode,
llm: hosted:followed by the hidden line
<!-- codeboarding: platform_url=https://app.codeboarding.org/CodeBoarding/eshop/pull/6 head=abc1234 failure=llm_quota_exhausted -->.On
llm: licensethe second paragraph names the plan's allowance and offers your own key; on a provider key it points at your provider account's quota or a license. Sync writes the same text to the job summary with "The baseline was not updated, and no base analysis was published." Rejected credentials get "stopped: LLM credentials rejected" andfailure=llm_auth.Ordering with the engine
Both halves need CodeBoarding/CodeBoarding#580 in a release and the
codeboarding==pin bumped (the release flow does that; this PR keeps 0.14.4). Until thenrun_diagnosticsis absent and the engine never exits 3, so every run looks exactly as it does today. There is deliberately no scraping of the old engine's fallback log line.Verification
python -m unittest discover -s testsinpython:3.12-slim: 203 tests OK, 7 skipped. New: the script against a stand-in engine exiting 3 / 2 / 1 (record written, exit code kept, annotation text, generic path untouched);analyze.shsync and review with a refusing engine (exit 3, no outputs, nothing staged for delivery or publishing, stale record cleared); the comment copy per tier; step order and conditions inaction.yml.black25.9.0 clean;shellcheck scripts/run_local.shclean;actionlintreports nothing new (only pre-existing findings in.github/workflows/codeboarding.yml).Companion changes
metadata.run_diagnosticsand aborts on quota.CodeBoarding-webviewfix: update CodeBoarding dependency to 0.14.4 #127 banners the diagnostics above the diagram.🤖 Generated with Claude Code