ci: relocate conda_avail (unwired) + repo-wide ASCII cleanup - #394
Conversation
Replaces cosmetic Unicode characters (checkmark/cross/warning glyphs, em dashes, bullets, middle dots, arrows) with meaning-preserving ASCII in CI log/summary text and promo SVG assets, so a repo-wide ASCII sweep reports clean, not just the files CLAUDE.md's own sanity sweep currently checks. Files: .github/workflows/batch-check.yml, .github/workflows/pr-automerge.yml, tools/diag/publish_index.py, tools/run_prechecks.ps1, and 6 promo/*.svg logo assets (title/comment text only, no visual change). One real bug caught while verifying: replacing the em dash inside the promo SVGs' <!-- ... --> header comments with "--" produced invalid XML, since XML comments forbid "--" anywhere in their content, not just at the delimiters. Fixed by using a single hyphen there instead; verified all 6 SVGs still parse as valid XML afterward. The 4 promo/*.png files are binary image data and were never actually non-ASCII text -- they only matched the initial repo-wide grep because raw pixel bytes happen to include bytes >0x7F, which is meaningless for a binary format. Left untouched. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015xbWLPbiaKVsobB9FZy8kS
…red) Step 1 of 2 (owner-authorized: "if confident then implement"). Moves the existing, currently-unused conda_avail diagnostic step from right after the empty-repo bootstrap (its old, premature position -- the root cause of the item-7/PR-390 circular-skip bug) to right after selfapps_envsmoke.ps1, the traced genuine first step that performs a real run_setup.bat bootstrap under HP_FORCE_CONDA_ONLY=1. Deliberately still unwired -- zero if: conditions reference it in this commit. This mechanism has already produced two real bugs in quick succession; landing the corrected position alone first, with nothing depending on it, lets the next conda-full CI run prove `available` correctly flips to 'true' before any gating logic returns. Found and fixed a real YAML syntax error while validating locally (actionlint/yamllint): the new step name contained a literal "if:" inside unquoted text, which YAML parsed as a nested mapping key -- quoted the name to fix. Also documents a wrinkle found mid-implementation: envsmoke itself was one of the 9 "every lane" steps gated in the ORIGINAL design, but it's now the producer of the signal, not a consumer, so it (and the other 8 steps in that category) must stay unconditional going forward -- only the 27 already-enumerated real/conda-full-restricted steps are ever candidates for the follow-up wiring commit. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015xbWLPbiaKVsobB9FZy8kS
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📜 Recent review details⏰ Context from checks skipped due to timeout. (8)
🧰 Additional context used📓 Path-based instructions (10)**/*📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{md,bat,cmd}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{yml,yaml,bat,ps1,py}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{bat,cmd,ps1,py,yml,yaml,json}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{py,bat,ps1,yml,yaml}📄 CodeRabbit inference engine (AGENTS.md)
Files:
tools/**/*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{py,bat,ps1,yml,yaml,json}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
.github/workflows/*.yml📄 CodeRabbit inference engine (AGENTS.md)
Files:
.github/workflows/**/*.{yml,yaml}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (1)📓 Common learnings🪛 LanguageTooldocs/agent-ndjson.md[uncategorized] ~532-~532: The official name of this software platform is spelled with a capital “H”. (GITHUB) 🔇 Additional comments (6)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe pull request moves the Miniconda diagnostic after the real environment smoke test, keeps it unwired, documents its NDJSON output, updates CI and diagnostic report formatting, changes precheck status markers, and standardizes auto-merge success log markers. ChangesSelf-test diagnostics and output
Auto-merge status logging
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/batch-check.yml:
- Around line 1997-2006: Update the self-test summary totals alongside the row
formatting that emits `[-]` for non-boolean r.pass values, ensuring those
unknown rows are represented in the totals. Add an UNKNOWN count or apply the
same classification used by the later NDJSON gate, and include it in the summary
output without changing PASS or FAIL counting.
- Around line 325-364: Update the conda_avail step to append a non-gating NDJSON
result row containing the computed available value, using the existing
test-results output convention. Extend the “Upload test logs” artifact list with
both tests/~test-results.ndjson and tests\~test-results.ndjson variants.
In `@tools/diag/publish_index.py`:
- Around line 2063-2065: Update the preview formatting expression in the
surrounding publish index logic to use an f-string instead of the positional
.format call, interpolating info.filename, info.compress_size, and
info.file_size directly while preserving the existing output text and byte
formatting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9cede674-e148-42c6-839a-d8fb0f219c9d
⛔ Files ignored due to path filters (6)
promo/avc-icon-dark-base.svgis excluded by!**/*.svgpromo/avc-icon-dark-contrast.svgis excluded by!**/*.svgpromo/avc-icon-purplebg-whitecouch.svgis excluded by!**/*.svgpromo/avc_horizontal.svgis excluded by!**/*.svgpromo/avc_icon.svgis excluded by!**/*.svgpromo/avc_stacked.svgis excluded by!**/*.svg
📒 Files selected for processing (5)
.github/workflows/batch-check.yml.github/workflows/pr-automerge.ymlCLAUDE.mdtools/diag/publish_index.pytools/run_prechecks.ps1
📜 Review details
⏰ Context from checks skipped due to timeout. (8)
- GitHub Check: auto_merge
- GitHub Check: Batch syntax/run check (contract-uv-fail)
- GitHub Check: Batch syntax/run check (contract-uv)
- GitHub Check: Batch syntax/run check (conda-full)
- GitHub Check: Batch syntax/run check (uv-dl-fallback)
- GitHub Check: Batch syntax/run check (uv)
- GitHub Check: Batch syntax/run check (justme-test)
- GitHub Check: Batch syntax/run check (real)
🧰 Additional context used
📓 Path-based instructions (12)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Use GitHub Actions results, especially Windows runner behavior, as the authoritative source of truth; treat local or sandbox runs as advisory.
Do not weaken tests, remove logging or artifacts, or delete or skip checks to obtain a green build.
Any new observable log line, disk file, or assertion-detectable behavior must produce an NDJSON row and have its artifact path added to thebatch-check.ymltest-logs upload using both existing slash-style variants.
Follow an explicit Plan -> Check the plan -> Execute -> Self-check/tests loop, and make one change set per loop.
Push every commit before it can be lost; do not leave commits unpushed when a turn ends.
Do not implement scope changes without first creating a backlog item unless explicitly instructed; fix CI failures before proceeding, and add a derived-requirement comment for untraceable code.
**/*: Do not duplicate authoritative content from README.md, AGENTS.md, or CONTRIBUTING.md; reference those sources instead.
Implement exactly one missing feature slice and add exactly one missing test per iteration loop; freeze scope and defer new requirements to the backlog.
Run the full mandatory sanity sweep before every commit, extending the ASCII file list to cover all files touched by the change.
Files:
tools/run_prechecks.ps1CLAUDE.mdtools/diag/publish_index.py
**/*.{yml,yaml,bat,ps1,py}
📄 CodeRabbit inference engine (AGENTS.md)
Enforce conda-forge only: before updates or installs add conda-forge to the environment channels, remove defaults if present, and install with
--override-channels -c conda-forge.
Files:
tools/run_prechecks.ps1tools/diag/publish_index.py
**/*.ps1
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ps1: Prepend TLS 1.2 configuration to every PowerShell 5.1Invoke-WebRequestcall and retain-UseBasicParsing.
Validate touched PowerShell files with PowerShell AST parsing; do not skip validation because the host is Linux, and directly sanity-check modified scripts after installingpwsh.
Files:
tools/run_prechecks.ps1
**/*.{bat,cmd,ps1,py,yml,yaml,json}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{bat,cmd,ps1,py,yml,yaml,json}: Keep source text ASCII-only and avoid non-ASCII punctuation.
Run the repository delimiter and syntax checks where applicable, includingtools/check_delimiters.pyfor supported file types.
Files:
tools/run_prechecks.ps1tools/diag/publish_index.py
**/*.{py,bat,ps1,yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{py,bat,ps1,yml,yaml}: Make minimal, surgical patches in failing areas, especiallyrun_tests.bat,tests/harness.ps1, and files undertests/**.
Use professional comments to explain why non-obvious bug fixes or structural constraints exist.
Files:
tools/run_prechecks.ps1tools/diag/publish_index.py
**/*.{bat,cmd,ps1,py,yml,yaml,json,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Use ASCII-only content; avoid emojis, curly quotes, em-dashes, and other non-ASCII characters.
Files:
tools/run_prechecks.ps1CLAUDE.mdtools/diag/publish_index.py
**/*.{bat,cmd,ps1}
📄 CodeRabbit inference engine (CLAUDE.md)
Preserve CRLF line endings for batch and PowerShell files; do not manually override the repository’s .gitattributes configuration.
Files:
tools/run_prechecks.ps1
.github/workflows/*.yml
📄 CodeRabbit inference engine (AGENTS.md)
.github/workflows/*.yml: Do not change GitHub Actions workflow triggers, permissions, or retention settings.
Isolate slow, flaky, or environment-dependent diagnostics in their own non-gating (continue-on-error) lane instead of slowing deterministic gating lanes.
Keep the CI interface synchronized with bootstrapper messages and status summaries; update workflow parsers whenever emitted text changes, and vice versa.
Use only the single inline model path inbatch-check.ymlfor future model work; do not add older online-model or extra inline experiments.
Files:
.github/workflows/pr-automerge.yml.github/workflows/batch-check.yml
.github/workflows/**/*.{yml,yaml}
📄 CodeRabbit inference engine (CLAUDE.md)
Validate modified workflow files with yamllint and actionlint.
Files:
.github/workflows/pr-automerge.yml.github/workflows/batch-check.yml
**/*.{md,bat,cmd}
📄 CodeRabbit inference engine (AGENTS.md)
Reference
run_setup.batlocations in documentation by stable label or subroutine name rather than exact line numbers.
Files:
CLAUDE.md
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Sanity-check Python changes with
python -m compileall -q .andpython -m pyflakes .when applicable.
Files:
tools/diag/publish_index.py
tools/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Place non-trivial reusable helpers in tools/ rather than embedding substantial logic inline in YAML, batch, or PowerShell files.
Files:
tools/diag/publish_index.py
🪛 Ruff (0.15.21)
tools/diag/publish_index.py
[warning] 2063-2065: Use implicit references for positional format fields
Remove explicit positional indices
(UP030)
[warning] 2063-2065: Use f-string instead of format call
Convert to f-string
(UP032)
🪛 zizmor (1.26.1)
.github/workflows/batch-check.yml
[warning] 2698-2698: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🔇 Additional comments (9)
.github/workflows/pr-automerge.yml (1)
110-110: LGTM!Also applies to: 122-122, 134-134, 175-175, 187-187
.github/workflows/batch-check.yml (3)
1973-1973: LGTM!
2055-2055: LGTM!
2698-2698: LGTM!CLAUDE.md (1)
815-835: LGTM!tools/diag/publish_index.py (3)
1624-1628: LGTM!
3770-3779: LGTM!
4189-4207: LGTM!tools/run_prechecks.ps1 (1)
66-69: LGTM!
- Add a non-gating NDJSON row (diag.conda.available) for the relocated conda_avail step's already-observable Write-Host/output, which had no NDJSON coverage since it was introduced in PR #390. Purely additive -- pass is always true, no if: condition touches it, consistent with this PR's own "zero wiring yet" scope. Skipped CodeRabbit's suggested artifact-path addition (tests/~test-results.ndjson to "Upload test logs"): that file is already captured by the dedicated "Upload CI NDJSON" step earlier in the same job, so no new path is needed. - Include unknown-pass rows (rendered as [-] in the self-test summary list) in the totals line via a new UNKNOWN count, so a row that isn't a boolean pass/fail no longer silently vanishes from the totals while still appearing in the bullet list. - publish_index.py: replace a positional .format() call with an f-string per Ruff UP030/UP032. Registered diag.conda.available in docs/agent-ndjson.md per the AGENT DIRECTIVE (registry entry + Key-facts explanatory note). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015xbWLPbiaKVsobB9FZy8kS
…on (#395) CodeRabbit re-review on PR #394 caught a real, pre-existing gap (not introduced by this branch): "Upload test logs" listed only the forward-slash tests/~test-results.ndjson, missing its required backslash sibling tests\~test-results.ndjson -- every other entry in that path list carries both spellings per this repo's own convention (AGENTS.md: "any new observable ... artifact path added ... using both existing slash-style variants"). Claude-Session: https://claude.ai/code/session_015xbWLPbiaKVsobB9FZy8kS Co-authored-by: Claude <noreply@anthropic.com>
Downloaded the diag.conda.available NDJSON row from PR #395's merge run and confirmed available=true, verifying the staged conda_avail relocation (PR #394) works as designed. Read-only verification only; step 2 (wiring the 27 if: conditions) still needs its own separate owner sign-off per the existing reasoning. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015xbWLPbiaKVsobB9FZy8kS
) Downloaded the diag.conda.available NDJSON row from PR #395's merge run and confirmed available=true, verifying the staged conda_avail relocation (PR #394) works as designed. Read-only verification only; step 2 (wiring the 27 if: conditions) still needs its own separate owner sign-off per the existing reasoning. Claude-Session: https://claude.ai/code/session_015xbWLPbiaKVsobB9FZy8kS Co-authored-by: Claude <noreply@anthropic.com>
Summary
Two independent, low-risk commits, both owner-authorized ("if confident then implement or register open question. Remove all non ascii while you are at it so ascii sweep repo wide reports clean too").
400574fchore: repo-wide ASCII cleanup. Replaced non-ASCII glyphs (em/en dashes, arrows, checkmark/cross/warning emoji, bullets) with ASCII equivalents across.github/workflows/batch-check.yml,.github/workflows/pr-automerge.yml,tools/diag/publish_index.py,tools/run_prechecks.ps1, and 6promo/*.svgfiles. Found and fixed a real bug along the way:--is illegal inside XML<!-- ... -->comment content (not just at the delimiters) -- 3 SVGs briefly broke XML parsing until fixed to use a single hyphen there instead; all 6 SVGs re-verified viaxml.dom.minidom.parse(). 4 promo PNGs were confirmed (viafile) to be genuine binary image data, not text with false-positive matches, and were left untouched.tools/run_sanity_sweep.sh's repo-wide ASCII sweep now reports clean.7610ad6ci: move conda_avail to right after the real first-install step (unwired). Step 1 of the staged conda_avail re-wiring plan from CLAUDE.md's Active Backlog: the existingconda_availdiagnostic step (currently unused after PR ci: fix conda_avail circular gate silently skipping conda-full self-tests #391's revert) is relocated from right after "Bootstrap environment" (which never touches conda -- it runs against this repo's own empty root) to right afterselfapps_envsmoke.ps1("Self-test: real env smoke (CI-only)"), the genuine first selfapps step that performs a real, unconditionalrun_setup.batbootstrap underHP_FORCE_CONDA_ONLY=1. Deliberately left with zeroif:conditions referencing it in this commit -- this mechanism has already produced two real, independently-discovered bugs in quick succession (the original premature-gate design in ci: lane-aware if:always() rollout for remaining 36 gating-lane steps (item 7) #390, a CodeRabbit-caught wording slip on docs/tools: address PR #391 CodeRabbit findings; add advisory MD029 markdownlint check #392), so this lands the corrected position on its own first, verifiable via the step's own log output on the nextconda-fullrun, before any gating logic is reintroduced in a follow-up PR.CLAUDE.md updated in the same commit with the full trace and staging rationale.
Test plan
python -m compileall -q .python -m pyflakes .python tools/check_delimiters.py run_setup.batmarkdownlint-cli2 CLAUDE.md(clean, only the expected permanent baseline finding)python -m yamllint .github/workflows/actionlint -oneline .github/workflows/*.ymlpython -m pytest tests/test_*.py -q(437 passed, 2 skipped)xml.dom.minidom.parse()tools/run_sanity_sweep.shrun, all checks OKClaude-Session: https://claude.ai/code/session_015xbWLPbiaKVsobB9FZy8kS
Generated by Claude Code