docs/fix: console-message audit, two backlog fixes, backlog-numbering cleanup - #404
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe installer now distinguishes skipped and failed Miniconda AllUsers paths. Documentation records updated console output and EXE verification wording. A new CLI audits visible batch messages against the demo output. Backlog and workflow references were updated. ChangesBootstrapper behavior and output records
Console message audit
Backlog documentation and references
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.markdownlint-cli2.jsonc:
- Around line 13-25: Complete the documented sanity sweep before committing:
ensure actionlint and pytest are available and passing, then remove or replace
the block characters reported by the ASCII sweep at lines 970 and 983 of
docs/demo-bootstrapper-output.md. Preserve the existing MD029-clean
configuration and verify the full sweep exits successfully.
In `@docs/agent-ndjson.md`:
- Around line 533-534: Update the comments for the “Check Miniconda
availability” and “Enforce Miniconda availability” steps to reference item 7 in
docs/agent-closed-backlog.md directly instead of CLAUDE.md, while preserving the
existing explanation and workflow behavior.
🪄 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: ebb12b4b-c4e5-4bba-8df3-4631e5c08256
📒 Files selected for processing (9)
.markdownlint-cli2.jsoncAGENTS.mdCLAUDE.mddocs/agent-closed-backlog.mddocs/agent-cold-storage.mddocs/agent-ndjson.mddocs/agent-scratchlog.mddocs/demo-bootstrapper-output.mdtools/run_sanity_sweep.sh
📜 Review details
⏰ Context from checks skipped due to timeout. (8)
- GitHub Check: auto_merge
- GitHub Check: Batch syntax/run check (uv-dl-fallback)
- GitHub Check: Batch syntax/run check (real)
- GitHub Check: Batch syntax/run check (uv)
- GitHub Check: Batch syntax/run check (justme-test)
- GitHub Check: Batch syntax/run check (conda-full)
- GitHub Check: Batch syntax/run check (contract-uv)
- GitHub Check: Batch syntax/run check (contract-uv-fail)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{bat,cmd,ps1,py,md}
📄 CodeRabbit inference engine (AGENTS.md)
Cite
run_setup.batlocations in documentation by stable label or subroutine name rather than exact line number, unless a line number provides immediate value for the introducing commit.
Files:
docs/agent-ndjson.mdAGENTS.mddocs/demo-bootstrapper-output.mddocs/agent-scratchlog.mddocs/agent-cold-storage.md
**/*.{bat,cmd,ps1,py,yml,yaml,json,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Use ASCII-only text; avoid emojis, curly quotes, em-dashes, and other non-ASCII characters because batch/CMD parsing can break.
Files:
docs/agent-ndjson.mdAGENTS.mddocs/demo-bootstrapper-output.mddocs/agent-scratchlog.mddocs/agent-cold-storage.md
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Run the full sanity sweep before every commit, including compilation, linting, delimiter checks, workflow validation, ASCII checks, PowerShell parsing, and pytest.
Files:
docs/agent-ndjson.mdAGENTS.mdtools/run_sanity_sweep.shdocs/demo-bootstrapper-output.mddocs/agent-scratchlog.mddocs/agent-cold-storage.md
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: mixmansoundude/Python_vs_Windows
Timestamp: 2026-07-31T12:06:21.484Z
Learning: Treat GitHub Actions CI results, especially the Windows runner's Job Summary, grouped log tails, and PR failure comment, as the source of truth; local or sandbox runs are advisory.
Learnt from: CR
Repo: mixmansoundude/Python_vs_Windows
Timestamp: 2026-07-31T12:06:21.484Z
Learning: Use the explicit work loop: plan, check the plan, execute, then self-check/tests; make minimal surgical patches, fix root causes, and stop after one change set per loop.
Learnt from: CR
Repo: mixmansoundude/Python_vs_Windows
Timestamp: 2026-07-31T12:06:21.484Z
Learning: Push every commit before it can be lost; after every push, wait for CI, self-heal failures before proceeding, and do not open a PR until the final commit is green and diagnostics outputs have been verified.
Learnt from: CR
Repo: mixmansoundude/Python_vs_Windows
Timestamp: 2026-07-31T12:06:32.966Z
Learning: Do not duplicate authoritative content from README.md, AGENTS.md, or CONTRIBUTING.md; reference those documents instead.
Learnt from: CR
Repo: mixmansoundude/Python_vs_Windows
Timestamp: 2026-07-31T12:06:32.966Z
Learning: Freeze scope during an iteration loop; defer newly discovered requirements to the backlog, fix CI before tests and product code, implement one feature slice, and add one missing test per loop.
🪛 LanguageTool
docs/agent-cold-storage.md
[style] ~51-~51: Consider an alternative for the overused word “exactly”.
Context: ...t manufacture that evidence, which is exactly why this belongs in Cold Storage rather...
(EXACTLY_PRECISELY)
[style] ~71-~71: Consider an alternative for the overused word “exactly”.
Context: ...just a version bump. Condition (b) is exactly what the "Next-pin probe concept" secti...
(EXACTLY_PRECISELY)
[grammar] ~75-~75: Ensure spelling is correct
Context: ...nal reasoning plus a deliberately blunt writeup of why Tier B's narrow, well-justifie...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments (5)
AGENTS.md (1)
243-243: LGTM!tools/run_sanity_sweep.sh (1)
97-101: LGTM!docs/agent-cold-storage.md (1)
1-14: LGTM!Also applies to: 16-27, 29-52, 54-80, 82-116
docs/agent-scratchlog.md (1)
53-55: LGTM!docs/demo-bootstrapper-output.md (1)
1769-1777: LGTM!
The scan previously quoted only the trailing [WARN] REQ-025 log line; the same code path (run_setup.bat's disk-space guard) also echoes three raw *** lines directly to the console before it, which a real user sees in the same run. Add them so the scenario matches what actually prints.
Productionized the ad hoc script used for the bottom-up console-message audit (echo/call:log lines in run_setup.bat vs. docs/demo-bootstrapper- output.md coverage), following the same hand-run, not-wired-into-CI pattern as tools/audit_batch_exit_paths.py, so a future re-review of the demo doc doesn't have to re-derive the extraction/matching logic.
batch-check.yml's Miniconda-availability step comments still pointed at CLAUDE.md for Active Backlog item 7, which PR #403 moved out to docs/agent-closed-backlog.md -- update both references to match the pattern already used in docs/agent-ndjson.md. Also replace the two non-ASCII cursor-block characters in demo-bootstrapper-output.md's postexec-checkpoint quotes with a plain ASCII underscore.
CLAUDE.md Active Backlog item 9: README's REQ-018 bullet said the verification run is "force-stopped after a short interval even if running fine," which is the opposite of the activity-aware-kill behavior that actually shipped (only a completely silent process is force-stopped; any output at all keeps it running as long as needed). Update the bullet to describe the real condition, note the narrower --hidden-import repair-check re-run stays unconditionally time-boxed by design, and move the now-closed item 9 into docs/agent-closed-backlog.md per CLAUDE.md's own directive. No behavior changed -- documentation only.
…sion Active Backlog item 11 (:tci_justme's WARN unconditionally claims "AllUsers install failed" even when AllUsers was only ever skipped, never attempted): :try_conda_install now sets HP_CONDA_ALLUSERS_ATTEMPTED right before the real install attempt, and :tci_justme branches its log line on that flag -- a genuine failure keeps the original WARN wording, a skip gets a new, honest INFO line. Added a regression assertion to tests/selfapps_justme.ps1 confirming the new wording fires and the old one doesn't, in the non-elevated scenario that test already exercises. Updated demo-doc Scenario 19 to match (marked [Extrapolated Branch] pending a fresh CI capture). While closing this out, found item 11 collided with an already-closed, differently-numbered item from 2026-07-25 (still correctly cited by docs/agent-ndjson.md) -- and the same check on item 9 (closed earlier this session) turned up an identical collision, cited by both docs/agent-ndjson.md and docs/agent-interconnect.md. Renumbered both to 16/17 (the next genuinely unused numbers) when archiving them into docs/agent-closed-backlog.md, and filed a new item 18 documenting that the remaining current items (8, 10, 12, 13, 14, 15) show the same number-reuse signature via a grep-based check, without yet doing the individual verification each one needs before it can be safely renumbered too -- left for a dedicated future pass.
aa69d2d to
2b688ff
Compare
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 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 334-336: Update the remaining item 7 backlog reference in the
workflow, including the occurrence near the unconditional matrix-mode steps, to
point to docs/agent-closed-backlog.md instead of CLAUDE.md. Ensure all item 7
references in this workflow use the same canonical document.
In `@docs/agent-closed-backlog.md`:
- Around line 476-518: Resolve the backlog ID collisions described in the closed
entry and its sibling by establishing one stable numbering policy that preserves
each item's original ID. Update the archive entries and all repository
references, including references in run_setup.bat and tests/selfapps_justme.ps1,
so the hidden-import and Miniconda fixes no longer share item 11 and the item 9
collision is removed; search for both “Active Backlog item N” and bare “item N”
references to ensure consistency.
In `@docs/demo-bootstrapper-output.md`:
- Line 1562: Update the terminal failure message in run_setup.bat and the
corresponding documented scenario so it distinguishes the AllUsers-skipped path
from the branch where AllUsers was attempted; when elevation was unavailable and
JustMe fails, report only the JustMe failure, while preserving the existing
both-installations message for genuine AllUsers-attempted failures.
- Around line 1537-1538: Update the label in the documentation text from “Active
Backlog item 16” to “Closed Active Backlog item 16,” while retaining item 11
only in the explanation of the renumbering.
In `@README.md`:
- Line 463: Update the primary EXE verification documented in README.md to
include CI and non-interactive guards in addition to the existing 30-second
silence timeout, ensuring interactive input-waiting programs cannot hang CI
indefinitely while preserving the separate hidden-import re-verification
behavior.
In `@run_setup.bat`:
- Around line 4507-4518: In the AllUsers installation flow around
run_installer_timeout, capture ERRORLEVEL immediately after the call in
HP_CONDA_ALLUSERS_RC before any status check or branching. Update the fallback
warning in the HP_CONDA_ALLUSERS_ATTEMPTED branch to include
exitCode=HP_CONDA_ALLUSERS_RC and reason=installer_failed, while preserving the
existing JustMe retry behavior.
In `@tests/selfapps_justme.ps1`:
- Around line 64-67: The assertions in tests/selfapps_justme.ps1 currently
combine fresh envsmoke output with stale root setup-log content. Update the
$combinedText construction used by $skippedWordingCorrect and
$failedWordingAbsent to rely only on the fresh envsmoke log, removing
$mainSetupText; preserve the existing assertions and pass criteria.
In `@tools/audit_console_messages.py`:
- Around line 85-95: Update the input validation in the CLI flow around bat_path
and doc_path to use is_file() instead of exists(), preserving the existing
error: output and status 2 for non-file paths. Wrap extract_records(bat_path)
and doc_path.read_text(...) in OSError handling so read failures also emit the
existing error format and return 2 without a traceback.
- Around line 43-73: Add a focused test module for the parser functions
normalize, extract_records, and is_covered, covering percent-variable
normalization, echo and call :log extraction, exclusion of test-only commands,
and invalid-path handling. Include regression assertions for the audit contract
and ensure the tests exercise the invalid path behavior without changing
unrelated implementation.
- Around line 43-45: Update normalize to replace environment-variable,
positional (%~dp0-style), and for-variable (%%M-style) expansions while
preserving literal percent text; ensure adjacent expansions are normalized
independently without consuming surrounding content. Add focused tests covering
each expansion form, literal % text, and adjacent expansions.
- Around line 53-64: Update extract_records to strip an optional @ prefix before
parsing commands, normalize echo control tokens before comparing them, make the
call :log pattern case-insensitive, and skip call-log lines containing
redirection. Add focused tests covering `@echo` off, mixed-case ECHO/CALL :LOG,
and redirected log calls.
🪄 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: e80eae81-15f9-4953-9b6e-53f40cf3ce0a
📒 Files selected for processing (8)
.github/workflows/batch-check.ymlCLAUDE.mdREADME.mddocs/agent-closed-backlog.mddocs/demo-bootstrapper-output.mdrun_setup.battests/selfapps_justme.ps1tools/audit_console_messages.py
📜 Review details
⏰ Context from checks skipped due to timeout. (7)
- GitHub Check: Batch syntax/run check (uv)
- GitHub Check: Batch syntax/run check (contract-uv-fail)
- GitHub Check: Batch syntax/run check (justme-test)
- GitHub Check: Batch syntax/run check (real)
- GitHub Check: Batch syntax/run check (uv-dl-fallback)
- GitHub Check: Batch syntax/run check (contract-uv)
- GitHub Check: Batch syntax/run check (conda-full)
🧰 Additional context used
📓 Path-based instructions (14)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Use GitHub Actions CI results, especially Windows runner results, as the source of truth; local or sandbox runs are advisory.
External or environmental failures must be made explicit in logs/NDJSON, including installer exit codes and a reason token, rather than being mistaken for repository regressions.
Enforce conda-forge only: before updates or installs add conda-forge, remove defaults, and install with --override-channels -c conda-forge.
Follow an explicit Plan -> Check the plan -> Execute -> Self-check/tests loop, making minimal surgical changes and one change set per loop.
If code cannot be traced to a requirement, add a comment in the form derived requirement: and propose a requirement.
Push every commit before it can be lost; after every push, poll CI, wait for completion, and self-heal failures before proceeding.
Do not open a PR until the final commit is green and diagnostic outputs have been verified, including setup logs, artifact inventory, and non-decreasing NDJSON row counts.
Keep text ASCII-only, preserve repository line-ending rules, and sanity-check all touched files before submission.
**/*: Do not duplicate content from README.md, AGENTS.md, or CONTRIBUTING.md; reference those authoritative sources instead.
Freeze scope for the current iteration loop; defer new requirements to the backlog, fix CI before test failures and product code, implement one missing feature slice, and add one missing test per loop.
Run the full documented sanity sweep before every commit, extending the ASCII file list to cover files touched by the change.
Files:
tests/selfapps_justme.ps1docs/agent-closed-backlog.mdrun_setup.battools/audit_console_messages.pyCLAUDE.mdREADME.mddocs/demo-bootstrapper-output.md
**/*.{ps1,psm1,psd1}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ps1,psm1,psd1}: Prepend the TLS 1.2 SecurityProtocol assignment to every PowerShell Invoke-WebRequest call and retain -UseBasicParsing.
Validate PowerShell changes with PowerShell AST parsing, install pwsh if necessary, and directly invoke modified scripts with realistic environment variables; do not skip validation on Linux.
Files:
tests/selfapps_justme.ps1
**/*.{bat,cmd,ps1,py,yml,yaml,json}
📄 CodeRabbit inference engine (AGENTS.md)
Run tools/check_delimiters.py and maintain its targeted syntax heuristics; consider adding a zero-false-positive heuristic when a Windows-only bug is discovered.
Files:
tests/selfapps_justme.ps1run_setup.battools/audit_console_messages.py
**/*.{bat,ps1}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{bat,ps1}: Use ASCII-only content; avoid emojis, curly quotes, em-dashes, and other non-ASCII characters because Windows batch and PowerShell parsing can break.
Preserve CRLF line endings for .bat and .ps1 files; rely on .gitattributes rather than manually editing line endings.
Files:
tests/selfapps_justme.ps1run_setup.bat
tests/selfapps_*.ps1
📄 CodeRabbit inference engine (CLAUDE.md)
Use tests/selfapps_.ps1 for new PowerShell scenario tests and wire each new scenario into tests/harness.ps1 and the batch-check.yml job steps.
Files:
tests/selfapps_justme.ps1
docs/agent-*.md
📄 CodeRabbit inference engine (CLAUDE.md)
When a change teaches or invalidates a lesson, update the relevant knowledge document in the same commit, editing existing entries rather than only appending.
Files:
docs/agent-closed-backlog.md
run_setup.bat
📄 CodeRabbit inference engine (AGENTS.md)
run_setup.bat: run_setup.bat must function as a single bootstrapper when dropped next to the app, without committed helper files.
Every branch added to run_setup.bat or related helpers must have a CI test and an NDJSON assertion proving that branch executed; force unreachable branches with a dedicated lane or HP_* flag.
Keep the bootstrapper/CI message contract synchronized: update workflow parsers when bootstrap messages change, and preserve expected phrases when parsers change.
The core flow must remain non-admin; before system-wide installation check elevation with fsutil dirty query %systemdrive% >nul 2>&1, and fall back to per-user installation when not elevated. NI-VISA is optional and warn-only unless pyvisa or visa is imported.
Use tools/sync_payload.py as the only sanctioned method to re-encode and update embedded HP_* payload lines; never hand-roll payload synchronization.
Keep all embedded helper payloads under :define_helper_payloads synchronized with their canonical tools sources, and run the delimiter check after payload changes.
Preserve tilde-prefixed runtime artifact paths and their documented purposes; do not remove the tilde prefixes.
When bootstrapper console text or entry rules change, update the self-test and entry-selection tests; the no-Python exit code is not a release contract, so test the console text instead.
If CI passes on newer Windows, PowerShell, or Python versions than recorded, update the VERSION_METADATA block with the date and verified versions.
run_setup.bat: Keep run_setup.bat self-contained: do not commit helper files that should be embedded; update embedded helpers with tools/sync_payload.py rather than manually encoding or splicing payloads.
Run python tools/check_delimiters.py run_setup.bat after every edit.
Use --override-channels -c conda-forge on every conda installation or package-install command.
Invoke conda through call "%CONDA_BAT%" ... so the parent batch script continues running after conda invocations.
Avoid...
Files:
run_setup.bat
**/*.{bat,cmd}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{bat,cmd}: Use set "VAR=value" rather than set VAR="value"; quote %VAR% at every filesystem command site. The NSIS /D= parameter is the exception and must remain /D=%VAR%.
Avoid EnableDelayedExpansion unless strictly scoped, disabling it afterward; preserve correct escaping and quoting.
Files:
run_setup.bat
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Run python -m compileall -q . and pyflakes as sanity checks, installing pyflakes if needed.
Files:
tools/audit_console_messages.py
tools/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Place new non-trivial standalone helpers in tools/ rather than embedding their logic inline in .yml, .bat, or .ps1 files.
Files:
tools/audit_console_messages.py
CLAUDE.md
📄 CodeRabbit inference engine (AGENTS.md)
CLAUDE.md: Run the advisory MD029-only markdownlint check on CLAUDE.md; new Active Backlog entries must use bullets with the number in prose, not literal ordered-list markers.
Cite run_setup.bat locations by stable label or subroutine name rather than line number; use line numbers only when immediately necessary.When an Active Backlog item is fully resolved, remove it from CLAUDE.md and archive it in docs/agent-closed-backlog.md under Closed Active Backlog Items, preserving its original number.
Files:
CLAUDE.md
.github/workflows/*.yml
📄 CodeRabbit inference engine (AGENTS.md)
.github/workflows/*.yml: Do not change workflow triggers, permissions, or artifact retention; do not weaken, remove, or skip checks or logging/artifacts.
Any new observable log line, disk file, artifact, or behavior must have an NDJSON row and its artifact path must be added to batch-check.yml test-logs upload using both slash-style variants.
Files:
.github/workflows/batch-check.yml
.github/workflows/batch-check.yml
📄 CodeRabbit inference engine (AGENTS.md)
Keep the single inline auto-patcher path using tools/inline_model_fix.py and the gpt-codex-5 model; future model work must extend this path.
Files:
.github/workflows/batch-check.yml
.github/workflows/*.{yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Validate YAML and GitHub Actions workflows with yamllint and actionlint.
Files:
.github/workflows/batch-check.yml
🔇 Additional comments (8)
CLAUDE.md (1)
689-723: LGTM!run_setup.bat (2)
4484-4488: LGTM!
4484-4518: 📐 Maintainability & Code QualityNo delimiter issues found
python tools/check_delimiters.py run_setup.batpasses with exit code 0.> Likely an incorrect or invalid review comment.tests/selfapps_justme.ps1 (1)
60-79: 📐 Maintainability & Code QualityRun the required PowerShell validation.
Parse
tests/selfapps_justme.ps1with PowerShell AST parsing. Runpython tools/check_delimiters.py tests/selfapps_justme.ps1. Execute the script on Linux and Windows with realistic environment variables and fresh logs. Exercise theHP_TEST_NOT_ELEVATED=1path on Windows.As per coding guidelines:
**/*.{ps1,psm1,psd1}requires PowerShell AST parsing and direct execution, and**/*.{bat,cmd,ps1,yml,yaml,json}requirestools/check_delimiters.py.Source: Coding guidelines
docs/demo-bootstrapper-output.md (1)
970-970: LGTM!Also applies to: 983-983, 1776-1784
tools/audit_console_messages.py (3)
32-42: LGTM!Also applies to: 68-83, 107-123
99-105: 🎯 Functional CorrectnessKeep the current classification order. No extracted console record contains a test-only marker inside a
%...%token.> Likely an incorrect or invalid review comment.
1-123: 📐 Maintainability & Code QualityRun the complete sanity checks before merge.
Run
tools/run_sanity_sweep.sh tools/audit_console_messages.pyandpython tools/check_delimiters.py tools/audit_console_messages.pyin an environment withpyflakesavailable. The sweep adds this file to the ASCII check; record missing-tool failures as external setup failures.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
CLAUDE.md (1)
567-567: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winUpdate stale backlog references.
CLAUDE.mdhas no active item 11. The hidden-import exhaustion finding is archived as item 11, and the Miniconda AllUsers finding is archived as item 16. Update the workflow, NDJSON, and hidden-import test comments to reference closed item 11. Update the Miniconda source and test comments to reference item 16. Include these references in Item 18's audit.🤖 Prompt for 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. In `@CLAUDE.md` at line 567, Update stale backlog references in CLAUDE.md: change the workflow, NDJSON, and hidden-import test comments to cite closed item 11; change the Miniconda source and test comments to cite item 16; and include both reference groups in the Item 18 audit.
🤖 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 334-336: Update the comments near the references to
docs/agent-closed-backlog.md and PR `#391` to identify this as closed backlog Item
7, replacing every occurrence of “Active Backlog item 7” including the
additionally noted location; preserve the remaining incident and revert
references.
- Around line 334-336: Remove or replace the constant `if: false` condition in
the workflow step near the comments referencing `docs/agent-closed-backlog.md`
and PR `#391`, preserving the intended `matrix.mode == 'conda-full'` gating
behavior. Rerun both yamllint and actionlint to confirm the workflow validates.
In `@CLAUDE.md`:
- Around line 700-701: Update CLAUDE.md to document that archived items retain
their original numbers unless a collision requires renumbering, explicitly
preserving the mappings 11 → 16 and 9 → 17. Then update the stale item reference
near the :try_conda_install label in run_setup.bat from item 11 to item 16.
In `@run_setup.bat`:
- Around line 4514-4518: Update the :tci_justme fallback branches so each
AllUsers failure or skipped-install log also appends an HP_NDJSON telemetry row
containing the attempted mode and reason. Include both
tests/~test-results.ndjson and tests\~test-results.ndjson in the test-logs
upload paths.
In `@tools/audit_console_messages.py`:
- Around line 88-91: The audit script’s result reporting is incomplete and
disconnected from CI. Update the audit entry point and result-processing flow in
tools/audit_console_messages.py to emit an NDJSON row for every audit result
while retaining human-readable messages only as supplemental output, then invoke
the audit from batch-check.yml and include its artifact path in the Upload test
logs step for both slash variants.
---
Outside diff comments:
In `@CLAUDE.md`:
- Line 567: Update stale backlog references in CLAUDE.md: change the workflow,
NDJSON, and hidden-import test comments to cite closed item 11; change the
Miniconda source and test comments to cite item 16; and include both reference
groups in the Item 18 audit.
🪄 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: 7b5d97d3-1625-4e2a-82a1-308c13c58915
📒 Files selected for processing (8)
.github/workflows/batch-check.ymlCLAUDE.mdREADME.mddocs/agent-closed-backlog.mddocs/demo-bootstrapper-output.mdrun_setup.battests/selfapps_justme.ps1tools/audit_console_messages.py
📜 Review details
⏰ Context from checks skipped due to timeout. (7)
- GitHub Check: Batch syntax/run check (real)
- GitHub Check: Batch syntax/run check (contract-uv)
- GitHub Check: Batch syntax/run check (conda-full)
- GitHub Check: Batch syntax/run check (uv)
- GitHub Check: Batch syntax/run check (justme-test)
- GitHub Check: Batch syntax/run check (contract-uv-fail)
- GitHub Check: Batch syntax/run check (uv-dl-fallback)
🧰 Additional context used
📓 Path-based instructions (17)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
Treat GitHub Actions results, especially Windows runner results, as authoritative; local or sandbox runs are advisory.
**/*: For each iteration loop, freeze scope, fix CI before tests and product code, implement exactly one missing feature slice, and add exactly one missing test.
When a change teaches or invalidates a lesson, update the relevant knowledge document in the same commit; move resolved Active Backlog items todocs/agent-closed-backlog.md.
Files:
tests/selfapps_justme.ps1CLAUDE.mddocs/agent-closed-backlog.mdREADME.mdrun_setup.batdocs/demo-bootstrapper-output.mdtools/audit_console_messages.py
**/*.{bat,cmd,ps1,py,yml,yaml,json}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{bat,cmd,ps1,py,yml,yaml,json}: Every new observable log line, disk file, or assertion-detectable behavior must have an NDJSON row and its artifact path must be included in thebatch-check.ymltest-logs upload using both existing slash variants.
Keep source text ASCII plain text and avoid non-ASCII punctuation.
Run the repository delimiter and syntax-regression checks; for batch files respect caret escaping and comments, and for PowerShell respect comments and here-strings.
Files:
tests/selfapps_justme.ps1run_setup.battools/audit_console_messages.py
**/*.{bat,cmd,ps1,yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Keep bootstrapper messages and CI parsers synchronized: update workflow checks when messages change, and preserve expected phrases when parser logic changes.
Files:
tests/selfapps_justme.ps1run_setup.bat
**/*.{ps1,psm1,psd1}
📄 CodeRabbit inference engine (AGENTS.md)
Prepend the TLS 1.2
SecurityProtocolassignment to every PowerShellInvoke-WebRequestcall and retain-UseBasicParsing.
Files:
tests/selfapps_justme.ps1
**/*.ps1
📄 CodeRabbit inference engine (AGENTS.md)
Validate modified PowerShell scripts with
pwsh; use AST parsing as the default syntax check and run modified scripts directly with realistic environment variables.
Files:
tests/selfapps_justme.ps1
**/*.{bat,cmd,ps1}
📄 CodeRabbit inference engine (CLAUDE.md)
Use ASCII-only characters in Windows batch and PowerShell files; do not add emojis, curly quotes, em-dashes, or other non-ASCII characters.
Files:
tests/selfapps_justme.ps1run_setup.bat
**/*.{bat,ps1}
📄 CodeRabbit inference engine (CLAUDE.md)
Preserve CRLF line endings in
.batand.ps1files; do not manually override the repository’s.gitattributeshandling.
Files:
tests/selfapps_justme.ps1run_setup.bat
tests/selfapps_*.ps1
📄 CodeRabbit inference engine (CLAUDE.md)
Name PowerShell scenario tests
selfapps_<scenario>.ps1.
Files:
tests/selfapps_justme.ps1
tests/**/*.ps1
📄 CodeRabbit inference engine (CLAUDE.md)
Wire new PowerShell scenarios into
tests/harness.ps1so they emit the expected NDJSON rows.
Files:
tests/selfapps_justme.ps1
**/*.{bat,ps1,yml}
📄 CodeRabbit inference engine (CLAUDE.md)
Tag non-obvious constraints with comments in the form
# derived requirement: <why>(or the appropriate shell comment syntax) to prevent regression of subtle fixes.
Files:
tests/selfapps_justme.ps1run_setup.bat
.github/workflows/*.yml
📄 CodeRabbit inference engine (CLAUDE.md)
Validate workflow YAML with both
yamllintandactionlintbefore committing workflow changes.
Files:
.github/workflows/batch-check.yml
CLAUDE.md
📄 CodeRabbit inference engine (AGENTS.md)
Run the narrowly scoped advisory Markdown lint for
CLAUDE.md; new Active Backlog entries must be bullets with the identifier in prose, reserving literal ordered-list syntax for grandfathered item 7.
Files:
CLAUDE.md
**/*.{md,bat,cmd}
📄 CodeRabbit inference engine (AGENTS.md)
Document
run_setup.batlocations using stable label or subroutine names rather than exact line numbers.
Files:
CLAUDE.mddocs/agent-closed-backlog.mdREADME.mdrun_setup.batdocs/demo-bootstrapper-output.md
run_setup.bat
📄 CodeRabbit inference engine (AGENTS.md)
run_setup.bat:run_setup.batmust function as a single bootstrapper when placed beside the application, without committed helper files.
Every branch added torun_setup.bator related helpers must have a CI test, including feature flags, fallback/error-recovery paths, and fast/full paths; use an NDJSON assertion and a dedicated lane orHP_*flag when necessary.
When changing an embedded helper payload, update its matching base64HP_*line from the canonicaltools/source and run delimiter checks.
Preserve tilde-prefixed runtime artifact paths and the documented~env.state.jsonschema; unknown schemas must trigger a stale/full rebuild rather than an error.
Update the[VERSION_METADATA]block when CI verifies a newer Windows, PowerShell, or Python environment.
run_setup.bat: Keeprun_setup.batself-contained: helper implementations must be base64-encoded inside the batch file under:define_helper_payloads; synchronize canonical helper sources withpython tools/sync_payload.pyrather than hand-editing payloads.
Runpython tools/check_delimiters.py run_setup.batafter every edit.
Test all three bootstrap paths—cache, real, and conda-full—after significant changes by checking the corresponding CI lanes.
Every bootstrap run must write~bootstrap.status.jsonwithstateset took,no_python_files, orerror, plusexitCodeandpyFilesfields.
Usepython -m pipreqs.pipreqs, not thepipreqsconsole script, for bootstrap dependency discovery; keep pipreqs pinned to version 0.4.13.
Files:
run_setup.bat
**/*.{bat,cmd}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{bat,cmd}: Useset "VAR=value"for batch assignments; execute filesystem paths as"%VAR%"at filesystem call sites. The NSIS/D=argument is the exception and must remain/D=%VAR%.
Before system-wide installation, silently test elevation withfsutil dirty query %systemdrive% >nul 2>&1; on failure, use the per-user installation path.
AvoidEnableDelayedExpansionunless it is strictly scoped, and disable it afterward; quote and escape batch syntax rather than silencing errors.
Files:
run_setup.bat
**/*.bat
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.bat: Invoke every conda command withcall "%CONDA_BAT%" ...so the parent batch script continues executing.
Use--override-channels -c conda-forgeon all conda installs to prevent defaults-channel contamination.
Do not depend on console scripts during bootstrap; invoke tools through an explicit interpreter path or direct Python module/API invocation.
Anchor every bootstrap tool invocation to an explicit Python executable such as%HP_PY%or%CONDA_PREFIX%\python.exe; never rely on PATH or activation.
Bootstrap must fail fast and explicitly when interpreter, environment, or dependency availability cannot be guaranteed; do not use silent fallbacks unless explicitly logged.
AvoidEnableDelayedExpansion; if required, scope it tightly to prevent collisions with values containing exclamation marks or parent shells using/V:ON.
Prefix temporary runtime files with~, such as~setup.logand~bootstrap.status.json.
Escape or quote CMD special characters such as&,%,^,!, and~; double%insideforloops.
Do not use conda commands without--override-channels -c conda-forge.
Files:
run_setup.bat
tools/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Place new non-trivial helper logic in
tools/rather than embedding it inline in YAML, batch, or PowerShell files.
Files:
tools/audit_console_messages.py
🔇 Additional comments (24)
run_setup.bat (2)
4507-4518: Duplicate: preserve the AllUsers installer result.Line 4508 returns the installer status through
ERRORLEVEL. Line 4509 only tests it, so the fallback cannot distinguish an installer failure from a timeout. CaptureERRORLEVELimmediately after the call and includeexitCodeand a reason token in the warning.This repeats the previous review comment for this code path.
4484-4488: LGTM!tests/selfapps_justme.ps1 (3)
60-63: Update the backlog reference after renumbering.This comment still labels the fix as
Active Backlog item 11. The fix is recorded as closed Item 16 on July 31, 2026. Update the reference.
64-67: Use only the fresh envsmoke log for these assertions.
$combinedTextincludes$mainSetupText. A stale repository-root~setup.logcan change both assertions. Remove$mainSetupTextfrom the assertion input, or clear and identify the root log for this run.
69-79: Run the required PowerShell validation before merge.Validate
tests/selfapps_justme.ps1withpwshAST parsing. Run it with realisticHP_CI_LANEand log inputs. Run the repository delimiter and syntax-regression checks. Also verify that the NDJSON row andbatch-check.ymlupload expose the required fresh-log artifact path.As per coding guidelines, modified PowerShell scripts must be validated with
pwsh, use AST parsing by default, run with realistic environment variables, and pass the repository delimiter and syntax-regression checks.Source: Coding guidelines
docs/demo-bootstrapper-output.md (4)
1536-1538: Use the closed-backlog label.Line 1537 says
Active Backlog item 16, but Item 16 was closed on July 31, 2026. Change this toClosed Active Backlog item 16. Keep item 11 only in historical renumbering text.
1557-1562: Use a skip-aware terminal error message.Lines 1561-1562 show that AllUsers was skipped. The following error must not claim that both AllUsers and JustMe failed. Keep the both-failed message only after an attempted AllUsers failure. Update
run_setup.batand this scenario together.
970-970: LGTM!Also applies to: 983-983
1776-1784: LGTM!docs/agent-closed-backlog.md (3)
478-489: Synchronize all references before claiming the renumber is clean.The archive records this fix as closed Item 16, but
tests/selfapps_justme.ps1still references Active Backlog item 11 anddocs/demo-bootstrapper-output.mdstill uses the Active status. Update those references, then revise this historical note if needed.
491-518: LGTM!
522-549: LGTM!README.md (1)
463-463: Keep primary EXE verification bounded in CI.Line 463 documents an unbounded wait after any output, while Line 466 states that CI and non-interactive runs do not perform untimed runs. The supplied
tools/exe_smokerun.ps1contract does not show a CI or non-interactive guard. Add a bounded CI path, or revise the REQ-018 contract and tests.This repeats the previous review finding. Verify the helper and caller contract with:
#!/bin/bash set -euo pipefail rg -n -C 6 'HP_CI_LANE|HP_NONINTERACTIVE|NOINPUT|sawOutput|WaitForExit|Kill' \ tools/exe_smokerun.ps1 run_setup.bat README.mdtools/audit_console_messages.py (8)
43-45: Fix percent-expansion normalization.
normalize()replaces any text between two percent signs. For example,10% free on %DRIVE%becomes10<V>DRIVE%; the literal percent text is consumed. It also leaves%~dp0and%%Munchanged. The audit can therefore report incorrect coverage results. Recognize environment-variable, positional, andfor-variable forms separately, and preserve literal percent text.
52-64: Alignextract_records()with batch command semantics.The parser misses optional
@prefixes, comparesoffandoncase-sensitively, matches only lowercasecall :log, and does not skip redirectedcall :loglines. These cases produce incorrect audit records. Strip@, normalize control tokens before comparison, use a case-insensitivecall :logpattern, and apply the redirection check to both command forms.
43-73: Add regression coverage for the audit contract.The submitted change contains no regression test for the parser. Add focused tests for percent expansion forms,
@and mixed-case commands, redirected logs, test-only classification, coverage checks, and non-file inputs.As per coding guidelines,
**/*requires implementing one missing feature slice and adding exactly one missing test.Source: Coding guidelines
85-95: Keep invalid paths on the CLI error path.
Path.exists()accepts directories. A directory or unreadable file can reachread_text()and raiseIsADirectoryErroror anotherOSError, so the CLI emits a traceback instead of itserror:message and status 2. Useis_file()for both inputs and catchOSErroraround both reads.
32-40: LGTM!
68-83: LGTM!
97-105: LGTM!
122-123: LGTM!.github/workflows/batch-check.yml (1)
349-351: Complete the Item 7 reference migration.Line [443] still says
See CLAUDE.md Active Backlog item 7, while Lines [334-336] and [349-351] now referencedocs/agent-closed-backlog.md. Update Line [443] to the same canonical closed-backlog reference.CLAUDE.md (2)
536-536: 🗄️ Data Integrity & IntegrationVerify that removed backlog items remain archived.
The change removes Active Backlog items 9 and 11. Confirm that each completed finding has a corresponding record in
docs/agent-closed-backlog.mdand that all references use the final item number. The supplied context does not show replacement records for these two items.As per coding guidelines: when a change teaches or invalidates a lesson, update the relevant knowledge document in the same commit; move resolved Active Backlog items to
docs/agent-closed-backlog.md.Also applies to: 567-567
Source: Coding guidelines
689-723: 📐 Maintainability & Code QualityRun the required
CLAUDE.mdlint.Run the repository's narrowly scoped advisory Markdown lint for
CLAUDE.mdbefore merge. The supplied validation summary does not report this check.As per coding guidelines:
CLAUDE.mdrequires the narrowly scoped advisory Markdown lint.Source: Coding guidelines
…lUsers exit code, audit tool hardening) (#405) * fix: address CodeRabbit review round on PR #404 - batch-check.yml: fix a 3rd stale "CLAUDE.md Active Backlog item 7" reference (line 443) missed in the earlier pass. - run_setup.bat/tests: fix an item-11-vs-16 inconsistency left behind when item 11 was renumbered to 16 -- the source comment and test comment still said "item 11". - run_setup.bat: capture the AllUsers installer's real exit code (HP_CONDA_ALLUSERS_RC) and include exitCode/reason tokens in the WARN line, per this repo's own convention that external/environmental failures must be explicit in logs. Extended the same HP_CONDA_ALLUSERS_ATTEMPTED distinction to :tci_both_failed's terminal :die message, which had the identical "implies AllUsers was attempted when it was only skipped" problem one level up. Updated selfapps_conda_bothfail.ps1's assertion and the demo doc to match. - tests/selfapps_justme.ps1: scope the new skip/failed-wording assertions to the envsmoke-scoped log instead of the combined root+envsmoke text, since a negative ("must NOT appear") assertion is vulnerable to unrelated stale content the positive assertions aren't. - tools/audit_console_messages.py: normalize %~dp0/%~1-style positional params and %%M-style for-loop variables (previously only %VAR% was handled); tighten the %VAR% pattern to require no internal whitespace so an isolated literal '%' earlier in a line can't be greedily paired with a real %VAR% later in the same line; handle leading '@' and case-insensitive echo/call :log; use is_file() + OSError handling instead of a bare exists() check. Added tests/test_audit_console_messages.py (12 cases) covering all of the above -- caught one real edge case in the process (an adjacent %A%%B% sequence with zero separator is genuinely ambiguous with a %%-style for-loop variable; confirmed this shape doesn't occur in real run_setup.bat message text, so scoped the test to the realistic separated-adjacent-expansion case instead). docs/agent-closed-backlog.md's Item 16 entry updated to describe all of the above; CLAUDE.md's new item 18 (backlog-numbering collision) is unaffected by this commit -- still tracking the other six items. * docs: document collision-renumbering exception; closed-Item-7 wording CLAUDE.md: state the renumber-on-collision exception to "item numbers are stable" explicitly, with the 9->17 / 11->16 precedent, per a CodeRabbit review comment noting the exception existed in practice (item 18's own text) but was never written into the policy itself. batch-check.yml: reword the three "Active Backlog item 7" references to "closed Item 7" -- item 7 has been closed since 2026-07-27 and the comments already point at agent-closed-backlog.md, but still called it "Active Backlog," which read as pointing at a still-open item. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A4iE1BRSkUETwz237XeuTW --------- Co-authored-by: Claude <noreply@anthropic.com>
Summary
Bottom-up console-message audit (
docs/demo-bootstrapper-output.mdcoverage check):echo/call :logline fromrun_setup.bat(~597 records) plus everyprint/Write-Hostline from the canonical embedded-helper sources undertools/, andcross-checked each against the demo doc's 38 existing scenarios.
scenario; embedded helpers that could plausibly print user-visible text either route their
output through a file (never hitting the live console, confirmed by tracing each call site's
redirection) or are already represented.
echo ***lines beforethe one
[WARN]line Scenario 24 quoted -- added the missing lines.tools/audit_console_messages.py(hand-run, not wired intoCI, matching the existing
tools/audit_batch_exit_paths.pypattern) so a future doc re-reviewdoesn't have to re-derive the extraction/matching logic.
Backlog item 11 fix (
:tci_justme's misleading WARN): the shared label unconditionallylogged
[WARN] Miniconda AllUsers install failed; retrying with JustMe.even when AllUsers wasonly ever skipped (not elevated), never attempted.
:try_conda_installnow tracks whetherAllUsers was actually launched and
:tci_justmepicks the right wording. Added a regressionassertion to
tests/selfapps_justme.ps1confirming the new INFO wording fires and the old WARNwording doesn't, in the non-elevated scenario that test already runs.
Backlog item 9 fix (README
[REQ-018]bullet was stale): it claimed the verification run is"force-stopped after a short interval even if running fine," the opposite of the shipped
activity-aware-kill behavior (only a silent process gets stopped; any output keeps it running).
Updated the bullet to match reality.
Backlog-numbering cleanup (unplanned, found while closing the two items above): items 9 and
11 both turned out to collide with older, already-closed items that permanently retired those
same numbers back in 2026-07-25 (one collision was cited by two auto-loaded docs,
docs/agent-ndjson.mdanddocs/agent-interconnect.md). Renumbered both to 16/17 when archivingthem into
docs/agent-closed-backlog.md. A quick grep shows the rest of the current batch(items 8, 10, 12, 13, 14, 15) shows the same signature -- filed as a new item 18 for a dedicated
future pass rather than batch-renumbering without individually verifying each one.
Also fixed two CodeRabbit findings from this PR's own review: two non-ASCII cursor-block
characters in the demo doc, and two stale
CLAUDE.md-pointing comments inbatch-check.ymlleft over from the PR #403 docs split.Test plan
python -m compileall -q ./pyflakes-- cleanpython tools/check_delimiters.py run_setup.bat-- cleanyamllint,actionlint-- all clean (installedactionlint/pwshin thissession to validate for real rather than skip)
pwshAST parse sweep over all.ps1files -- cleanpython -m pytest tests/test_*.py -q-- 437 passed, 2 skippedHP_CONDA_ALLUSERS_ATTEMPTEDflag through all three paths into:tci_justmeand confirmed no existing test asserts on the old unconditional WARN textgit diff --statagainst currentorigin/mainreviewed for scopeGenerated by Claude Code