[juhovainio]

Adds pre-review self-check rules to AGENTS.md, distilled from the review history of #715 (4 automated and 2 human review rounds, about 30 blocking findings, mostly repeats of a few patterns).

• §3: mutation-check new tests; write a state matrix before adding a state flag or placeholder; keep cfg(windows) logic unit-testable; one shared constant for cross-file tags.
• §5: grep for stale "not supported" claims across docs, comments and the PR body; write the PR body from the final diff and re-read it after each fix commit.
• §6: subprocess and FFI checklist (kill_on_drop, spawn_blocking, absolute DLL paths, no index pairing, field-width checks, ticker behavior, degrade to unknown).
• §12: rerun the reviewer's and CI's checks locally after each review-fix commit.
• §13: three new checklist items pointing at the above.

Docs only; no behavior change, so no scenario. No links added, so docs-links is unaffected (the local lychee hook skipped, lychee is not installed here).

[nowycondro]

🔴 Automated review · pr-review-watcher · ee7140b

Review — needs work

Full review of the whole change (single-file, 51-line addition to AGENTS.md; no deletions).
Adds pre-review self-check rules distilled from PR #715's review history. The description's claims about this diff (§3/§5/§6/§12/§13 edits, docs-only/no behavior change, no links added) match the diff exactly. PR #715 exists, is open, and has substantial multi-round review activity (13 review submissions from 4 accounts), consistent with the background claim, though exact round/finding counts were not independently re-tallied.

Blocking

• §13 checklist item 12 omits one of the three new §3 self-check rules it's meant to summarize — AGENTS.md:445, AGENTS.md:145-150
  Item 12 reads "New tests mutation-checked; state matrix written for new state or placeholders — §3", naming only the mutation-check and state-matrix rules. The third new §3 paragraph ("Platform-gated code gets a test that runs on its gate...", lines 145-150) has no corresponding checklist line. §13 is this file's actionable pre-push checklist; a contributor working strictly off it is never prompted to apply the platform-gated-test rule.
  Confidence 85 · mechanical · Fix: extend item 12 to also cover platform-gated/cfg(windows) testing, or add a further item citing §3.

• New §6 subprocess/FFI checklist cites two examples that don't correspond to anything in this codebase — AGENTS.md:268-271
Win32_VideoController.AdapterRAM appears nowhere in the repo (git grep -n AdapterRAM origin/main — zero hits; the actual WMI queries in crates/rocm-core/src/host_gpu.rs and neighbors read Name, DriverVersion, DriverDate, PNPDeviceID, AdapterCompatibility, never AdapterRAM). "A ticker that mirrors the daemon['s] MissedTickBehavior" describes a relationship that doesn't exist: the repo's only two tickers (apps/rocmd/src/service.rs:438, crates/rocm-dash-daemon/src/runner.rs:300) use different policies (Delay vs. Skip) independently, with no mirroring pairing between them. This breaks the section's own established convention — every neighboring claim (kill_on_drop, spawn_blocking+timeout, DLL absolute-path loading) is independently verifiable against real code in this repo; these two are not, so a future agent has nothing to check its work against.
  Confidence 85 · logic · Fix: replace AdapterRAM with a real WMI/Win32 field this repo's code actually bounds-checks, and either name a real producer/consumer ticker pair or rephrase without asserting a mirroring relationship that doesn't exist.

Non-blocking

• New §5 bullet substantially overlaps the bullet immediately above it — AGENTS.md:200-204 (existing), AGENTS.md:224-226 (new)
  Both instruct: grep README/docs/doc-comments/help-text for a stale claim, then check every hit against the code. The new bullet narrows to "X is not supported"/"there is no Y" claims and adds "status-bar" and "the PR body" as extra grep surfaces — real additions, but narrow enough not to need a full second paragraph restating the shared mechanism.
  Confidence 70 · mechanical · Fix: fold the new surfaces/scope into the existing bullet as a clause.

• Checklist label "mutation-checked" doesn't appear in the §3 text it summarizes, and "mutation" already means something else in this repo — AGENTS.md:445 vs AGENTS.md:129-134
  The §3 paragraph it points to never uses the word "mutation." Elsewhere in this same file and repo, "mutation" already means state mutation ("inspect state before proposing mutation", AGENTS.md:242; the allow_mutation/--allow-mutation flag throughout apps/rocm/src/chat.rs). The new checklist term risks being read against that established, different sense.
  Confidence 75 · mechanical · Fix: use a term that doesn't collide (e.g. "break-and-confirm-red checked"), or spell out "mutation-testing" explicitly where first used.

• "Fix the class, not the instance" is stated as a fully general review-conduct principle but only appears nested inside the narrow state-matrix paragraph — AGENTS.md:141-143
  "Fixing only the state a reviewer named invites the next round to name its sibling; when a finding is one instance of a class, fix the class and list the sibling cases in the reply" applies to any review finding (a subprocess-leak class, a stale-doc-claim class), not just state-matrix gaps, but has no other home in the file.
  Confidence 70 · mechanical · Fix: consider lifting it to §11 (PR/Review Conduct) or its own line, leaving only the state-matrix-specific instance here.

Decisions for the author

• Embedding a detailed, Windows/subprocess-specific checklist (WMI field widths, tokio ticker policy, kill_on_drop) directly in the root AGENTS.md — tradeoff. Every contributor working on any change reads this file; keeping platform-specific depth inline maximizes discoverability but grows a file already at 462 lines. Neither option is wrong; confirm this is the intended tradeoff rather than factoring Windows-specific detail into a linked sub-document.

Positive signals

• The non-fabricated items in the new §6 checklist — kill_on_drop(true), "a timeout around spawn_blocking drops only the join handle, so the thread still leaks", DLL absolute-path loading, "never pair two independently ordered lists by index; correlate by id" — are all verified-accurate against real, current code in this exact repo (crates/rocm-dash-tui/src/jobs.rs:78; crates/rocm-core/src/host_gpu.rs's existing positional-pairing comment), reading as genuinely incident-derived rather than generic.
• The mutation-testing and state-matrix paragraphs in §3 follow the file's established "concrete wrong way vs. right way" teaching convention well (e.g. contains("0.0%") can be satisfied by unrelated text), with a state list that reads as drawn from a real TUI incident.

Deployment notes

None.

What this covered

Full review of the whole change: the complete diff (51 insertions, 0 deletions, AGENTS.md only) plus the entire pre- and post-PR AGENTS.md (411 and 462 lines) read in full, via three independent fan-out workers — a combined ai-instruction-quality-skill + code pass, a history pass (git log/git blame/git log -S on AGENTS.md), and an impact-analysis/cross-reference/prior-changes/code-comments pass — consolidated and re-verified directly against the live repository (git grep for AdapterRAM, MissedTickBehavior, mutation, kill_on_drop, spawn_blocking). Commits 24e405a7...ee7140bb.

Did not run: the code-comments pass found no new file/function citations in the diff to check against (the new text cites none, unlike the surrounding paragraphs it sits beside). CI was read via statusCheckRollup at face value (all SUCCESS/SKIPPED, nothing failing) and not re-derived from workflow definitions.

Not reconciled against any discussion on this pull request; no reviews existed on it at review time.

[jussielo-amd]

Review — AGENTS.md: add pre-review self-check rules

Two blocking issues, both in the new checklist/guardrail text; six non-blocking consistency notes follow.

Blocking

1. New §12 instruction silently weakens the §8 verification gate (line 425)
§8 requires cargo clippy --workspace --all-targets -- -D warnings plus python scripts/smoke_local.py. The new §12 ("rerun locally the checks the reviewer and CI ran ... clippy with --all-targets") drops -D warnings and the smoke-test invocation. A contributor following §12 literally after a review-fix commit gets a clean local clippy pass despite real warnings, and only discovers the CI gate failure later — the exact round-trip §12 says it's trying to avoid. Suggest either referencing §8 directly ("rerun §8 locally") or restating the full command including -D warnings and smoke_local.py.

2. "Mutation" is overloaded with a conflicting, already-tooled meaning (line 445)
docs/testing.md already defines "mutation" precisely: an unguarded write to shared/global test state, gated by a build-time MUTATIONS scanner and --allow-mutation. The new checklist item 12 ("New tests mutation-checked") uses "mutation" to mean classic break-the-implementation-and-confirm-red testing instead. A contributor familiar with the existing term will misread the checklist item as being about env-var mutation guarding rather than test efficacy, and tick the box without verifying the test actually catches a broken implementation. Suggest a different term for the new concept (e.g. "efficacy-checked" or "red/green verified").

Non-blocking

3. Danger named without a fix, breaking the list's own pattern (line 263)
Every other sub-bullet in the new subprocess/FFI guardrail list pairs a danger with a corrective practice (kill_on_drop(true), "DLLs load by absolute path", "correlate by id", etc.) — except the spawn_blocking/timeout sub-bullet, which states the thread-leak danger with no named alternative. Reads as awareness-only.

4. Six unrelated lessons bundled into one guardrail bullet (line 261)
The new "subprocess and FFI code" guardrail flattens async runtime behavior, DLL loading, list correlation, WMI field width, ticker scheduling, and multi-adapter degradation into a single bullet, breaking the one-concern-per-bullet convention the rest of §6 follows (and unlike the existing pattern of pointing at a narrower doc). Sets precedent for the bullet to keep absorbing unrelated postmortem lessons.

5. Mandatory example list doesn't generalize to its own stated scope (line 136)
The state-matrix example rows ("connecting, ... reconnect, replay end") are phrased for a streaming/telemetry connection, but the rule claims to cover every "new state flag, placeholder, or loading display." A one-shot install-progress placeholder has no reconnect/replay concept — a contributor either force-fits nonsensical rows or skips the rule as inapplicable when it should still apply.

6. Duplicated principle across two adjacent bullets (lines 266 and 272)
"never pair two independently ordered lists by index; correlate by id, or degrade to 'unknown'" and "derived values degrade to 'unknown', never to a guess, on multi-adapter and phantom adapter hosts" teach the same rule for the same scenario. A future edit to the principle risks being applied to one and not the other.

7. New §5 bullet duplicates the existing §5 paragraph above it (line 224)
The pre-existing paragraph already instructs grepping a claim's wording across README/docs/help/CLI output and checking each hit against code. The new bullet restates the same grep-and-verify procedure narrowed to negation-phrased claims, adding only "the PR body" as a new target. Same maintenance-drift risk as #6.

8. Two rules fire on the same trigger without cross-referencing (line 227)
The new §5 bullet (re-read PR body after every review-fix commit) and the new §12 paragraph (self-review against §3–§6 after each review-fix commit) both fire on "after a review-fix commit" but don't reference each other. A later edit narrowing that trigger in one place risks leaving the other silently out of sync.

---

Automated review.

[juhovainio]

Addressed in 8c9d753 (AGENTS.md only):

• Blocking: §12 now says "rerun the §8 gate locally" and spells out clippy ... -D warnings and smoke_local.py.
• Blocking: checklist item 12 no longer says "mutation-checked" (collides with --allow-mutation); it now reads "shown to fail when their target is broken" and also covers platform-gated tests, which the item omitted.
• Blocking: removed the AdapterRAM example (not in this repo) and the "ticker that mirrors the daemon" claim; the ticker bullet now cites the real, differing policies (rocmd Delay, dash daemon Skip).
• Non-blocking: spawn_blocking bullet now names the fix (own deadline/cancellation flag); the two "degrade to unknown" bullets are merged; the state-matrix rule notes that one-shot placeholders list only the states they have.

Left unchanged on purpose: the overlap with the existing §5 grep-the-claim paragraph, lifting "fix the class" into §11, the one-bullet-per-concern split of the FFI list, the §5/§12 cross-reference, and whether Windows/FFI depth belongs inline in AGENTS.md or a linked doc. These are judgment calls for the author.

Addressed in 8c9d753

[nowycondro]

🔴 Automated review · pr-review-watcher · 8c9d753

Review — needs work

Re-review of the current head. Since the last automated review (at ee7140b), a follow-up commit ("address review feedback") landed that resolves both blocking findings from the two prior review rounds (nowycondro's fabricated AdapterRAM/ticker-mirroring claims, and jussielo-amd's §12-weakens-§8 gate and overloaded "mutation" term) — all four verified accurate against the live repo below. One new, narrow issue surfaced: the PR description still describes the pre-fix wording.

Blocking

• PR description still describes the discarded "mutation-check" wording for §3, which the fix commit replaced — PR description vs AGENTS.md:129
  The description's bullet list says "§3: mutation-check new tests," but the current diff's §3 text (line 129) reads "A new test is shown to fail when the code it covers is broken" — the word "mutation" does not appear anywhere in the new §3 paragraphs, precisely because the fix commit removed it in response to a reviewer flagging that "mutation" collides with this repo's existing, differently-scoped allow_mutation/docs/testing.md sense (confirmed: grep -n mutation AGENTS.md now returns only the pre-existing, unrelated line 242). The description is therefore stale relative to the final diff it should describe — ironic given this same PR's new §5 rule ("write the PR body from the final diff... and re-read it after every review-fix commit," line 229) exists specifically to catch this.
  Confidence 75 · mechanical · Fix: reword the §3 bullet in the PR description to match the shipped text (e.g. "tests shown to fail when their target is broken"), per the rule the PR itself just added.

Non-blocking

• New §5 bullet substantially overlaps the existing paragraph immediately above it — AGENTS.md:202 (existing), AGENTS.md:226 (new)
  Both instruct grepping README/docs/doc-comments/help-text for a stale claim and checking every hit against the code. The new bullet narrows to "X is not supported"/"there is no Y" phrasing and adds "status-bar" and "the PR body" as extra grep surfaces — real additions, but narrow enough to fold into the existing bullet as a clause rather than needing a second paragraph restating the shared mechanism.
  Confidence 70 · mechanical · Fix: merge the new surfaces/scope into the existing bullet.

• A general review-conduct principle is stated only inside the narrow state-matrix paragraph — AGENTS.md:143
  "Fixing only the state a reviewer named invites the next round to name its sibling; when a finding is one instance of a class, fix the class and list the sibling cases in the reply" applies to any review finding (a subprocess-leak class, a stale-doc-claim class), not just state-matrix gaps, but has no other home in the file (e.g. §11, PR/Issue/Review Conduct).
  Confidence 70 · mechanical · Fix: consider lifting it to §11, leaving only the state-matrix-specific instance here.

• Two rules fire on the same "after a review-fix commit" trigger without cross-referencing each other — AGENTS.md:229 (§5: re-read the PR body), AGENTS.md:427-429 (§12: rerun the §8 gate and self-review against §3-§6)
  Both are triggered by the same event and sit in different sections with no pointer between them. A later edit narrowing one trigger risks leaving the other silently out of sync.
  Confidence 70 · mechanical · Fix: have one reference the other, or merge into a single "after each review-fix commit" checklist item.

Decisions for the author

• Embedding a detailed, Windows/subprocess-specific checklist (WMI field widths, tokio ticker policy, kill_on_drop) directly in the root AGENTS.md — tradeoff. Every contributor working on any change reads this file; keeping the depth inline maximizes discoverability but keeps growing a file already past 460 lines. Neither option is wrong; confirm this is the intended tradeoff rather than factoring Windows-specific detail into a linked sub-document.

Positive signals

• The fix commit resolved all four prior blocking findings precisely and verifiably: the ticker claim now reads "rocmd uses Delay, the dash daemon Skip," which matches apps/rocmd/src/service.rs:438 and crates/rocm-dash-daemon/src/runner.rs:300 exactly; the fabricated Win32_VideoController.AdapterRAM citation was replaced with a verifiably-true generic claim ("a 32-bit field saturates at 4 GiB"); the §12 gate now spells out the exact §8 commands (cargo clippy --workspace --all-targets -- -D warnings, python scripts/smoke_local.py, both confirmed present/real); and the collision-prone "mutation" term is gone from the checklist item entirely.
• The state-matrix example was generalized well in response to feedback — it now explicitly calls out that a one-shot placeholder has no reconnect/replay rows and should list only the states it has, rather than forcing an inapplicable template.
• The new subprocess/FFI checklist items are independently verifiable against real code in this repo (kill_on_drop(true), the spawn_blocking+timeout thread-leak note, DLL absolute-path loading, id-based list correlation), reading as genuinely incident-derived rather than generic.

Deployment notes

None. Docs-only change (AGENTS.md, 53 insertions, 0 deletions relative to base); no behavior, migration, or dependency impact.

What this covered

Full review of the whole change: the complete diff (24e405a...8c9d753, two commits) read in full, the complete current and pre-PR AGENTS.md read for context, plus the diff isolating the fix commit (ee7140b..8c9d753) to verify exactly which prior findings it addressed. Verified directly against the live repository via git grep/grep for AdapterRAM, MissedTickBehavior, mutation, feature_naming, smoke_local.py, and the section-number cross-references (§3, §5, §6, §8, §11, §12, §13) all resolve to their stated headings. Confirmed PR #715, named in the description as background, exists and is open. CI checked via gh pr checks at the reviewed head: all checks pass or skip (path-filtered), nothing failing.

Given this is a small, single-file, iterative change, this pass was done single-context rather than fanned out to sub-workers; coverage was still complete (every line of the diff and surrounding sections read in full).

Not reconciled against the discussion on this pull request (the two existing review threads) — this review was reached blind from the code and description only, per method. The prior nowycondro review on this PR was at commit ee7140b, which predates the current head (8c9d7536); this is a fresh review of the advanced head, not a continuation of that one.

[tomastola]

🔴 Automated review · pr-review-watcher · 8c9d753

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 at 8c9d7536.
The description says these rules come from the review history of #715, which is still open. The PR names no ticket and closes nothing. Findings the other reviewers already raised, and that 8c9d7536 fixed, are left out.

Blocking

• The new "never pair lists by index" rule contradicts a positional pairing the code ships on purpose, and doesn't say which one wins — AGENTS.md:269-271, crates/rocm-core/src/examine.rs:3144-3170
  The rule says "correlate by id, or degrade to "unknown" when no id exists". fill_unidentified_gpus_from_hipinfo is exactly that no-id case, and its doc comment explains why it pairs by position anyway: "hipInfo … gives no stable handle tying a block to a Win32_VideoController row, so the pairing is positional … That is the only correspondence available, and it is a fallback — the PCI device id and the marketing name both answer first". An agent following the new rule would remove that fallback, and with it the 680M/6800M arch resolution it provides.
  The parenthetical "(a test covers those cases)" names no test, and git grep -i phantom finds no multi-adapter or phantom-adapter test.
  The code should win here: there really is no id, and the stronger identifiers are tried first.
  Confidence 85 · logic · Fix: narrow the rule to the case it was written for (lists that do share an id, or pairing by position as the first resort), or state the exception. If the rule is meant to override the code, say so and change examine.rs in the same PR.

• The branch conflicts with the base in the §13 checklist — AGENTS.md:447-449
  #767 (5880635, now on main) added §13 items 12 ("Force-push's live review/approval state…") and 13 ("Review comments checked for unreplied threads…"). This PR adds its own items 12–14 at the same place. git merge-tree --write-tree against the base exits 1 with CONFLICT (content): Merge conflict in AGENTS.md, and the conflict is exactly those two numbered lists.
  Confidence 100 · mechanical · Fix: rebase, keep #767's items 12–13, and renumber this PR's items to 14–16.

• The kill_on_drop(true) rule covers every spawned child, including ones the code deliberately leaves running — AGENTS.md:264
  "spawned children set kill_on_drop(true)" has no exceptions. Three things in the code show it can't be followed as written:

  • apps/rocm/src/service_launch.rs:220 (detach_background_command) and spawn_detached_no_inherit detach engine children so they outlive the CLI.
  • crates/rocm-dash-tui/src/app/event_loop.rs:466-476 leaves an install/serve child "to the OS rather than reaped via kill_on_drop", so that a child in the middle of a write isn't cut off.
  • Most spawns use std::process::Command, which has no kill_on_drop at all.

  The leak the rule is aimed at is narrower: a tokio::process child awaited under a timeout. crates/rocm-dash-collectors/src/amd_smi.rs:149/161 is exactly that case on main today. That code is pre-existing, so it is not a finding here, but it's worth a follow-up.
  Confidence 85 · logic · Fix: limit the rule to tokio::process children awaited under a timeout, and say that detached children are exempt.

• The ticker bullet cites the dash daemon as its example, but the daemon breaks the bullet's second clause — AGENTS.md:274-275, crates/rocm-dash-daemon/src/runner.rs:469,521,602
  The Delay/Skip facts are correct. But the daemon's tick runs g.metrics(), then d.discover_async(), then l.discover(), one after another, so a slow amd-smi call delays every other source. Yet "a slow source does not block the fast ones sharing its tick" is written as a statement of fact right after citing that daemon. A reader takes it as a description of the cited code, not as an instruction.
  Confidence 80 · logic · Fix: rewrite it as a directive ("do not let a slow source block…"), and either stop citing the daemon for it or note that the daemon doesn't do this yet.

Non-blocking

• The new §5 search list drifts from the one already there — AGENTS.md:226-228 vs :202-206
  Other reviewers already raised the overlap. What's new: the two lists of places to search now differ. The existing one includes "printed CLI output" and the new one leaves it out, so an agent following the new bullet skips it.
  §12's "including feature_naming" adds nothing either: feature_naming.rs is an ordinary test target that the §8 gate's cargo test --workspace --all-targets already runs, and §13 item 6 already requires that gate.
  Confidence 80 · mechanical · Fix: add only the new search places (the PR body and status-bar text) to the existing bullet, and cut §12 down to "rerun the §8 gate".

• Still standing, raised by other reviewers and declined by the author, so not argued again here:

  • The pairing and multi-adapter rules sit under a "subprocess and FFI" heading, though neither is about subprocesses or FFI.
  • The general "fix the class" reply rule sits inside the state-matrix paragraph (§3), where it reads as applying only to state flags.

Decisions for the author

• These are coding-time rules, but the file is framed as a pre-push workflow — tradeoff
  §0 (AGENTS.md:9, :20-25) activates the file "before any upstream PR … push". The state matrix applies "Before writing it", and the cfg(windows) rule asks for a helper or an injectable seam. An agent that loads AGENTS.md only at push time meets both rules after the code exists. Keeping one file is simpler; widening §0's activation line would make these rules apply when they're needed.
• The DLL and WMI-width rules have no code on main to apply to yet — non-blocking-improvement
  No LoadLibrary/libloading calls exist, and no 32-bit WMI memory field is read. They match the still-open #715. Saying which code they were written for would let a reader judge their scope.

Positive signals

• The ticker bullet now gives the real MissedTickBehavior values, and the "scenario number out of sequence" cost refers to a real check (scenario_names_are_indexed_sequentially_per_feature).

Deployment notes

None

What this covered

• Read: every line of AGENTS.md at 8c9d7536, plus the diff from the merge-base 24e405a7. The code each new rule names or implies was checked in apps/, crates/, engines/, tests/e2e-cucumber/ and ci.yml.
• Checks run: a merge against the current base was simulated. The sign-off check ran: 2 commits, none missing a sign-off.
• How it ran: a single pass, one file, with two independent read-only workers.
• CI: green on 8c9d7536.
• Discussion: the review itself read none of the earlier reviews. Afterwards, the discussion was used only to leave out findings that are already fixed or already raised.

[juhovainio]

Addressed in 05ad196 (AGENTS.md only). The branch was rebased onto current main to resolve the §13 conflict, so earlier commit SHAs changed.

Blocking:

• Pairing rule vs fill_unidentified_gpus_from_hipinfo: the rule now says correlate by id where an id is shared, and allows positional pairing only as the last resort after stronger identifiers, citing that function as the example. The unnamed "a test covers those cases" claim is removed.
• §13 conflict: rebased, kept docs(agents): add PR safety checks for force-pushes and review rounds #767's items 12-13, renumbered this PR's items to 14-16.
• kill_on_drop(true): limited to tokio::process children awaited under a timeout; deliberately detached children (detach_background_command) are exempt.
• Ticker bullet: now a directive ("do not let a slow source block..."), and notes that the dash daemon's tick polls its sources serially and does not do this yet.

Non-blocking:

• §5 search lists merged: "status-bar text" and "the PR body" added to the existing claim bullet; the duplicate bullet shortened to "grep the same surfaces".
• §12 cut to "rerun the §8 gate locally".

Left unchanged on purpose (judgment calls for the author): widening the §0 activation line so coding-time rules apply earlier, and stating which code the DLL/WMI-width rules were written for (they track the still-open #715). The heading placement of the pairing rule and the "fix the class" placement were declined earlier and stay as they are.

Addressed in 05ad196

[tomastola]

Re-review at 05ad1961. Compared by tree against 8c9d7536, since the branch was rebased. All four blocking findings from my change request are resolved:

• Positional pairing vs examine.rs: the rule now allows positional pairing as the last resort when no id exists, and cites fill_unidentified_gpus_from_hipinfo as the example. The unnamed "(a test covers those cases)" is gone.
• §13 conflict with #767: the PR's items are renumbered to 14–16 after #767's 12–13, and GitHub reports the PR mergeable.
• kill_on_drop: now limited to a tokio::process child awaited under a timeout, and detached children are exempt.
• Ticker: the slow-source clause is now a directive, and it says the dash daemon doesn't do this yet.

Nothing new blocks.

Non-blocking note: crates/rocm-dash-collectors/src/amd_smi.rs awaits a tokio::process child under timeout without kill_on_drop. That is exactly the case the new rule covers. The code is pre-existing on main, so this is a follow-up, not something for this PR.