firehose/doc/improve-verification.md

# Improve verification for firehose

Plan for bringing the "internal code health" sensor stack to firehose, based on
what worked in stickies (Oct 2026) and
[Velasco's ExCrap blog post](https://www.germanvelasco.com/blog/i-wrote-excrap-without-looking-at-the-code).

Two kinds of verification (the post's frame):

1. **External** — is the product what we wanted? Visible: the site, the blogs.
   Firehose already has decent coverage here: Playwright suites for the app
   (`make playwright*`), controller/LiveView tests.
2. **Internal** — is the codebase healthy and evolving well? Invisible unless
   we install **sensors and signals**: coverage, CRAP scores, credo, sobelow,
   mutation testing, boundary checks. This plan is about the sensors.

## Where firehose differs from stickies

- **Two OTP apps.** `app/` (Phoenix, the Firehose site) and `blogex/`
  (publishable library, NimblePublisher-based blog engine, no DB). Every sensor
  must run per-app; the root `Makefile` orchestrates (`make test` already runs
  both).
- **blogex is a library** (has `package`/`docs` config). Quality bar is
  higher; it's also mostly *pure* code (post parsing, link validation, feeds,
  SEO) — the ideal mutation-testing target, like stickies' `lib/cardgame/domain`.
- **blogex has no mix aliases at all** (no precommit, no format in aliases).
  `app` has a `precommit` alias (compile --warnings-as-errors, deps.unlock,
  format, test).
- **A gitea remote already exists** — unlike stickies at the start, a pre-push
  hook will actually fire on every push from day one.
- Already present: credo dep in `app` (default config, not tuned/strict);
  nothing else yet. No coverage config, no boundary/ex_crap/sobelow/muex in
  either app. Current suites green: app 178 tests, blogex 156.
- `AGENTS.md` has a **zero-defects policy** ("never continue when there are
  errors") — the fast/push split must be written to fit that policy: Lens
  failures are triaged and reported, never ignored (see triage classes below).

## Phases

Order matters: the first yak is the *container* the other sensors wire into
(same conclusion as stickies — `pre-push-verification-loop-and-lens-review-practice`).

### Phase 1 — Fast/push split (the container)

Keep `precommit` cheap and fast (commit freely); move heavy checks to a
**Lens pass** = `make verify` + `.githooks/pre-push`.

- [ ] Root `Makefile`: `verify` target that loops over `app` and `blogex`,
      running the heavy checks cheapest-first (credo → sobelow → coverage+CRAP).
- [ ] `.githooks/pre-push` + `core.hooksPath=.githooks` (wire into `make`/setup).
      Fail loudly with a summary; trivially skippable via `git push --no-verify`.
- [ ] `blogex/mix.exs`: add an `aliases` block with `precommit`
      (compile --warnings-as-errors, format, test) so both apps share the loop.
- [ ] AGENTS.md: write the **Lens practice** down — review is a separate pass
      with its own context; don't chase hook nits inline while building; file a
      yak instead. Add the **triage classes** for Lens failures:
      *real bug* (fix inline) / *environment-dependent test* (the test is the
      bug — make it deterministic by construction, no ambient state as
      fixtures) / *style or design nit* (note or file a yak). Reconcile with
      the zero-defects policy: "never continue silently" ≠ "fix nits inline";
      it means report, triage, decide.
- [ ] Watch hook runtime: `app`'s `test` alias drops/creates/migrates the DB
      every run. If the pre-push run re-tests, suite time doubles. Consider a
      `test.crap` alias reusing an exported coverage run (stickies:
      `test --cover --export-coverage default` + `test.coverage` + `crap`).

### Phase 2 — Coverage floor + exclusions policy

- [ ] Add `test_coverage: [threshold: N]` to **both** `mix.exs` files
      (blogex first — it's pure and should be near-perfect; app likely lower).
      Run coverage once to see actual numbers before picking the floor.
- [ ] Make the **exclusions policy decision early**, not later (in stickies
      this came late, commit `af12c2d`): exclude framework-owned glue
      (`Firehose.Repo`, `ErrorHTML/JSON`, `Telemetry`, `Mailer`) — counting it
      inflates the floor without reflecting app behavior.
- [ ] Rule from stickies: the threshold is a **floor, ratcheted up as yaks
      land** — never lowered.

### Phase 3 — Behavior tests for the gaps (the bulk of the work)

Run per-app coverage, list the uncovered modules, file yaks per area
(stickies: "behavior tests: boundary shell", "…: web LiveViews", "…:
DevToolsPage"). See *Steering the model on missing tests* below for the
principles — this is where most steering happened.

Expected gaps in firehose, by module type:

- `blogex/*` — pure library, should be drivable to very high coverage through
  behavior tests (post → rendered HTML, invalid links → errors, feed output).
- `app/lib/firehose/accounts*` — context behavior (stickies did this for
  MultiTenancy: narrate from the actors' view).
- `firehose_web/live/*` and controllers — outcome tests through
  `Phoenix.LiveViewTest` / conn tests, not HTML-string assertions.
- Editorial features (`editor_dashboard_live`, `microprints_live`) — likely the
  thinnest coverage; behavior-test them like stickies' DevToolsPage.

- [ ] Coverage-driven yak list per app; work them one by one.
- [ ] After each yak: **ratchet the floor** up to the new total + small margin.

### Phase 4 — CRAP scores with ex_crap

- [ ] Add `{:ex_crap, "~> 0.1", only: [:dev, :test], runtime: false}` to both
      apps. **Sandbox gotcha:** `mix deps.get` cannot fetch new hex packages
      from inside the sandbox — edit `mix.exs`, then ask the user to run
      `mix deps.get` outside; stop there. Same for every dep below.
- [ ] Wire `mix crap` into the Lens pass (needs the coverage export from
      Phase 1's `test.crap` alias).
- [ ] Baseline at default `--max-score 30`. Fix offenders the stickies way:
      * pure data refactor where the complexity is real logic (9-branch
        `case` → compile-time lookup map);
      * for shell glue (LiveView `handle_event`), **close coverage gaps
        instead of refactoring** — a fully-covered complexity-22 function
        scores 24.48, under the default threshold.
- [ ] CRAP formula reminder: `comp^2*(1-cov)^3 + comp`; coverage ≥ ~75% of a
      comp-22 function clears threshold 30.

### Phase 5 — Credo strict, tuned

- [ ] Tune `.credo.exs` (both apps; blogex has none): **defaults, zero
      findings under `--strict` at introduction** — the config is the policy.
      Deliberate deviation: disable `Credo.Check.Refactor.CyclomaticComplexity`
      in favor of CRAP scores (a wide `case` with good coverage is not a
      problem; that was the original itch behind ExCrap).
- [ ] Runs in the Lens pass, not precommit.
- [ ] When strict flags code: refactor along **pure-data lines** (lookup
      tables, small pure classifiers, pure helpers that return values) — never
      split orchestration/shell code into async calls or scattered processes
      just to satisfy a metric (user, 2026-10-10: "I don't want an agent to
      arbitrarily break them down, but respect our design principles").

### Phase 6 — Sobelow

- [x] Add to the Lens pass for `app` (Phoenix surface). blogex is a library
      without endpoints; decide whether to run it there anyway — in ex_crap's
      own history sobelow caught a `String.to_atom` in a library, so it's not
      strictly web-only value. Decided 2026-10-28: **app only**; blogex's
      sobelow scope is a separate yak.
- [x] **Fail when sobelow is not installed** (stickies: user explicitly
      removed the skip — a silently-skipped check is a lying sensor).
      `make -C app sobelow` has no fallback: a missing task is a hard Mix
      error, and `verify`'s `|| exit 1` loop fails the pass.
- [x] Fix findings or add per-check skips *with rationale comments*
      (stickies: `Config.CSP` skipped because the plug is invisible to sobelow).
      Baseline 2026-10-28 (sobelow 0.17.0), 3 findings: `Config.HTTPS` (High),
      `Config.CSP` (High), `Traversal.FileModule` (Low). All three are skipped
      in `app/.sobelow-skips`, each with a written rationale: HTTPS and CSP are
      deployment/design decisions with follow-up yaks, and the Traversal flag
      is a false positive (the path is matched against the server-scanned list
      before `File.read`). Unlike stickies, the CSP skip is *not* "the plug is
      invisible" — this app genuinely has no CSP yet.

### Phase 7 — Boundary

Different shape here than in stickies:

- The **app/blogex split already enforces the biggest boundary** via the mix
  dep (`{:blogex, path: "../blogex"}`): app code can only use blogex's public
  API. The library direction is naturally clean.
- [ ] Inside `app`: declare boundaries for `Firehose` (contexts),
      `FirehoseWeb` (exports Endpoint/Telemetry), and make **`Firehose.Repo` a
      top-level sibling boundary with `deps: []`**, removed from `Firehose`'s
      exports — so web→Repo calls become compile-time violations, not
      convention (stickies commits `35f2eff`, `337a0c8`).
- [ ] Enable the boundary compiler in `:dev` **and** `:test` so precommit's
      `compile --warning-as-errors` gates on violations. Watch for
      unclassified-module warnings from test support modules (declare an
      ignored top-level boundary for them, as stickies did for `CardgameTest`).
- [ ] External apps whitelist: `[:ecto, :phoenix, :plug, :phoenix_live_view]`
      with per-module lists where boundary splits them.
- [ ] *Optional, not used in stickies:* Velasco's `boundary.spec.check` /
      `boundary.spec.accept` tasks — an agent-proof sensor that fails when the
      boundary definition itself changed without human approval. Cheap to add
      once boundaries exist; worth considering given firehose's zero-defects
      culture.

### Phase 8 — Mutation testing (muex)

- [ ] Add muex (dev-only, `runtime: false`) + `mix mutate` / `mix mutate.fast`
      aliases. **Scope tightly to pure code**: `blogex/lib/**` (link_validator,
      seo, feed, post parsing) and any pure modules in `app` — stickies scoped
      to `lib/cardgame/{domain,events}` and left shell code alone.
- [ ] Curated mutator set: `arithmetic,boolean,comparison,conditional,
      function_call,literal,return_value` (excludes macro/use-based mutators —
      in ex_crap itself, `use Boundary` removals produced false-positive
      survivors).
- [ ] `--fail-at 0`: mutation testing stays a **sensor, not a gate**; documented
      false positives (guard `and`↔`or` mutants, removed log lines) are noted,
      not chased.
- [ ] Genuine survivors → new tests that pin the behavior (e.g. per-field
      error tests pinning chained guards).

### Phase 9 — Ratchets, decisions, handoff

- [ ] Ratchet coverage floors after each behavior yak lands.
- [ ] Decide (with the user, don't assume): CRAP `--max-score` below 30?
      boundary.spec.check adoption? Playwright in the Lens pass or kept as a
      separate external-verification loop?
- [ ] "Landing the plane": after each session with code changes, run
      `make precommit` always; run `make verify` (or push, which fires the
      hook) before handing off.

## Steering the model on missing tests (what made stickies work)

The sensors tell you *where* tests are missing; the steering below is what
made the resulting tests good rather than line-chasing. Recorded from the
stickies session history (Oct 10–12) so it can be pasted into firehose
sessions or this repo's AGENTS.md:

1. **Behavior, not state.** Narrate tests from the actors' view — "host starts
   a team, members join, roles change, members leave" — and assert *outcomes*
   (the next actor's experience), not internal struct/assign shapes. Pin
   current behavioral decisions explicitly when a test does so (e.g. "same-name
   teams cannot both exist").
2. **Functional core, imperative shell.** Details of persistence (how joining
   works internally) stay out of most tests; the test drives the public API of
   the core. GenServers/stores get *outcome* tests (persistence failure → state
   unchanged; restart → replay), not process-internals tests.
3. **Don't test dead code — delete it.** Coverage gaps that are unreachable
   mount branches (plug guarantees the precondition) are removed with a comment
   stating the contract, not tested.
4. **Leave genuinely defensive branches uncovered, deliberately.** Store
   failure paths only reachable by DB failure, fallbacks the router guarantees
   — chasing them with artificial mocks tests lines, not behavior. Note them
   in the yak context so the next ratchet isn't confused.
5. **Refactor along pure-data lines when a metric flags code**; never split a
   linear shell script into async tasks to satisfy a metric. A flat lookup
   `case` that is genuinely clearer than a table may stay — note it, move on.
6. **Don't chase Lens nits inline** while building; file a yak, let the Lens
   pass handle them.
7. **Sandbox:** new hex deps need the user to run `mix deps.get` outside the
   sandbox (read-only `~/.hex/packages`). Never work around it.

## Suggested yak list (for `yx` in firehose)

1. Fast/push split: make verify + pre-push hook + Lens practice in AGENTS.md
2. Coverage floors + framework-glue exclusions policy (both apps)
3. Behavior tests: blogex library (probably 2–3 yaks by area)
4. Behavior tests: accounts context
5. Behavior tests: firehose_web LiveViews/controllers
6. CRAP scores with ex_crap (both apps)
7. Credo strict tuned config (both apps)
8. Sobelow in the Lens pass
9. Boundary: Repo as sibling boundary, web→Repo forbidden
10. Mutation testing with muex (blogex + pure app code)