motovaultpro/.claude/skills/mvp-proof-and-analysis-toolkit/SKILL.md

---
name: mvp-proof-and-analysis-toolkit
description: Prove-it methods for high-stakes decisions and investigations in MotoVaultPro. Load this when you are about to commit to an architectural or design decision (schema change, API contract, caching strategy, "just return raw rows"-class shortcuts), when a bug has survived one or more failed fix attempts, when you catch yourself writing "this should fix it" or "this is probably faster", when a root-cause claim needs to be defended in a PR, or when someone asks to stress-test / red-team / sanity-check a decision. Symptom keywords - fix didn't work, still broken after fix, intermittent, works locally fails on staging, off by one, wrong value displayed, performance claim, premature optimization, root cause unclear, second fix for same bug.
---

# MotoVaultPro Proof and Analysis Toolkit

Five recipes for not being wrong when being wrong is expensive. Each recipe carries a worked example from this repo's actual git history — every commit hash cited here is verifiable with `git show <hash>`.

**When to use this skill:**
- Before committing to a decision that is costly to reverse (schema, API contract, dependency, data-flow convention).
- When a bug has already eaten one failed fix — stop patching, start proving.
- Before making any performance or "root cause" claim in a PR description or issue comment.

**When NOT to use this skill:**
- Routine debugging with a fresh symptom: start with `mvp-debugging-playbook` (symptom-to-triage table). Come back here if the first fix fails.
- Checking whether a battle was already fought and settled: `mvp-failure-archaeology`.
- "What counts as done/tested": `mvp-validation-and-qa`.
- Reading logs and metrics mechanics (LogQL, Grafana): `mvp-diagnostics-and-logging`.
- Issue/branch/PR mechanics and review severity taxonomy: `mvp-change-control`.

---

## Recipe 1: The decision stress-test protocol

Run this before committing to any decision where reversal is expensive. It is a checklist you execute directly in-session (the former Python step-injector delivery is retired; the content is preserved here). Grounded in Chain-of-Verification (Dhuliawala et al., 2023), Self-Consistency (Wang et al., 2023), and Multi-Expert Prompting (Wang et al., 2024).

### Phase 1 — Decompose into stable IDs

Break the decision into parts, each with an ID that persists through every later step:

| Prefix | Type | Count | Prompt |
| ------ | ---- | ----- | ------ |
| C1, C2... | CLAIMS — factual assertions | 3-7 | What facts does this decision assume true? What cause-effect relationships does it depend on? |
| A1, A2... | ASSUMPTIONS — unstated beliefs | 2-5 | What is implied but not stated? What would someone unfamiliar with the context not know? |
| K1, K2... | CONSTRAINTS — hard boundaries | 1-4 | What technical, organizational, or timeline limits apply? |
| J1, J2... | JUDGMENTS — subjective tradeoffs | 1-3 | Where are values weighed against each other? What "it depends" calls were made? |

### Phase 2 — Classify verifiability

Tag every item:

- **[V] VERIFIABLE** — can be checked against evidence or tested ("pg returns DECIMAL as strings" — checkable).
- **[J] JUDGMENT** — subjective tradeoff, no objectively correct answer ("simplicity beats flexibility here").
- **[C] CONSTRAINT** — given condition, fixed for this decision ("no fully working local dev loop; e2e verification is on staging").

Edge-case rule: when an item fits multiple categories, prefer [V] over [J] over [C] — verifiable items can be checked, judgments only debated, constraints merely accepted. ("The team can deliver in 4 weeks" is [V]: checkable against past velocity.)

### Phase 3 — Chain-of-Verification questions

For each [V] item, write 1-3 questions (1 for simple claims, 3 max for compound ones) that:
- are specific and independently answerable,
- are designed to reveal if the claim is FALSE (falsification focus),
- do not assume the claim is true in their own wording,
- each test a different aspect of the claim.

### Phase 4 — Factored verification (the epistemic boundary)

Answer each question INDEPENDENTLY, without the original decision's reasoning in view. This is the step that catches confirmation bias, so it gets the strictest rule:

Answer using ONLY:
- (a) established domain knowledge (documentation, references — e.g. node-postgres docs),
- (b) stated constraints from the decision context,
- (c) logical inference from first principles that holds regardless of whether this decision is correct.

Do NOT:
- assume the decision is correct and work backward,
- assume it is incorrect and hunt for disproof,
- reference whether the claim "should" be true given the decision.

For every question, separate **Answer** (factual, evidence-based) from **Implication** (what it means for the claim). Then mark each [V] item: **VERIFIED** | **FAILED** | **UNCERTAIN** (state what additional information would resolve it).

In this repo, "verify against evidence" usually means: read the actual code path (`grep -rn` in `backend/src/features/`), read the actual schema, or `git log -S "<string>"` for history — never memory of what the code "should" say. Where docs contradict code, code wins.

### Phase 5 — Steel-man contrarian

Generate the STRONGEST case against the decision — the opposition's best case, not a strawman. Start from Phase 4 results: FAILED items are direct ammunition; UNCERTAIN items are attack vectors; even VERIFIED items may hide dependencies. Explore: what could go wrong that was not considered; what alternatives were dismissed too fast; what second-order effects were missed; what happens if key assumptions change; who would disagree and why they might be right. Output a one-sentence contrarian position, a 2-3 paragraph argument referencing item IDs, and 3 key risks.

### Phase 6 — Problem reframing

Phase 5 challenged the solution; this challenges the PROBLEM STATEMENT. Set the proposed solution aside and ask: approached fresh, how would I state this problem? Is it a symptom of a deeper issue? What would a different stakeholder (user, ops, security) prioritize? What if the K constraints were negotiable? Is there a simpler formulation that dissolves the tradeoffs? State the alternative framing, what it emphasizes, which hidden assumptions it reveals (by item ID), and whether it strengthens, weakens, or redirects the decision.

### Phase 7 — Verdict

| Verdict | Trigger (ANY for ESCALATE/REVISE; ALL for STAND) |
| ------- | ------------------------------------------------ |
| **ESCALATE** | Any FAILED item involves safety, security, or data integrity; OR a critical UNCERTAIN item cannot be cheaply verified; OR the reframing shows the problem itself is wrong. Escalation target here: stop, write it up in the Gitea issue, get the owner's call before proceeding. |
| **REVISE** | Any FAILED item on a core (not peripheral) claim; OR multiple UNCERTAIN items on feasibility/effort/impact; OR the challenge phase revealed gaps that change the calculus. |
| **STAND** | No FAILED core claims; UNCERTAIN items explicitly acknowledged as accepted risks; Phase 5-6 challenges addressable within the current approach. |

Tie-breakers: between STAND and REVISE, favor REVISE (cheaper to refine than to fail). Between REVISE and ESCALATE, state both options with the conditions under which each applies.

Output: VERDICT, verification summary (Verified/Failed/Uncertain ID lists), strongest challenge + reframing insight + how the decision answers them, and a specific next action.

### Worked example: the #47 "return raw rows" decision (2026-01-17)

The bug: Fuel Logs API returned 500 "Invalid time value". Diagnosis: enhanced repository methods called `mapRow()` (snake_case to camelCase) but the service's `toEnhancedResponse()` read snake_case keys, so `new Date(row.created_at)` got `undefined`. The decision taken in commit `574acf3`: make seven enhanced repository methods return **raw pg rows**. Decomposed retroactively:

```
C1 [V]: toEnhancedResponse() reads snake_case keys from its input rows.
C2 [V]: mapRow() output (camelCase) makes row.created_at undefined, causing "Invalid time value".
A1 [V]: Raw pg rows satisfy the TypeScript types the enhanced methods declare (numbers as numbers).
A2 [V]: Returning raw rows is consistent with this project's repository conventions.
J1 [J]: Reverting the repo to raw rows is cheaper than changing the service to consume mapped rows.
```

Factored verification, behind the epistemic boundary (no access to the fix's own reasoning):

- **A1 — Q: What JavaScript types does node-postgres return for DECIMAL/NUMERIC columns?** Answer, from pg documentation alone: strings, to avoid float precision loss. Implication: raw rows put strings where the declared types say `number`. **A1: FAILED.** (Corroborating evidence available the same day: #49, fuel values displaying wrong for exactly this reason.)
- **A2 — Q: What do project conventions say repositories may return?** CLAUDE.md (which already contained the rule at the time, added 2025-12-21): "All methods returning data to the API must use these mappers - never return raw database rows." **A2: FAILED.**

Verdict under the Phase 7 rubric: **REVISE** — two FAILED items on core claims. The revised direction is the one the codebase eventually adopted anyway: keep a mapper in the repository and coerce numeric columns (what `mapEnhancedRow` in commit `0d90829` did). Instead, the raw-rows shortcut shipped and lay dormant for four months until #244 (2026-05-15): maintenance cost blank on the vehicle summary, traced back through #239 and #241 to DECIMAL columns arriving as strings from exactly these methods. One hour of factored verification in January would have bought back a three-issue debugging chain in May. That is the ROI profile of this protocol: it pays on decisions whose failure mode is *latent*.

---

## Recipe 2: The root-cause evidence bar

A root-cause claim is accepted only when it clears BOTH bars:

1. **ONE mechanism explains ALL observations — including the negatives.** List every observation, including what did NOT happen (errors that did not fire, platforms that did not break, states that stayed intact). A candidate cause that explains 4 of 5 observations is not "mostly right"; it is either wrong or incomplete (there may be two causes — see the worked example).
2. **It survives an assigned adversarial refutation pass.** Before writing the fix, deliberately try to break your own explanation: "Assume this mechanism is NOT the cause. What observation would prove that?" Then go check that observation. In a multi-agent session, assign the refutation to a separate subagent with only the observation list and the candidate mechanism — not your reasoning (same epistemic boundary as Recipe 1 Phase 4).

Practical worksheet:

```
OBSERVATIONS (positive):  O1, O2, ...
OBSERVATIONS (negative):  N1 "no error was logged", N2 "desktop unaffected", ...
CANDIDATE MECHANISM:      M
COVERAGE CHECK:           for each O and N: does M predict it? YES/NO/PARTIAL
REFUTATION ASSIGNMENT:    "If M were false, we would observe ____" -> go observe.
```

Any PARTIAL or NO means M is not yet the root cause. Fixes shipped on partial coverage are symptom fixes.

### Worked example: the mobile login saga (#188/#190, 2026-02-14 to 02-15)

Symptom: on mobile, login redirected to the homepage without ever showing the Auth0 page. Four symptom fixes shipped in two days — `a195fa9` (allow callback route to complete code exchange), `6e493e9` (detect/clear stale IndexedDB tokens), `38debaa` (skip stale-token validation during callback), `b5b82db` (resolve callback failure from cache issues) — and the symptom persisted. Each fix explained SOME observations. None explained all of them, because there were **two independent mechanisms**:

- **Root cause 1** (`da59168`): `loadCacheFromDB` used IndexedDB `store.getAll()`, which returns raw values, not key-value pairs — so an `item.key` check always failed and `memoryCache` was empty after every page reload. Auth0 SDK state stored before the redirect was lost on mobile Safari (no bfcache). This explains the negative observation that desktop often worked: desktop kept the page alive across the redirect, so the in-memory cache never needed reloading.
- **Root cause 2** (`850f713`): React fires child effects before parent effects. App.tsx's URL-sync effect called `history.replaceState()` on `/callback`, stripping `?code=` and `&state=` before Auth0Provider's own effect could read them — so the SDK fell through to `checkSession()` and failed **silently**. This explains the key negative observation: no error anywhere.

What "explains all observations" looked like: fixing root cause 1 alone still left logins failing (state survived reload, but the code/state params were still being stripped); only mechanism 2 predicted the total silence of the failure. Each mechanism was separately proven — `da59168` demonstrated the empty-cache behavior directly in `frontend/src/core/utils/indexeddb-storage.ts`, `850f713` demonstrated the effect-ordering strip in `frontend/src/App.tsx`. Lesson: when no single mechanism covers the observation list, do not force one — test the hypothesis "there are two."

---

## Recipe 3: Hypothesis predicts numbers BEFORE running

Write the predicted observation — the exact string, count, date, or status code — BEFORE running the experiment. A hypothesis that only *postdicts* (explains the result after you see it) is not evidence, because most wrong hypotheses can also explain the result after the fact. The discriminating power is in the prediction differing between hypotheses.

Format, written down before the experiment:

```
H1: <mechanism>  ->  PREDICTS: <exact observable, e.g. API returns "2026-03-22", status 500, count=0>
H2: <mechanism>  ->  PREDICTS: <a DIFFERENT exact observable>
EXPERIMENT: <one command / one click path>
```

If two hypotheses predict the same observable, the experiment cannot distinguish them — design a different experiment before running anything.

### Worked example: the date off-by-one (#237, fixed four times on 2026-03-23)

Symptom: dates displayed one day off. There were THREE distinct UTC traps live at once, and each predicts a DIFFERENT specific wrong observation — which is what makes them separable without guessing:

| Trap | Mechanism | Discriminating prediction (write BEFORE looking) |
| ---- | --------- | ------------------------------------------------ |
| A — backend pg parser (`f0fc427`) | pg's default parser turns DATE columns into JS `Date` objects at local midnight; JSON serialization via `toISOString()` converts to UTC and can shift the calendar day | The **raw API JSON** (curl the backend directly, bypassing the UI) shows the date as a full ISO timestamp (e.g. `"2026-03-23T00:00:00.000Z"`), not a plain `"YYYY-MM-DD"` string. The DB value itself is correct. |
| B — frontend display (`1e056f0`) | `new Date("YYYY-MM-DD")` parses as UTC midnight; `toLocaleDateString()` in a browser west of UTC renders the **previous** day | The raw API JSON shows the correct plain `"YYYY-MM-DD"` string; only the rendered UI is wrong, shifted one day EARLIER, and only for negative-UTC-offset browsers. DB and API both correct. |
| C — OCR write path (`087f7b9`) | receipt-OCR hook built the date via `toISOString().split('T')[0]` on a local-time Date, shifting the day at WRITE time | The **stored DB row itself** contains the wrong date (query PostgreSQL directly), and the error is time-of-day dependent — records created in local evening hours shift, morning ones do not. UI faithfully displays the wrong stored value. |

Three layers (DB value / API JSON / rendered UI) and three predictions that disagree at a specific layer. Two curl commands and one SQL query discriminate all three — no fix needed to be "tried" to find out which trap was live. The actual history (`c05e33e` then `f0fc427` then two more fixes the same day) shows what happens without this: each fix addressed one trap, the symptom "came back," and the day ended with four commits. The settled outcome (do not re-fight it — see `mvp-failure-archaeology`): DATE columns flow as plain `YYYY-MM-DD` strings end to end; `types.setTypeParser(1082, ...)` in `backend/src/core/config/database.ts` is load-bearing; display via dayjs; never `new Date(dateString)` or `toISOString()` for DATE values.

---

## Recipe 4: Benchmark before claiming (the Measure First rule)

CLAUDE.md, verbatim: "No premature optimization" and "Benchmark before claiming something is faster." In this project a performance claim is valid only with:

1. **A real measurement source.** Every backend request is logged with a `duration` field in milliseconds (`backend/src/core/plugins/logging.plugin.ts`, `msg="Request processed"`, fields: `requestId`, `method`, `path`, `status`, `duration`, `ip`). Query it in Grafana (logs.motovaultpro.com) via Loki. LogQL mechanics live in `mvp-diagnostics-and-logging`.
2. **N >= 20 samples per variant.** Single requests are noise — Gemini cold starts alone (see `mvp-failure-archaeology`, the VIN saga) produce order-of-magnitude outliers. Compare medians or p95, not single numbers or means.
3. **Same environment for both sides of the comparison.** Before/after numbers from different environments (local container vs staging) are not comparable. Given this project's reality — no fully working local dev loop, e2e verification on staging via the PR deploy pipeline — that means both measurements come from staging.
4. **The prediction written first** (Recipe 3): "this change should move p95 for `GET /api/...` from Xms to under Yms" — before deploying it.

Corollary (no premature optimization): if you cannot state which logged endpoint's `duration` is currently unacceptable and by how much, the optimization has no justified motivation. Skip it and note it as a candidate in the issue instead.

---

## Recipe 5: The diff-format authority model (for plans and review findings)

When a plan, review finding, or handoff references a code location, use unified-diff conventions with this authority model (ported from the retired planner skill; it is the standard here because line numbers drift between when a plan is written and when it executes):

| Component | Authority | Meaning |
| --------- | --------- | ------- |
| File path (`--- a/backend/src/...`) | **AUTHORITATIVE** | Exact target file. Never "the auth files" — always the repo-relative path. |
| Line numbers (`@@ -123,6 +123,15 @@`) | **APPROXIMATE** | May drift as earlier changes land. Never treat as exact; never write "insert at line 42" in prose or comments. |
| Function context after `@@` | **SCOPE HINT** | Which function/method contains the change. |
| Context lines (unchanged) | **AUTHORITATIVE ANCHORS** | The executor locates the change by matching these patterns in the actual file. Include 2-3 before and after. |
| `+` / `-` lines | The change itself | Complete code. |

Rules:

- **No elision placeholders.** `...`, `[existing code]`, `// rest unchanged` are forbidden inside diff blocks — an executor cannot match an anchor that is not real text from the file.
- **Context lines must actually exist** in the target file. Validate by grepping before finalizing the plan or finding.
- **Comments in `+` lines explain WHY, not WHAT**, with no hidden baselines ("generous timeout" — compared to what? Write "60s accommodates p95 upstream response time" instead) and no change-narrative ("added to fix #244" belongs in the commit message, not the code).
- **Location directives in comments are forbidden** — the diff structure IS the location.

Validation checklist before finalizing:

- [ ] File path exact and repo-relative
- [ ] Context lines verified present in the target file
- [ ] 2-3 context lines each side of the change
- [ ] No `...` / `[existing code]` placeholders
- [ ] Comments are WHY-comments with concrete baselines
- [ ] No line-number directives in prose or comments

The same authority model applies to review findings: a finding anchored only to a line number is fragile; a finding anchored to a file path plus a quoted code snippet (the anchor) survives rebases.

---

## Provenance and maintenance

Authored 2026-07-07 from direct inspection of this repo. The five recipes are method (stable); the worked examples and anchors below are facts (volatile). Re-verify before relying on them:

| Volatile fact | Re-verification command |
| ------------- | ----------------------- |
| #47 raw-rows commit and its rationale | `git show 574acf3 --stat` |
| #244 mapEnhancedRow fix (repays #47) | `git show 0d90829 --stat` |
| "never return raw database rows" rule predates #47 | `git log --date=short -S "never return raw database rows" -- CLAUDE.md` |
| Mobile login root cause 1 (IndexedDB getAll) | `git show da59168` |
| Mobile login root cause 2 (effect-order URL strip) | `git show 850f713` |
| Date trap fixes (backend parser / frontend / OCR) | `git show f0fc427 087f7b9 1e056f0 --stat` |
| DATE parser override still present | `grep -n "setTypeParser(1082" backend/src/core/config/database.ts` |
| mapEnhancedRow still present in fuel-logs repo | `grep -n "mapEnhancedRow" backend/src/features/fuel-logs/data/fuel-logs.repository.ts` |
| Request log `duration` field and `msg="Request processed"` | `grep -n "duration" backend/src/core/plugins/logging.plugin.ts` |
| Grafana host | `grep -n "logs.motovaultpro.com" docker-compose.yml` |
| Retired source protocols (decision-critic, diff-format) | `git show HEAD:.claude/skills/decision-critic/scripts/decision-critic.py` and `git show HEAD:.claude/skills/planner/resources/diff-format.md` (valid while the old library remains in history; adjust the ref if deletion has been committed) |

Note on sample size in Recipe 4: the N >= 20 threshold is a working convention set by this skill (2026-07-07), not an owner directive — adjust with judgment, but never claim from N=1.