Skip to content

feat: replace twelve thresholds with one sensitivity control that shows its limits (S8) - #85

Merged
matthewpmunger merged 1 commit into
mainfrom
chunk-s8
Aug 26, 2026
Merged

feat: replace twelve thresholds with one sensitivity control that shows its limits (S8)#85
matthewpmunger merged 1 commit into
mainfrom
chunk-s8

Conversation

@matthewpmunger

Copy link
Copy Markdown
Collaborator

Depends on F1, F3, S3 — all merged. Base: f37b1569c7e4c286a7407bddd3945cb2b922f8f8.

Option 10b: one sensitivity control that displays the limits it resolves to. Twelve per-metric thresholds and no-thresholds-at-all were both considered and rejected — the first gives you every number honestly and no way to say what any of them does to tonight's digest; the second has no answer to "why am I being told this" other than "the product decided", and the digest's threshold clause is the reason a reader trusts the line.

⚠️ Behaviour change, not a refactor

The default savings gate moves from 0 ms to 250 ms. DEFAULT_PERFORMANCE_THRESHOLDS is now the Normal position, read from sensitivity.ts rather than restated.

Existing sites will see fewer findings in the digest and in the list: anything with a measured saving below 250 ms now folds. That is a real loss of lines, and it is deliberate. A digest line that said "above the limit you set" against a gate of 0 was claiming something false — there was no limit and the reader set nothing — so thresholdOf withheld the whole clause. Every position now resolves to a limit the reader can see, which is what makes the sentence true.

Sites that had tuned their thresholds by hand are not silently moved to 250 ms: they map to the nearest position and are told, once. See Migration below.

What the screen is

Five groups, in the order a reader needs them, and the order is an argument. Sensitivity first because it is the only setting that changes what the product says. Digest second because it is how it says it. Excluded third because it is the answer to "why am I not seeing X". Connected systems fourth because it is infrastructure. Appearance last because it is the only one not about the site.

Settings was a second mode of the watchlist component before this — one file, one mode prop, two pages sharing a header and nothing else. It is its own route now and the watchlist is a watchlist.

The limits are the digest's, not a second copy

digestLimit in digest-copy.ts has exactly two readers: the digest's threshold clause, and the row under the control. sensitivity.test.ts builds a real digest at each position and asserts the line contains above the ${shown} you set — so a reworded unit fails the build rather than reaching one reader and not the other (rule 20; rule 21 for asserting against the other half of the decision instead of a literal).

No position resolves the savings gate to 0. At 0 the gate is off, the clause is withheld, and there is nothing to display — a position that resolves to nothing cannot be shown. "Everything" is 1 ms, which is every measurement formatImpact can write.

Migration

A site whose thresholds match no position was configured by hand. Both dishonest answers were available: drop it (their site quietly starts reporting different things) or keep it (a settings screen that cannot show the state it is in). It is mapped to the nearest position and the reader is told once, in the digest footer, in the position's own words — never the numbers, which are what they no longer have a control for.

Cleared only when a message that carried it completes, so a failed send leaves the sentence owed rather than spent.

The distance metric divides by the span the three positions actually cover, not by each field's legal range: regressionFloor is legal 1–100 but sensitivity only moves it 90–100, and measuring against the range would let a field the control barely touches outvote one it swings across.

Excluded from results

One list over pages and checks, because applicability is one registry concept and four screens for it is how a reader ends up with an exclusion they cannot find — the failure the audit recorded when the agent tab hid evidence without saying why.

Every row keeps its last reading, struck through. Struck through says "not counted"; an empty cell would say "never measured". A row with no reading says so in words rather than showing 0 (rule 18). A check standing for several pages shows the worst reading one of them produced, never a tally (rule 19).

Excluding now asks for a reason and stores it (AgentIgnoreSettings.reasons), which is what applicability always required. Records written by the old unlabelled toggle read as UNLABELLED_EXCLUSION_REASON — the definition of the state that toggle put them in, following the precedent nativeElements.ts already set for its retired suppressed disposition, now stated once in vocabulary.ts and read by both.

Include is offered only where the change can be kept. A case-page exclusion is derived with nowhere to persist to until F5, so that row renders without the button rather than with a dead one — the same rule CasePages states for the same concept.

Deleted, at f37b156

Item State at base Action
Per-page threshold controls in page detail Already gone (S3 removed the panel) Deleted the orphaned plumbing: route, mutation, WatchPage.performanceThresholdOverrides, store action, seed override, and effectivePerformanceThresholds layering. normalizeState erases any stored override on read — a stored value with no writer would otherwise sit there waiting for someone to re-add a reader.
Per-metric threshold controls Existed — "Monitoring tolerances", 12 fields + device policy, in the watchlist component's settings branch Deleted, with NumberStepper / ToleranceField / SettingTooltip / SettingHeader and ~150 lines of dead CSS
Alert severity levels on watchlist rows Did not exist — only match was a comment in case-detail.tsx Nothing to delete
Threshold UI outside /settings None sensitivity.test.ts asserts it structurally over the whole src tree, rather than promising it in a comment

Appearance at 320px

Canonical here. The sidebar footer keeps its copy as a shortcut and collapses with the rest of the sidebar footer on a narrow viewport — correct behaviour, and only correct because this screen exists. settings-reachability.test.ts is what keeps it correct: it checks the route is in the collapsed sidebar's nav, that no narrow-viewport block hides a .settings-* container, that .settings-group__head stacks (side by side at 320px the three-segment control gets ~90px, which is not a control), and that the page has no min-width.

Options stay Auto / Light / Dark. APPEARANCE_LABEL and the pre-paint script own those values; renaming them to a parenthetical would be a rule 20 defect with the added risk of drifting from the script R1 found had no test.

Registry

vocabulary.json's banned-vocabulary allowlist loses src/app/(app)/watchlist/page.tsx. The Ignore/Suppress copy that earned the entry moved to /settings and became the applicability concept; the watchlist itself never used those words. The list may only shrink, and it did.

Not done, by decision

  • The lifecycle, the 2/7/30 schedule and source weighting are not configurable.
  • No per-metric alerting anywhere, tidier or otherwise.
  • No weighting, ranking or trust order over connected systems. The ledger keeps one entry per system so a disagreement is visible instead of averaged away; a control that ordered them would be a blend with a nicer name.
  • docs/visual-inventory.md still describes the deleted panels. It is a dated snapshot ("Inventory date: August 4, 2026") and S3 set the precedent of leaving it. Rewriting a record of what the app looked like on a date falsifies it rather than updating it. If it is meant to be current rather than historical, that is a separate call about what the file is for.

Validation

npm run lint, npm run typecheck, npm test -- --run (100 files, 1229 tests), npm run build, and npx --yes npm@10.9.7 ci --dry-run --ignore-scripts --no-audit --no-fund all pass. No dependency or lockfile change.

New tests: sensitivity.test.ts, settings-copy.test.ts, settings-exclusions.test.ts, settings-reachability.test.ts. digest-cadence.test.ts's "is stated, not stored — the setting is S8's" flips: the slot has a producer now, and what it asserts instead is that the setting landed as one switch, on one route, with no per-field variants.

@matthewpmunger

Copy link
Copy Markdown
Collaborator Author

⚠️ main was already red when this branch rebased onto it

origin/main at 728eb6c (the F4 merge) fails CI on its own: run 32992892380. Two separate things, and this PR handles them differently.

1. Semantic merge conflict, F4 × S7 — fixed here (57aab84)

F4 split the registry's actor (which classes MAY fire a transition) from the record of who did, renaming TransitionOptions.actorby: Caller. S7's digest fixtures still pass the bare class "person", which now type-checks against neither half. Ten typecheck errors across digest.test.ts, digest-arrival.test.ts and webhook.test.ts.

Neither PR's CI could see it: F4 branched before S7 merged and S7 branched before F4 merged, so each was green against a main that did not contain the other. This is the first branch to touch both sides.

Fixed in a separate commit so it can be reviewed or cherry-picked apart from the feature. The repair is the one F4's own migration note prescribes — a whole Caller, named — using the same rae@webflow.com the rest of the suite already uses.

I took this one because S8's new tests hit the same API and the branch cannot be green without it.

2. F4's guard flags F4's own code — left alone, needs an F4 ruling

caller.test.ts › no identity string is ever compared against a permission list
  offenders: ["lib/issue-case.ts: .actor.includes(\"person\")"]

personActionsFor (issue-case.ts:413) does ISSUE_TRANSITIONS[action].actor.includes("person") — asking the permission list whether it permits the person class, to decide whether an action can be a button. That is the legal direction; the guard is aimed at the opposite one, an identity string tested against a list of permitted names.

The guard's regex requires the argument to mention .kind, and a class literal does not. The narrowest true fix is to accept a registry actor class as well as a caller's .kind — which keeps every case the guard exists for (an email, "checkpoint") failing. But that is a judgement about F4's guard, in files S8 does not touch, so I have not made it.

This PR will stay red on that one test until F4 rules. Everything else — lint, typecheck, 1244 tests, build — passes.

…ws its limits (S8)

Settings is one page, five groups, no tabs: sensitivity, digest, excluded
from results, connected systems, appearance. It was a second mode of the
watchlist component before this; it is its own route now, and the watchlist
is a watchlist.

Option 10b, and the two rejected alternatives are worth naming because both
look reasonable from a distance. Twelve per-metric thresholds: every number
honest, and nobody could say what any of them would do to tonight's digest.
No thresholds at all: nothing to get wrong, and no answer to "why am I being
told this" other than "the product decided" — but the digest's threshold
clause is the reason a reader trusts the line, and it needs a setting behind
it to be true.

So one control, three positions, and the limits it resolves to are printed
beneath it in the digest's own words. Not equivalent numbers: the same
string, from `digestLimit`, which the digest's threshold clause also reads.
`sensitivity.test.ts` builds a real digest and asserts the screen shows what
the message wrote, character for character, so a reworded unit fails the
build rather than reaching one reader and not the other. No position resolves
the savings gate to 0 — at 0 there is no limit the reader set, so the clause
is withheld and there is nothing to display, which is why "Everything" is
1 ms rather than off.

A site with hand-tuned thresholds maps to the nearest position and is told
once, in the digest footer, in the position's own words. Cleared only when a
message that carried it completes, so a failed send leaves the sentence owed
rather than spent. Silently discarding someone's configuration is worse than
the configuration was; silently replacing it is the same failure with a
nicer result.

Excluded from results is one list over pages AND checks, because
applicability is one concept and four screens for it is how a reader ends up
with an exclusion they cannot find. Every row keeps its last reading, struck
through — struck through says "not counted", an empty cell would say "never
measured" — and its reason. Excluding now asks for that reason rather than
carrying the migrated one, which is what the concept always required.

Deleted, at f37b156: the monitoring-tolerance panel and its twelve fields,
the per-page threshold route, mutation, stored overrides and store action,
and the dead CSS for all of it. The page-detail calibration panel and the
watchlist severity levels were already gone — S3 removed the first and
nothing carried the second — so there was nothing to delete there and
nothing is given a new home. There is no threshold UI outside /settings, and
`sensitivity.test.ts` asserts it structurally rather than promising it in a
comment.

Appearance is canonical on this screen and reachable at 320px. The sidebar
footer control stays as a shortcut and collapses with the rest of the
sidebar footer on a narrow viewport; that is correct behaviour, and
`settings-reachability.test.ts` is what keeps it correct.

The registry's banned-vocabulary allowlist loses watchlist/page.tsx: the
copy that earned the entry moved to /settings and became the applicability
concept.

Also removes six duplicate `const PERSON` declarations in three digest test
files. F5 and R2 each independently repaired F4's `actor` → `by` rename over
S7's fixtures, and the merge applied both, so main has not type-checked since
96964f9. Not S8's breakage and not S8's to have caused; kept here because the
branch cannot be green without it and because deleting a redeclaration is not
a judgement anyone needs to make twice. The surviving comment in each file is
F5's, which says why the fixture needs a caller at all rather than only that
it does.
@matthewpmunger

Copy link
Copy Markdown
Collaborator Author

Rebased onto 96964f9 — superseding my earlier note

Since this PR opened, F5 (#83) and R2 (#86) both merged. Three things changed for S8, and one of them found a bug in main.

The F4 × S7 repair is no longer mine — dropped

R2 shipped the same fix. My repair commit is gone; this branch is now a single feature commit on top of current main.

main is still red, for a new reason — fixed here

F5 and R2 each independently repaired F4's actorby rename over S7's fixtures. The merge applied both, so origin/main at 96964f9 has six duplicate const PERSON declarations across digest.test.ts, digest-arrival.test.ts and webhook.test.ts, and does not type-check:

git show origin/main:src/lib/__tests__/digest.test.ts | grep -n "const PERSON"
38:const PERSON: Caller = { kind: "person", userId: "rae@webflow.com" };
71:const PERSON: Caller = { kind: "person", userId: "rae@webflow.com" };

Deleted the redundant declaration in each file. Not S8's breakage, but deleting a redeclaration is not a judgement anyone needs to make twice, and the branch cannot be green without it. The surviving comment in each file is F5's — it says why the fixture needs a caller, not merely that it does.

The earlier caller.test.ts guard failure I flagged is gone; R2/F5 resolved it.

F5 ungated the exclude control — S8 now uses it

My first draft rendered excluded-page rows without an Include button, because excludedPages had no writer and a button that loses a reader's decision on reload is worse than no button. F5 changed that: issueCasesFrom now applies caseDecisions, so the exclusion is real and reversible.

So the Excluded list now:

  • passes caseDecisions into issueCasesFrom, so the pages half of "pages AND checks" describes decisions people actually made;
  • offers Include on all three record kinds, wired to recordCaseDecision({ decision: "include", ... }) for a case page.

One thing worth a look from F5's author: F5's guard caught my first attempt, correctly. I had the row carry a precomputed remediationKey, which type-checked and read fine — but case-decisions.test.ts requires every recordCaseDecision({...}) call site to contain remediationKey: remediationKey(, and the reason is exactly right. A row carrying a key puts a second key in circulation, and nothing can then tell one this module produced from one a caller invented. The row now carries the IssueCase and the call site derives the key where the guard can see it. Good mechanism.

Green

lint, typecheck, 1277 tests, build. No lockfile change.

@matthewpmunger
matthewpmunger merged commit 37d6260 into main Aug 26, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant