mirror of
https://github.com/alexbelgium/hassio-addons.git
synced 2026-08-23 21:33:32 +02:00
3e561a3f9f
* refactor(skill): shorten hassio-addon-workflow via progressive disclosure SKILL.md was 365 lines, loaded in full on every add-on task. Split it per Anthropic's Agent Skills best practices: keep steps, completion criteria, and the mechanism ladder inline; push rationale, war-story examples, and Codex CLI invocation details into reference files loaded only when that branch is taken. - SKILL.md: 365 -> 171 lines. The "ship the simplest solution" rule was stated three times; now once. Steps 3/6/9 keep their load-bearing checklist but point to detail files instead of inlining it. - references/evidence.md (new): measurement methodology, the host-generalization failure examples, the merged-and-inert case studies - references/codex-review.md (new): CLI invocation, prompt guidance, plan-attack checklist - references/simplify.md (new): mechanism-ladder case studies Also replaces the old "Token efficiency" section (which named this maintainer's personal MCP tools - rtk/headroom/tokensave - not guaranteed present for CI agents or other collaborators using the checked-in copy) with a subagent-delegation instruction: Codex's plan/code review and PR-comment triage on >5 threads should run in a subagent that returns a condensed summary, not raw output, into the calling session. * fix(skill): address PR review feedback - SKILL.md: define $SKILL once and state that all scripts/ and references/ shorthand paths are relative to it — they read as repo-root-relative otherwise and don't resolve from an add-on directory - codex-review.md: drop the reference to a "global CLAUDE.md note" that isn't in this repo's CLAUDE.md; describe --sandbox read-only accurately (reads allowed, writes/exec blocked, approvals disabled) instead of "cannot run anything"; add the missing & so the example is actually backgrounded as the prose claims - evidence.md: use smaps_rollup for process RSS — plain smaps prints one Rss: line per mapping (47 for a trivial process here), not a total - simplify.md: drop the absolute "removals cannot regress" claim, which contradicted the /dev/shm case study in evidence.md * refactor(skill): apply independent quality-review findings From an independent model review of the skill against the Agent Skills best-practice guides: - traps.md: CHANGELOG was described as "the only hard gate", contradicting SKILL.md — the HA add-on linter and the image build also block PRs (verified against onpr_check-pr.yaml). Gates list corrected. - codex-review.md: the "attack your own plan" checklist is run by the main agent, but lived in the file whose stated consumer is the delegated subagent — an agent that delegates correctly would never see it. Moved inline into SKILL.md step 3. - SKILL.md: delegation instruction now says exactly what to do (spawn a subagent whose prompt includes references/codex-review.md and follows its invocation) instead of "delegated per the rule above". - SKILL.md: traps.md is no longer mandatory reading on the light path — the light-path facts it holds (versioning, CHANGELOG format) are inline in step 7; it stays required when touching scripts/Dockerfiles/env. - SKILL.md: dropped the bundled-files table (every row already cited at point of use) and the "read the script when you use it" anti-instruction — scripts are run, not read. 177 -> 167 lines. - description: 982 -> 550 chars; removed workflow narrative that does nothing for skill selection and the stale-prone model name, kept all trigger terms. * fix(skill): note that CI hard gates skip on non-addon PRs The three hard gates in onpr_check-pr.yaml (changelog check, addon-linter, check-build) are each matrixed over check-addon-changes.outputs.changedAddons and if:-skipped when it is '[]'. A PR touching only docs, .github/ or .claude/ therefore shows them as "skipping" rather than passing — which should not be read as a green build. Verified against onpr_check-pr.yaml lines 64, 83, 101 and against this PR's own check output.
55 lines
3.2 KiB
Markdown
55 lines
3.2 KiB
Markdown
# Evidence — measurement methodology and case studies
|
|
|
|
## Why summed RSS and reserved-vs-resident both matter
|
|
|
|
- **Summed RSS double-counts shared pages.** Removing a duplicate process frees its *private*
|
|
memory, not its RSS. `scripts/measure.sh` reports PSS and private alongside RSS — quote
|
|
**private** when arguing "removing this saves N MB".
|
|
- **A big mapping is not necessarily resident.** Large SysV/tmpfs segments are lazily populated;
|
|
reserved size is reported separately from resident for this reason.
|
|
- `/proc/meminfo` and `free` show **host** figures (no memory cgroup namespace here) — never
|
|
attribute those to the add-on.
|
|
- Sample duration matters: a 3 s CPU sample measured 2.3% where a 20 s sample measured 21.6% for
|
|
the same process. Use ≥20 s for anything you report.
|
|
|
|
## Before asserting anything, ask what would show it false
|
|
|
|
- "This process is duplicated" → is it? `ps -ef --forest`, compare parents and start times.
|
|
- "This costs 500 MB" → is it resident? `grep Rss /proc/<pid>/smaps_rollup` — plain `smaps` prints
|
|
one `Rss:` line per mapping (dozens of them), not a process total.
|
|
- "This block never runs" → is its payload in the image? `command -v`, `apt` history.
|
|
- "The flag isn't set" → `tr '\0' '\n' < /proc/<pid>/cmdline`.
|
|
|
|
When you correct yourself mid-analysis, keep the correction visible in your notes and in what you
|
|
report — a retracted claim that stays retracted is worth more than one quietly dropped.
|
|
|
|
## The failure mode this loop keeps producing
|
|
|
|
Every bug shipped from the source session came from one move: **measuring this host correctly,
|
|
then generalising it to all hosts.**
|
|
|
|
- `/dev/shm` was 7.7 GB here, so a flag looked useless — but Home Assistant ignores `shm_size`, so
|
|
elsewhere it is Docker's 64 MB default and removing the flag reintroduces a crash loop.
|
|
- An MCP entry was identified by its URL — but that URL is the documented default, so the rule
|
|
would have deleted a user's hand-written configuration.
|
|
- A GPU probe created a hardware context — but that proved the driver worked, not that Chromium's
|
|
GPU path did.
|
|
|
|
The pattern is always *inference standing in for detection*. Before changing a default, ask what
|
|
this is like on a host unlike yours. Prefer detecting the condition at runtime over asserting it.
|
|
When ownership matters, **record it rather than infer it**.
|
|
|
|
## "Merged and inert" — CI passing proves the build works, not that the change does anything
|
|
|
|
Both changes in the session that produced this skill passed CI, merged, and were **inert**:
|
|
|
|
- The Xvfb resolution cap wrote its env file correctly and Xvfb still started at the base-image
|
|
default — wrong env mechanism for that service.
|
|
- The GPU flags reached Chromium's command line exactly as intended, and the GPU process still
|
|
reported `--use-gl=disabled`, having overridden them after its own init failed.
|
|
|
|
Once the rebuilt add-on is running, re-run the measurement that motivated the work. Some fixes
|
|
cannot be self-verified — a service that reads its environment only at start makes an env-var fix
|
|
unproven until the add-on restarts, which needs the user or `ha-cli` with their agreement. If you
|
|
cannot restart, the change is **Assumed**, not Verified, and must be reported that way.
|