Files
firehose/doc/improve-verification.md
T

14 KiB
Raw Blame History

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.

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

  • 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.
  • 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.
  • 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)