[jussielo-amd]

Why

This delivers ROCMAI-761, part of the rocm-cli modular refactoring epic. Three modules in apps/rocm/src each bundle several unrelated seams: runtimes_cmd.rs mixes the runtimes() dispatch and activation transaction with import and adopt code and the uninstall flow, runtime_install_finalize.rs carries host system-package installation (sudo apt, dnf, zypper) next to engine settling, and driver_install.rs is mostly a per-distro plan catalog plus a digest-pinned WSL rocDXG plan rather than the handler its docs describe. driver_install.rs also reached into runtime_install_finalize.rs only to read /etc/os-release. This gives each seam its own module and removes that reach.

The ticket asks for each step to ship as its own pull request. This pull request carries all seven steps instead, as seven commits in the ticket's order, each one a self-contained move that builds and passes the module tests on its own.

What changes

Every commit is a pure move: code is moved verbatim, and the only edits are re-exports, visibility widening and import paths, each listed in the commit message.

1. system_packages.rs (new, a leaf using only std, anyhow and rocm_core): the OpenMPI, libatomic and libnuma install flows, the print-or-run decision, the apt guard that refuses removals of ROCm packages, the argv runner and real_os_release. runtime_install_finalize.rs re-exports the three ensure_* names so main.rs and engines_cmd.rs keep their imports, and driver_install.rs now imports real_os_release from system_packages and no longer refers to runtime_install_finalize at all.
2. driver_plan.rs (new): the plan model (DriverInstallPlan, DriverPlanCommand, DriverCommandPhase, PrivilegeEscalation), its renderer and the helpers every builder shares.
3. driver_plan/rocdxg.rs (new, a child of driver_plan.rs): the WSL rocDXG plan including ROCDXG_PINNED_DIGESTS, which is a trust anchor; its doc comment and table were compared byte for byte against the old file and are identical.
4. The per-distro builders (build_driver_install_plan_read/_at, the apt, dnf and SLES plans, the ID_LIKE fallback, repository helpers, install_driver_only_reports_on/host_install_driver_only_reports) move into driver_plan.rs. driver_install.rs keeps the handler, the shell runner and the state and reconciliation code, and re-exports host_install_driver_only_reports for chat.rs and freeform_planner.rs.
5. runtime_install_finalize/torch_alignment.rs (new, a child): torch alignment and retention and the manifest readers. It is a child rather than a sibling because it uses the parent's device check (RuntimeDeviceCheck, device_check_verdict), which stays there; as a crate-level sibling the two would form a cycle.
6. runtime_adopt.rs (new): import, adopt and setup-runtime recovery. paths_equivalent and normalize_path_for_compare move to runtime_registry.rs, because adopt and uninstall both need them and leaving them in runtimes_cmd.rs would make a cycle. runtimes_cmd.rs re-exports the names other modules import from it.
7. runtime_uninstall.rs (new): the uninstall plan, confirmation, revalidation and apply code. First, in the same commit, ActiveRuntimeMarker, active_runtime_marker_path and active_runtime_marker_matches move to runtime_registry.rs (uninstall reads the marker; leaving it in runtimes_cmd.rs would be a cycle). The marker is user state on disk: its derive, field names, types and order are unchanged, so its serde shape is unchanged; only field visibility changes.

Path and symbol pins that named the old locations are updated in the same commits: docs/testing.md (including the driver_install test command, which now also selects driver_plan), docs/wsl.md (ROCDXG_PINNED_DIGESTS), the rocm-core openmpi.rs and diagnose.rs comments, and the comment text in tests/e2e-cucumber/tests/e2e/driver_steps.rs and tests/e2e-cucumber/features/examine.feature (no step or scenario changes). docs/architecture.md gets one entry per new file and rewritten entries for the three source files; I checked each claim in them about who depends on whom, and what is pub(crate), pub(super) or private, against the code. A few existing claims that the moves made false or that were already wrong are corrected where they sat: the runtimes_cmd.rs header said comfyui.rs read normalize_path_for_compare and recover_setup_runtime_registration from it (it imports runtime_usability_status), and one driver_install.rs comment said every other optional override lived in that file.

Differences from the ticket text: driver_install.rs re-exports only host_install_driver_only_reports, not also install_driver_only_reports_on, because no consumer outside the moved tests uses the latter and an unused re-export fails clippy. On current main the staying test sdk_install_auto_engine_selection_prefers_vllm_for_supported_families does not use real_os_release, so it needed no import change. Tests that need the activation transaction or list rendering as well as a moved function (a few in runtimes_cmd.rs) and the executor, state and reconciliation tests in driver_install.rs stay with that code rather than moving, so no moved module's tests depend on its caller. No rocm-core system_packages.rs exists on main or in an open pull request, so apps/rocm/src/system_packages.rs uses the name the ticket gives; if rocm-core later gets a module with the same name it lives in another crate.

Verification

Run locally, per commit unless noted:

• Leaf-name test lists (cargo test -p rocm --bin rocm -- --list, and with --ignored, : test lines, module paths stripped, sorted) before and after every commit: the diff is empty each time, and also for the e2e-test-hooks variant at the end.
• Moved tests pass under their new module filters (system_packages, driver_plan, torch_alignment, runtime_adopt, runtime_uninstall).
• cargo fmt --all --check, cargo clippy -p rocm --all-targets -- -D warnings and the same with --features e2e-test-hooks (the torch_runtime_dep_checks_disabled variant and the real_os_release simulated-root test only compile there), cargo build -p rocm --features e2e-test-hooks, cargo build --workspace, cargo xtask manifest --check, cargo xtask check-crate-edges, cargo test -p xtask.
• A full cargo test -p rocm --bin rocm run, with and without e2e-test-hooks: all pass.
• A multiset comparison of every removed and added line across the whole range, with visibility prefixes stripped: apart from imports, mod and re-export lines, module and doc comments, and the two function signatures that rustfmt re-wrapped because pub(crate) or pub(super) lengthened them, no code line differs.
• No new module cycle: the new files' production use crate:: lines reach only runtime_registry, therock, cli_report, engines_cmd (from the torch-alignment child, the same edge its parent already has) and the root helpers; driver_plan and system_packages use no crate module in production code, and driver_plan/rocdxg.rs reads truthy_env from therock.
• Leak scan of the diff against origin/main and a conflict-marker grep: clean.

Not run locally: the full workspace test suite, tests/e2e-cucumber, and the Windows lane. The imports that only a cfg(unix) test uses are gated on cfg(unix) to match, but that is reasoned rather than compiled on Windows. Nothing user-observable changes (no command output, exit code, file or engine selection), so there is no new scenario; the existing e2e scenarios, including the rocm_doctor_skill and driver-install-* ones, are expected to pass unchanged and are left to the e2e lanes.

Scope

A behaviour-preserving split of three apps/rocm modules, nothing else: no behaviour, output, serde shape or trust-anchor value changes. Out of scope, as the ticket lists them: merging the two registry writers, the two infer_python_env_root copies and the two real_os_release copies, and the existing engines_cmd/runtime_install_finalize cycle (this removes driver_install's reach into runtime_install_finalize but does not touch that one).

Open pull requests that edit the same files and will have to re-apply any change to a moved function in its new file: #840, the main.rs command-cluster move (driver_install.rs, runtime_install_finalize.rs), #842 and #829 (driver_install.rs), #832 (runtime_registry.rs, runtimes_cmd.rs), and #811, #610, #541 and #437 (runtimes_cmd.rs). #181 and #778, which the ticket flagged as touching driver_install.rs, are both merged.

[Copilot]

🟡 Changes recommended

The edited source citation in docs/wsl.md remains a bare backtick path rather than a relative Markdown link, leaving the reference unchecked by the docs-links job.

1 open issue

• Use a relative Markdown link for the source-file citation · New

What changed in this PR

This PR advances the CLI modular refactor by separating runtime lifecycle, driver planning, and system-package installation responsibilities, while preserving existing behavior and caller imports.

Changes:

• Extracts focused modules for system packages, rocDXG planning, torch alignment, runtime adoption, and uninstall.
• Moves shared path helpers and active-runtime marker definitions into the runtime registry.
• Updates module documentation, test commands, and comments to reflect the new locations.

File	Description
tests/​e2e-cucumber/​tests/​e2e/​driver_steps.rs	Updates a comment naming the warning test’s location.
tests/​e2e-cucumber/​features/​examine.feature	Updates a comment naming the driver-plan tests.
docs/​wsl.md	Updates the digest trust-anchor source path.
docs/​testing.md	Updates test filters and source references.
docs/​architecture.md	Documents extracted modules and dependency boundaries.
crates/​rocm-core/​src/​openmpi.rs	Updates a comment naming the OS-release reader.
crates/​rocm-core/​src/​diagnose.rs	Updates comments referencing the driver-plan support matrix.
apps/​rocm/​src/​system_packages.rs	Houses system-package installation flows and tests.
apps/​rocm/​src/​runtime_uninstall.rs	Houses uninstall planning, revalidation, application, and tests.
apps/​rocm/​src/​runtime_registry.rs	Receives shared path helpers and active-runtime marker definitions.
apps/​rocm/​src/​runtime_install_finalize/​torch_alignment.rs	Houses torch alignment, retention, manifest readers, and tests.
apps/​rocm/​src/​runtime_adopt.rs	Houses runtime import, adoption, registration recovery, and tests.
apps/​rocm/​src/​main.rs	Declares the new crate-level modules.
apps/​rocm/​src/​freeform_planner.rs	Updates a comment referencing the platform reporting decision.
apps/​rocm/​src/​driver_plan/​rocdxg.rs	Houses WSL driver planning, pinned digests, and tests.
apps/​rocm/​src/​chat.rs	Updates module documentation naming a test consumer.

_{Review effort: Balanced}

[jussielo-amd]

Fixed in 8ee1b79: the ROCDXG_PINNED_DIGESTS citation in docs/wsl.md is now a relative Markdown link to apps/rocm/src/driver_plan/rocdxg.rs, so docs-links checks it. I grepped the other docs for bare citations of driver_install.rs, runtime_install_finalize.rs and runtimes_cmd.rs and found no others outside docs/architecture.md, where every file reference is already a link.

[jussielo-amd]

Correction to my earlier reply: the link I added broke the Sphinx -W build, because docs/wsl.md is {include}d into the docs site and a link to a file outside docs/ cannot resolve there. 2dfcfcd restores the bare path (now apps/rocm/src/driver_plan/rocdxg.rs), which is the form this file can carry. The docs-links job therefore will not check this citation.

[juhovainio]

I compared the moved code against the originals and it looks like a faithful mechanical move: the only differences are imports, mod/re-export lines, doc comments, a couple of rustfmt re-wraps and cfg attributes on moved items. I found no behaviour changes, no dropped cfg gates, the test count is unchanged (132 removed, 132 added), and ROCDXG_PINNED_DIGESTS is byte-identical.

One thing needs fixing before merge: the relative link added in docs/wsl.md breaks the Sphinx -W docs build (details inline). Everything else I checked is fine, though the cfg(unix) import gating for Windows was reasoned from the diff rather than compiled, so I'd lean on the Windows CI jobs, which were still pending when I looked.

[juhovainio]

This relative link fails the Sphinx -W docs build (docs/wsl.md:1077: 'myst' cross-reference target not found: '../apps/rocm/src/driver_plan/rocdxg.rs'), because docs/wsl.md is included in the Sphinx site, where links to files outside it can't be resolved. Please go back to a bare backtick path here, e.g. apps/rocm/src/driver_plan/rocdxg.rs.

[jussielo-amd]

Thanks, you are right: docs/wsl.md is {include}d into the Sphinx site, so a relative link to a file outside docs/ cannot resolve there, and the Sphinx docs build (-W) job failed on it. Reverted to the bare path in 2dfcfcd, so docs/wsl.md now differs from main only in the path (driver_install.rs to driver_plan/rocdxg.rs). I did not check the includes before making the Copilot-suggested change.

fixed

[juhovainio]

Reviewed the refactor against the saved diff. Moved code matches the removed code: the only differing lines are imports, mod/re-export lines, doc comments and two rustfmt re-wraps. Visibility changes are limited to pub(crate)/pub(super) (no bare pub), ActiveRuntimeMarker keeps its serde shape, 132 tests were removed and 132 added with cfg gates balanced, and there is no new module cycle. Docs links and the ticket scope look consistent.

Non-blocking:

• The examine.feature comment now points at the wrong file for real_os_release coverage (inline).
• runtime_install_finalize.rs still has a stale "calls into" module doc; fine to leave per the ticket.
• The cfg(unix) import gating is only proven by the Windows lanes, which were still pending when I reviewed; please make sure they go green before merge.

[juhovainio]

This says the driver plan reads the real /etc/os-release via real_os_release and that its parsing is covered in driver_plan.rs. real_os_release and its test now live in system_packages.rs; driver_plan.rs has the plan-builder tests. Consider naming both so the pointer stays accurate.

[jussielo-amd]

Done in e4911be: the note in examine.feature now names real_os_release in system_packages.rs (with its simulated-root test) and the plan-builder unit tests in driver_plan.rs. I left the module doc in runtime_install_finalize.rs as is, per your note.

[nowycondro]

🤖 Automated review (Claude Code) · 2dfcfcd

This automation never files a GitHub approval, so no approving review will appear here whatever the outcome — the merge decision stays with a human reviewer.

Review — needs work

Full review of the whole change (base 6eeec620…head 2dfcfcdb, the commit this review is pinned to — the PR head moved on twice more while this review was in progress, most recently to e4911be, a small e2e-comment-only follow-up not covered here).
Implements all seven PRs that ROCMAI-761 planned as a sequence (the ticket expected 7 incremental PRs; this one PR carries all seven commits instead, which the PR body discloses). Scope matches the ticket's symbol/destination table exactly; this is also the ticket's final/only PR, so naming the key in the title is correct under the ticket's own "only the final PR names this key" rule.

Blocking

• docs/architecture.md has four stale/self-contradictory cross-references left over from the split — docs/architecture.md:56, docs/architecture.md:34, docs/architecture.md:50, docs/architecture.md:74
  (1) Line 56's runtime_adopt.rs entry says "...recover_setup_runtime_registration registers the runtime rocm setup created, with newest_installed_runtime_manifest and setup_runtime_paths beside it... are pub(crate) for runtimes()... the rest is private" — but newest_installed_runtime_manifest, recover_setup_runtime_registration and setup_runtime_paths are all pub(crate) (confirmed: apps/rocm/src/runtime_adopt.rs:66,380,445), which the same bullet's own earlier sentence requires (they're re-exported from runtimes_cmd.rs). The bullet contradicts itself.
  (2) Line 34's chat.rs entry says ChatRocmCommandAction is "read by driver_install.rs's test module" — that test content moved to driver_plan.rs in this PR (confirmed via git grep: only driver_plan.rs:2779-2818 references it now). chat.rs's own doc comment (apps/rocm/src/chat.rs:10) was correctly updated to say driver_plan.rs; the separate architecture.md bullet about the same fact was not.
  (3) Line 50's freeform_planner.rs entry says plan_driver_request is "read from driver_install.rs's test module" — its only caller is now driver_plan.rs:2764.
  (4) Line 74's therock/fetch.rs entry says truthy_env is "re-exported by therock.rs for driver_install.rs" — the only consumer at the PR head is driver_plan/rocdxg.rs (confirmed via git grep; driver_install.rs no longer calls it); driver_plan.rs's own doc comment correctly attributes this to its child module.
  All four were confirmed directly against the code, not inferred. docs/architecture.md is this repo's authoritative, explicitly-verified module map (AGENTS.md requires keeping docs in sync with behavior/interface changes and grepping a claim's wording across every surface before trusting it); this PR rewrote the three source-file entries and the new-file entries carefully (verified accurate), but didn't propagate the consequence of the driver_install.rs→driver_plan.rs test/call-site relocation to these three unrelated, pre-existing bullets.
  Confidence 90 · mechanical · Fix: update the four bullets to name driver_plan.rs / driver_plan/rocdxg.rs as appropriate, and fix the self-contradiction in the runtime_adopt.rs bullet.

Non-blocking

• Orphaned explanatory comment left in driver_install.rs — apps/rocm/src/driver_install.rs:855-858
  A block comment ("EAI-7406: distro selection must honor /etc/os-release ID_LIKE...") sits at the very end of the file's mod tests, immediately followed by the closing brace — the three tests it describes (driver_plan_ubuntu_derivative_via_id_like_matches_ubuntu_plan, ..._debian_derivative..., ..._almalinux_via_id_like...) moved to driver_plan.rs:3078 onward in this PR without it. Confirmed via git grep EAI-7406 (one hit, in the now-comment-less location) and locating the tests' new home.
  Confidence 90 · mechanical · Fix: move the comment to precede the ID_LIKE tests in driver_plan.rs; delete it from driver_install.rs.

Decisions for the author

• Bundling all seven planned PRs into one — tradeoff
  The ticket's process explicitly expects 7 separate, incrementally-landable PRs (PRs 1-5 can start now. PRs 6-7 wait for Phase 8.1-A PR 2), partly so each is reviewable on its own. This PR deliberately combines them into one (disclosed up front: "The ticket asks for each step to ship as its own pull request. This pull request carries all seven steps instead"). Each commit is independently self-contained and was reviewed as such, but the combined diff (~8,300 lines) is a large single review surface. Not a defect — worth the author/maintainers confirming this was the intended tradeoff rather than a process slip.

Positive signals

• The security-critical ROCDXG_PINNED_DIGESTS trust anchor was independently re-verified byte-for-byte identical between its old (driver_install.rs) and new (driver_plan/rocdxg.rs) locations, not just taken on the commit message's word.
• Across all five reviewed clusters, every pub(crate)/pub(super) visibility widening traced back to a real external caller — no needless over-exposure, and nothing under-widened either.
• ActiveRuntimeMarker's on-disk serde shape (derive, field names, types, order) is byte-for-byte preserved; only field visibility changed, exactly as required for user-persisted state.
• All moved tests (well over 100, across every cluster) diffed byte-identical to their pre-PR bodies, including cfg(unix)/cfg(feature = "e2e-test-hooks") gates, which were correctly re-declared per new file rather than dropped.
• Module doc-comments' specific dependency/visibility/cycle-avoidance claims (e.g. torch_alignment.rs and driver_plan/rocdxg.rs as child modules specifically to avoid a cycle) held up under direct verification against the actual use crate:: lines.

Deployment notes

None.

What this covered

Every one of the 20 changed files read completely (base 6eeec620c3d451a52bd8f98ef218fb9e8a0549a4…head 2dfcfcdb3a59d09ac88817b545760433febefacd), each diffed function-by-function and test-by-test against its pre-PR body, via five independent fan-out workers grouped by coupling (system-packages/torch-alignment; driver-plan model+builders; driver-plan/rocdxg; runtimes_cmd/adopt/uninstall/registry; docs-accuracy), plus my own direct re-verification of every finding reported above and of the docs/wsl.md Sphinx-include question. CI for this exact commit: CodeQL completed (success); the main CI workflow (build/clippy/tests/docs-links/docs-build) was still in_progress and had not concluded at review time — no failure to report, but also no completed-green confirmation for this commit specifically; E2E self-hosted was cancelled (superseded by a later push) rather than run to completion.

Did not run: a real cargo build/cargo test (no Rust toolchain in this environment) — relied on exhaustive diff/grep-based static comparison instead of compiling; "prior changes" pass (no time to survey closed PRs touching these files); "history" (git log/blame) pass was done lightly and surfaced nothing. The PR head moved twice during this review (8ee1b794→2dfcfcdb→e4911be); this review is pinned to 2dfcfcdb, the state actually read. The mandatory pre-review duplicate-review check incidentally surfaced the text of two prior human/bot reviews, including a since-superseded finding about docs/wsl.md's link format — that specific point was independently re-derived here from AGENTS.md's documented Sphinx-{include} rule and the repo's actual docs/rocm-docs/install/wsl.md include directives (not taken from the other review), and was already fixed by the commit this review is pinned to.
Not reconciled against any discussion or review thread on this PR.

---

🤖 Generated with Claude Code

[nowycondro]

🤖 Automated review (Claude Code) · 9fa2739

This automation never files a GitHub approval, so no approving review will appear here whatever the outcome — the merge decision stays with a human reviewer.

Review — no blocking findings

Full review of the whole change (base 87a7c7a7…head 9fa2739).
Implements the full ROCMAI-761 ticket: all seven symbol-move clusters the ticket's scope table lists, delivered as one PR (seven primary commits in the ticket's order, plus four small follow-up commits fixing docs issues an earlier review round raised) instead of the ticket's planned seven incremental PRs. This is the ticket's only/final PR, so naming the key in the title is correct under the ticket's own "only the final PR names this key" rule. Every symbol named in the ticket's scope table was independently confirmed present in its stated destination file, every acceptance criterion (re-exports compile, no new module cycle, leaf-test-count parity, docs/architecture.md coverage, serde-shape preservation) was independently verified against the code rather than assumed.

Blocking

None

Non-blocking

None

Decisions for the author

• Bundling all seven planned PRs into one — tradeoff
  The ticket's process explicitly expects 7 separate, incrementally-landable PRs ("PRs 1-5 can start now. PRs 6-7 wait for Phase 8.1-A PR 2"), partly so each is reviewable on its own. This PR deliberately combines them into one, disclosed up front in the PR body ("The ticket asks for each step to ship as its own pull request. This pull request carries all seven steps instead"). Each commit is independently self-contained and moves a disjoint cluster, but the combined diff (~8,300 lines across 20 files) is a large single review surface. Not a defect — worth the author/maintainers confirming this was the intended tradeoff rather than a process slip.

Positive signals

• The security-critical ROCDXG_PINNED_DIGESTS trust anchor was independently re-verified byte-for-byte identical (diff + extraction, not eyeballed) between its old (driver_install.rs) and new (driver_plan/rocdxg.rs) locations.
• ActiveRuntimeMarker's on-disk serde shape (derive list, all field names, types, order) is byte-for-byte preserved between runtimes_cmd.rs (old) and runtime_registry.rs (new); only field visibility changed, exactly what user-persisted state requires.
• Total #[test] count across the three original files (196) exactly matches the total across all nine resulting files (196) — independently counted, not taken from the PR body's own count — confirming no test was silently dropped or duplicated anywhere in the three-way, nine-file split.
• Every pub(crate)/pub(super) visibility widening checked (dozens, across all five file clusters) traced back to a real external caller found via git grep; no bare pub was introduced anywhere, and nothing was under-widened either.
• docs/architecture.md's entries for all six new files and the rewritten entries for the three source files were checked claim-by-claim against the actual code (dependency direction, pub(crate)/pub(super)/private status, re-export lists) and held up in every case checked, including the two stale/self-contradictory bullets and the wrong-file comment an earlier review round had flagged — both are now fixed by this commit (9fa2739, ecbdec2) and independently reconfirmed fixed here rather than taken on that round's word.
• A corrected doc comment ("optional override in this file" → "optional override of the driver plan") precisely tracks the fact that the two functions it refers to no longer live in the same file after the split — the kind of accuracy detail a mechanical move usually lets drift.

Deployment notes

None.

What this covered

Every one of the 20 changed files read completely in both pre-PR and post-PR state (base 87a7c7a76188edb0af904020a0a58942910ad27d…head 9fa2739181300ebfcf9e8e75464edf7db2f2c812), each production function, struct, enum and test diffed individually against its pre-PR body. Four independent fan-out workers covered: (1) system_packages.rs + runtime_install_finalize.rs + runtime_install_finalize/torch_alignment.rs + openmpi.rs; (2) driver_plan.rs + driver_plan/rocdxg.rs; (3) the shrunk driver_install.rs + the four one-line comment edits in chat.rs/freeform_planner.rs/diagnose.rs + main.rs's new mod lines; (4) runtimes_cmd.rs + runtime_registry.rs. A fifth cluster (runtime_adopt.rs + runtime_uninstall.rs) could not get an independent worker spawned after repeated transient capacity failures in this environment, so I reviewed it myself directly with the same methodology (extracted and diffed every function/struct/test against the pre-PR runtimes_cmd.rs, confirmed re-exports and callers via git grep) — flagged here as not an independent pass. I also did my own docs-accuracy pass (docs/architecture.md, docs/testing.md, docs/wsl.md, examine.feature, driver_steps.rs) and retrieved ROCMAI-761 directly from Jira (not via any link in the PR body) to check the scope table and acceptance criteria.

CI for this exact commit: all 26 checks completed, every one SUCCESS except Skill checks (skillscope), correctly SKIPPED (no skill files touched) — full green, including both Windows jobs, clippy, cargo fmt, the ratcheted coverage check, Sphinx docs build (-W), Markdown links resolve (lychee), E2E tests and E2E consolidated report.

Did not run: a real cargo build/cargo test (no Rust toolchain in this environment) — relied on exhaustive diff/grep-based static comparison plus the green CI above as compile/test confirmation; a "prior changes" pass across the other open PRs the body names as touching these same files (out of scope for this review, and already disclosed by the author); "history" (git log/blame) was done lightly — confirmed 3 commits landed on main ahead of this PR's base, none overlapping the regions this PR edits.

The mandatory pre-review duplicate-check (required to avoid double-posting) necessarily surfaced the text of a prior nowycondro review at an older, superseded commit (2dfcfcdb) plus two human reviews. Both of that prior review's findings (stale/self-contradictory docs/architecture.md cross-references; a wrong-file reference in examine.feature) were independently re-confirmed fixed by this review's own direct reading of the current commits, not taken on the earlier review's word.

Not reconciled against any discussion or review thread on this PR.

---

🤖 Generated with Claude Code