Skip to content

Check any metric of any variant, and let every matching threshold fire - #1018

Merged
epompeii merged 9 commits into
develfrom
u/ep/parameters-api/thresholds
Sep 23, 2026
Merged

epompeii merged 9 commits into
develfrom
u/ep/parameters-api/thresholds

Conversation

@epompeii

@epompeii epompeii commented Aug 26, 2026 •

Copy link
Copy Markdown
Member

A threshold checked one thing: the value metric of every variant of its measure.
A benchmark now reports one variant per distinct parameters and as many
metrics as its harness measured, and neither was addressable. A project that
wanted p99 watched on the one configuration where it matters had nowhere to say
so.

The model

A threshold gains two optional fields.

metric is the name it checks. A threshold that names none checks the conventional
value name: a threshold always checks exactly one name, and a bare one never checks
all of them.

parameters is a filter over variants: a list of partial parameters, an OR across
the list and a subset match within each entry. A filter names only the keys it cares
about, so [{"size": 512}] matches a variant that also pins threads. A
threshold with no filter checks every variant.

Both defaults are what every threshold already does, so no existing row moves and no
project's alert volume changes.

Every matching threshold fires

There is no winner: not in checking, not in display. A variant that a bare
threshold and a filtered threshold both match earns a boundary from each and, on a
regression, an alert from each. Two thresholds asked for that row to be watched, so
two of them are told about it. This is deliberate and it is pinned by a test rather
than smoothed over.

Identity, and the null semantics

Identity is the three dimensions plus the two new fields, under the null semantics
they carry. SQLite treats nulls as distinct in a unique index, so two bare
thresholds on one branch, testbed, and measure would no longer collide under a plain
unique key over the five columns, and an explicit value would sit beside an absent
one.

The key is declared over the effective values instead:

CREATE UNIQUE INDEX index_threshold_dimensions ON threshold(
    branch_id,
    testbed_id,
    COALESCE(parameters, x''),
    measure_id,
    COALESCE(metric, 'value')
);

x'' is a value no stored filter can take, because a filter that matches everything
is stored as NULL.

The wire canonicalizes into those stored values. An explicit value and an absent
name are one threshold. An empty filter, a filter holding the empty entry, and an
absent filter are one threshold, because empty parameters are a subset of every
variant's. A filter has one spelling, because its entries sort by their RFC 8785
canonical bytes, duplicates collapse, and an entry another entry already covers is
dropped: [{"a": 1}, {"a": 1.0}] and [{"a": 1}, {"a": 1, "b": 2}] are both the one
entry [{"a": 1}], because every variant the wider entry matches the narrower one
matches too. The empty entry rule is that rule's special case.
The wire accepts any order and any spelling; the response returns the canonical form.

The sample keys on the name

The historical query behind detection filters on the threshold's metric name, so a
threshold on p99 is tested against p99 rows and never against the value rows
beside them. The per variant separation is unchanged: a threshold still samples
one variant's history and never the benchmark's variants pooled.

The boundary key

boundary keys on (metric_id, threshold_id) rather than on metric_id alone,
because a metric row may now carry a boundary per threshold that checked it. Both
threshold and boundary are rebuilt, each with its unique keys built after the
copy rather than declared on the table, which is what keeps the rebuild at the cost
of the scan. down.sql restores both old shapes, and because the old schema has no
shape for a threshold that names a metric or checks only some variants, it deletes
every such threshold with its models, boundaries, and alerts rather than flattening
it into a bare one.

The rebuilt boundary carries an index on threshold_id, so the foreign key check
that deleting a threshold runs over its boundaries is a seek rather than a walk of
the whole table. It carries one on model_id for the same reason: the delete
cascades to the threshold's models, and each of those is checked against boundary
in turn. The migration also indexes report_benchmark(variant_id, report_id), so the
historical query behind detection reads the reports of the one variant it samples
rather than every variant of the benchmark, and drops the index on variant_id
alone that this one makes redundant.

The rebuilt threshold carries an index on model_id, so the foreign key check
that deleting a model runs over the thresholds is a seek as well. The migration also
indexes head_version(version_id): deleting the last report of a version deletes the
version, and both the read of which heads hold it and the cascade into
head_version now seek by version rather than walk the table. It indexes
head(start_point_id) too, so clearing the start point of the heads that named a
deleted head_version row is a seek as well.

Responses

JsonAlert gains value, the metric value the alert fired on, and its metric
triple becomes optional. The triple is a convention over the value name, so an
alert on any other name has none; every alert a threshold could raise before this
carries the triple exactly as it did, so no existing response changes. The checked
name is readable at alert.threshold.metric.

The base of this branch carried a debug_assert_eq! in into_json_for_report
holding that a checked row is always a point estimate. That is the invariant this
branch retires, so the assertion is removed by design rather than by accident, and
the comment in its place says so.

The deprecated singular threshold, boundary, and alert fields carry the bare
threshold's check and no other's, wherever they appear. That is precisely what a
caller from before named checking has always been shown: a row that only a named or
filtered threshold checks reports no check in them at all. Where a list of boundaries
is returned, it is ordered by the threshold UUID, which is creation order for a
UUIDv7 and deterministic for the UUIDv4 a threshold minted before the move to
UUIDv7. Nothing about that order is a ranking.

Every entry of that list names what its threshold checks: JsonThresholdModel gains
parameters and metric, both absent when the threshold takes the default, so a
reader looking at two boundaries on one row can tell the filtered one from the bare
one without a second request. The hand written ParameterFilter and ParameterSet
schemas now carry the caps they enforce, so a generated client can see them.

The in-report thresholds.models map is unchanged and still addresses the bare
threshold. A map that names a measure and a model says nothing about a name or a
variant, so it neither creates nor resets anything narrower. A threshold that
checks a name or only some variants is created through the thresholds endpoint.

@epompeii
epompeii force-pushed the u/ep/parameters-api/thresholds branch 2 times, most recently from abd44e8 to 70525c7 Compare August 26, 2026 10:19
@github-actions

github-actions Bot commented Aug 26, 2026 •

Copy link
Copy Markdown
Contributor

🐰 Bencher Report

ProjectBencher
Branchu/ep/parameters-api/thresholds
Testbedintel-v1
Click to view all benchmark results
BenchmarkLatencyBenchmark Result
microseconds (µs)
(Result Δ%)
Upper Boundary
microseconds (µs)
(Limit %)
Adapter::Json📈 view plot
🚷 view threshold
5.19 µs
(+7.20%)Baseline: 4.84 µs
5.87 µs
(88.40%)
Adapter::Magic (JSON)📈 view plot
🚷 view threshold
4.97 µs
(+5.92%)Baseline: 4.69 µs
5.58 µs
(88.97%)
Adapter::Magic (Rust)📈 view plot
🚷 view threshold
27.61 µs
(+4.61%)Baseline: 26.39 µs
29.78 µs
(92.72%)
Adapter::Rust📈 view plot
🚷 view threshold
4.70 µs
(+21.40%)Baseline: 3.87 µs
6.00 µs
(78.32%)
Adapter::RustBench📈 view plot
🚷 view threshold
4.68 µs
(+20.96%)Baseline: 3.87 µs
5.98 µs
(78.21%)
🐰 View full continuous benchmarking report in Bencher

@epompeii
epompeii force-pushed the u/ep/parameters-api/thresholds branch from 70525c7 to 5d16149 Compare August 26, 2026 12:18
@epompeii
epompeii force-pushed the u/ep/parameters-api/thresholds branch from 5d16149 to 1a3e5a0 Compare August 27, 2026 03:46
@epompeii
epompeii force-pushed the u/ep/parameters-api/thresholds branch from 1a3e5a0 to 6b139dc Compare August 27, 2026 05:11
@epompeii
epompeii force-pushed the u/ep/parameters-api/thresholds branch from 6b139dc to 099ed97 Compare September 17, 2026 04:48
@epompeii epompeii changed the title Gate any metric of any grid point, and let every matching threshold fire Check any metric of any variant, and let every matching threshold fire Sep 17, 2026
@epompeii

Copy link
Copy Markdown
Member Author

Review after the rebase onto devel

Rebased onto devel with the terminology sweep (grid point to variant, gate to check, named value to metric). Two independent reviews and an adjudication found no major findings in this layer.

Rebase changes:

  • Conflicts with devel's boundary_json and deprecated_check renames resolved: the bare threshold's singular fields are now carried as bare_check.
  • Devel's boundary history query is now the hand-built HistoryQuery that walks the head newest first. This layer binds the threshold's metric name into it in place of value, leaving devel's SQL text unchanged, and devel's own query test builds its fixture as a bare value threshold.
  • Threshold::gates is now Threshold::checks.
  • Test fixtures pin an explicit perf start_time, because devel's four week default window postdates the 2024 fixture dates.
  • into_report_results_json grew past the clippy line limit after the rebase, so its per row fold is extracted into push_result_row.

Cleanups applied from review: the namedgate fixture label, the create_threshold_with_identity test helper rename, and a narrowed comment in metric_migration.rs.

@epompeii
epompeii force-pushed the u/ep/parameters-api/thresholds branch from 099ed97 to 42be461 Compare September 18, 2026 06:06
@epompeii
epompeii force-pushed the u/ep/parameters-api/thresholds branch 2 times, most recently from 511bc6f to b8b2d7f Compare September 19, 2026 02:33
@epompeii
epompeii force-pushed the u/ep/parameters-api/thresholds branch from b8b2d7f to 08b5c13 Compare September 19, 2026 04:43
Base automatically changed from u/ep/parameters-api/alerts to devel September 19, 2026 06:18
@epompeii
epompeii force-pushed the u/ep/parameters-api/thresholds branch from 08b5c13 to c06012e Compare September 19, 2026 06:18
A threshold checked one thing: the `value` metric of every variant of its
measure. A benchmark now reports as many variants as it has sets of parameters and
as many metrics as its harness measured, and neither was addressable. A project that
wanted `p99` watched on the one configuration where it matters had nowhere to say
so.

A threshold gains two optional fields. `metric` is the name it checks, and a
threshold that names none checks the conventional `value` name: a threshold
always checks exactly one name and a bare one never checks all of them.
`parameters` is a filter over variants, a list of partial parameters that is an
OR across the list and a subset match within each entry, and a threshold with no
filter checks every variant. Both defaults are what every existing threshold
already does, so no existing row moves and no project's alert volume changes.

Every threshold that matches a metric row runs. There is no winner: not in
checking, not in display. A variant that a bare threshold and a filtered
threshold both match earns a boundary from each and, on a regression, an alert
from each. That is the design and it is pinned by a test, because a row that two
people asked to be watched is a row two people hear about.

Identity is the three dimensions plus the two new fields, under the null
semantics they carry. SQLite treats nulls as distinct in a unique index, so two
bare thresholds on one branch, testbed, and measure would no longer collide under
a plain unique key over the five columns. The key is declared over the effective
values instead, `COALESCE(metric, 'value')` and `COALESCE(parameters, x'')`, and
the wire canonicalizes into them: an explicit `value` and an absent name are one
threshold, an empty filter and an absent one are one threshold, and a filter has
one spelling because its entries sort by their RFC 8785 canonical bytes and
duplicates collapse.

A threshold checks the sample it names. The historical query behind detection
filters on the threshold's metric name, so a threshold on `p99` is tested against
`p99` rows and never against the `value` rows beside them, and the per variant
separation stays exactly as it was.

`boundary` keys on `(metric_id, threshold_id)` rather than on `metric_id` alone,
because a metric row may now carry a boundary per threshold that checked it. Both
tables are rebuilt with their unique keys built after the copy, which is what
keeps the rebuild at the cost of the scan.

`JsonAlert` gains `value`, the metric value the alert fired on, and its `metric`
triple becomes optional: the triple is a convention over the `value` name, so an
alert on any other name has none. Every alert that a threshold could raise before
this carries the triple exactly as it did. The checked name is readable at
`alert.threshold.metric`.

The deprecated singular `threshold`, `boundary`, and `alert` fields carry the
bare threshold's boundary and alert and no other threshold's, everywhere they
appear. That is precisely what a caller from before named thresholds has always
been shown: a row that only a named or filtered threshold checks reports no
threshold in them at all. Where a list of boundaries is returned, it is ordered
by threshold creation time, oldest first, with the UUID breaking a tie.

The in-report `thresholds.models` map is unchanged and still addresses the bare
threshold: a map that names a measure and a model says nothing about a name or a
variant, so it neither creates nor resets anything narrower.
@epompeii
epompeii force-pushed the u/ep/parameters-api/thresholds branch from c06012e to cbcc856 Compare September 19, 2026 06:27
- A threshold's dimensions read branch, testbed, parameters, measure, metric,
  everywhere both new columns sit beside the measure: the migration, the schema,
  the queryables and tuple selections, the JSON types, and the generated spec.
- ThresholdDimensions and ThresholdIdentity merge into one ThresholdDimensions,
  the unique key the index is named for, keeping the canonicalizing constructor.
- ReportResults.thresholds becomes threshold_cache.
- Boundaries are ordered by the threshold UUID alone, which is creation order for
  a UUIDv7, so the detector carries the UUID rather than the creation time.
- jsonb::Array gains tests, and SQLite's own jsonb() pins a filter's bytes.
- ParameterFilter moves to its own file.
The previous commit rewrote schema.rs wholesale from a partially migrated
database rather than moving the two threshold columns. Everything else the
generator dropped is put back, and the threshold table keeps the intended
order: branch_id, testbed_id, parameters, measure_id, metric.
…cannot hold

- The PR comment matched an alert on benchmark and measure alone, so a named
  threshold's alert badged the `value` cell. It now matches the threshold the
  deprecated singular fields describe, and a measure counts as checked when any
  of its metrics carries a boundary.
- `reset` docs say what reset reaches: the thresholds the `models` map can
  address, the ones with no parameters filter and no metric name.
- `down.sql` deletes every threshold the old schema has no shape for, with its
  alerts, boundaries, and models, rather than flattening it into a bare one.
- Tests: the singular fields when the bare threshold is the younger of two, the
  filter set cap at the API, a threshold naming an absent metric, and
  `ParameterFilter` unit tests beside the type.
- `QueryBoundary::from_metric_id` and `boundary_order` are gone; the boundary
  order is documented once, on the `boundaries` field.
…ery boundary

- A parameters filter drops any set another set of the list already covers, so
  `[{"a":1},{"a":1,"b":2}]` is `[{"a":1}]` and one filter has one spelling.
  The match all rule is that rule's special case and behaves as before.
- One branch, testbed, and measure carry at most MAX_THRESHOLDS_PER_MEASURE
  thresholds. The endpoint reports the limit; the count runs again inside the
  transaction that inserts, which is what every creation path shares.
- The migration is re-dated so it sorts after the variant migration.
- Every boundaries[] entry names the filter and the metric its threshold checks,
  and the hand written schemas carry the caps they enforce.
The cap was a 400 only on the thresholds endpoint. Report ingest and the start
point clone reached it through the write, where it became a 500 asking the user
to file an issue, and a branch at the cap stayed there because the clone keeps a
destination threshold it no longer gives a model. Creation is already rate
limited, so the cap comes out whole.

Also tested is_checked, and carried the reset wording into the eight translated
chunks the way the console asks.
The comments that narrated a tuple's column order, a query's ordering, or a
cache's purpose said what the code beside them already says. The threshold docs
belong to a later change, so the nine chunks, the nine pages, and the two skill
files go back to what devel carries.
The one metric endpoint ordered the joined rows to put the bare threshold's row
first and took one of them. SQLite re-plans the ordered statement on every
execution, and the sort over the join is dearer to plan than the pick is to make
in Rust, so the handler now loads the rows the metric joins to, one per threshold
that checked it, and picks the bare one itself.

The response is unchanged. When no bare threshold checked the row, the deprecated
singular fields stay absent, as before.
Deleting a threshold reads the boundaries that point at it, then cascades to
its models and reads the boundaries that point at each of those, and each model
delete reads the thresholds that point at that model. Without an index on
`boundary(threshold_id)`, `boundary(model_id)`, or `threshold(model_id)` those
foreign key checks walk the whole table while holding the write connection.
The delete stays refused while those boundaries exist; the indexes make every
check a seek.

Deleting the last report of a version deletes the version, which reads the
heads that hold it and cascades into `head_version`, and each `head_version`
row it deletes clears the start point of any head that names it. Those walked
`head_version` and `head`, which had no index leading on `version_id` or
`start_point_id`, so `head_version(version_id)` and `head(start_point_id)`
are indexed here as well.

The history query reads the series of one variant, so it pins
`report_benchmark(variant_id, report_id)` rather than the benchmark index,
which returned every variant of the benchmark for each report in the history
and filtered the rest away after the seek. That leaves
`report_benchmark(variant_id)` redundant, the way the benchmark column alone
became redundant once the benchmark pair existed, so it goes. The benchmark
pair stays: other readers seek by benchmark.

A test pins the exact index list of `threshold`, `boundary`,
`head_version`, and `head` after the migrations, since a missing foreign key
index only makes a delete slow and no behavior test can see it.
@epompeii
epompeii marked this pull request as ready for review September 23, 2026 04:43
@epompeii
epompeii merged commit 2e58545 into devel Sep 23, 2026
68 checks passed
@epompeii
epompeii deleted the u/ep/parameters-api/thresholds branch September 23, 2026 05:13

This branch was successfully deployed

1 active deployment
Cloudflare — 3dbb236e Deployed Sep 23, 2026 by epompeii via Build / Build API Docker #1657
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant