Skip to content

Every filter is a condition - #6

Merged
grahamwetzler merged 6 commits into
mainfrom
rule-conditions
Sep 12, 2026
Merged

grahamwetzler merged 6 commits into
mainfrom
rule-conditions

Conversation

@grahamwetzler

Copy link
Copy Markdown
Owner

"Limits and scope" was a collapsed block below each rule holding altitude,
distance, circling, the pass prediction and database membership — as though
they were a different kind of thing from a tag or an operator. They are not.
They are all filters, and they now sit in the same "Add a condition…" list, in
the same removable rows. There is no "Limits and scope" left.

Grouped as they are flown rather than as they are stored:

identity, from the feed icao, reg, icao_type, squawk
identity, from the database operator, type, cmpg, category, tags, listed
altitude and distance min_altitude_ft, max_altitude_ft, max_distance_nm
flight path circling, passes_within_nm, passes_within

Circling and the pass prediction share a row because they read the same recent
motion. withinLimits splits to match: matchesPosition and
matchesFlightPath.

all: true is gone

A rule with no conditions matches every aircraft. An empty rules list is
still silent — there is no rule to reach. The key survives in the struct only
to name its replacement; without it, a live deployment's next reload gets
KnownFields' "field all not found", which reads as a typo rather than a move.

Migration: delete all: true from any rule that has it. Nothing else in
alerts.yaml changes.

Two review findings, and one stale claim they turned up

  • The preview's "this can still match the live feed" note had lost the reason
    it existed. matchesPlane only ever sees the database side of reg and
    icao_type, so listed: true plus a type code no listed aircraft carries
    previewed as zero in red — when a live aircraft broadcasting it would alert.
    Squawk is not one of those fields: the preview strips it, so it can never
    be why a count was zero, only an excuse for one the other conditions caused.
  • max_distance_nm does not hide Mode-S-only traffic. Evaluate holds back
    anything without a position before any rule is asked, and validate requires
    the receiver coordinates, so HasDistance is always true by then. The claim
    was in the README and in a comment carried over from withinLimits.

Testing

go test ./... and go vet ./... clean. The UI was driven against a running
instance: all 12 conditions offered, grouped rows round-trip through save →
YAML → reload, passes_within without passes_within_nm still errors at save,
a rule stripped to nothing saves as a bare name, and each preview case above
was checked by hand.

🤖 Generated with Claude Code

https://claude.ai/code/session_01S5ML1fYwLryQsR5FXAQHNr

grahamwetzler and others added 5 commits September 12, 2026 10:32
A rule with no conditions now matches every aircraft, so `all: true` has no
job left. An empty rules *list* is still silent: there is no rule to reach.
The key survives in the struct only to name its replacement — KnownFields
would otherwise report it as a typo on a live deployment's next reload.

Split withinLimits into matchesPosition (altitude, distance from the
receiver) and matchesFlightPath (circling, predicted pass). Both read the
same recent motion, which is why circling and the prediction belong
together, and neither is a "limit" — they are conditions like any other.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S5ML1fYwLryQsR5FXAQHNr
Altitude, distance, circling, the pass prediction and database membership
were a separate collapsed block below the rule, as though they were a
different kind of thing. They are not: they are filters on the same footing
as a tag or an operator, and they now sit in the same "Add a condition…"
list, in the same removable rows.

Grouped as they are flown rather than as they are stored: "Flight path"
holds circling and the pass prediction, which read the same recent motion;
"Altitude and distance" holds where the aircraft is right now. Each carries
its own note about why the preview cannot answer it.

"Match every aircraft" is gone with the block. A rule with no conditions
says it by saying nothing, and the row where the conditions would be says so.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S5ML1fYwLryQsR5FXAQHNr
The README split the rule keys into "fields" and "limits" and gave the
wildcard its own key. Both are gone: one table of conditions grouped by what
they ask, and the wildcard is the rule that asks nothing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S5ML1fYwLryQsR5FXAQHNr
…e_nm

Two review findings on this branch.

canMatchFeed had lost the reason the old check existed. A rule keyed on reg,
icao_type or squawk can match an aircraft whose database row says otherwise —
matchesPlane can only see the database side of those — so listed: true plus a
type code no listed aircraft carries previewed as zero in red, when a live
aircraft broadcasting it would alert. Restored as the union of both reasons a
zero is inconclusive, which also covers the `icao`-only rule and the wildcard
that the old check missed.

And max_distance_nm does not hide Mode-S-only traffic: Evaluate holds back
anything without a position before any rule is asked, and validate requires
the receiver coordinates, so HasDistance is always true by then. The claim was
in the README and in the comment carried over from withinLimits; both go.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S5ML1fYwLryQsR5FXAQHNr
The preview strips squawk before matching, so a squawk can never be the
reason a count came back zero — only an excuse for one the rule's other
conditions caused. operator: ["Nonexistent Operator"] with squawk: ["7700"]
claimed the live feed could still satisfy it; nothing can, since no database
row carries that operator and an unlisted aircraft carries none at all.

Squawk rules that really are inconclusive are already covered by the second
branch, which asks whether the rule can match an aircraft with no row at all.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S5ML1fYwLryQsR5FXAQHNr

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a64297611b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ui.html
Comment on lines +573 to +575
if (cond.values) {
if (cond.values.length) rule[cond.dataset.key] = cond.values;
continue;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Warn before empty condition rows become wildcards

When a user adds a condition but leaves it unset—for example, an Operator row with no chips or an Altitude and distance row with blank inputs—readRule omits that row from the payload. The UI nevertheless hides the “no conditions” warning because a .cond element exists, so saving produces a name-only rule that now matches every positioned aircraft and can shadow all subsequent rules. Treat an empty row as visibly unstated, remove it before saving, or require confirmation before serializing the resulting wildcard.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Valid, and fixed in 04645f6.

Confirmed reachable: the note was keyed on whether a .cond element exists, while the payload is keyed on whether a row contributes a value to readRule. An empty row satisfies the first and not the second, so the warning hid itself about a rule that saves as the wildcard. Before this branch the same mis-step was a 400 from validate, so removing that check is what made it silent.

Fixed at the disagreement rather than at the symptom: the note now reads its state from readRule — the same function that builds the payload — so the two cannot drift apart again. Reworded to be true of a rule that has rows but states nothing, and to say why it matters: "No conditions stated — this rule matches every aircraft, so no rule below it is ever reached. Fill one in to narrow it."

Verified against a running instance, six states, note vs. payload:

note payload
bare new rule shown {name}
+ empty Operator row shown {name}
Operator filled hidden {name, operator}
chip removed again shown {name}
+ blank Altitude row shown {name}
distance filled hidden {name, max_distance_nm}

Not removing the empty row on save: it is the row the user is mid-way through filling in, and deleting it under them to prevent a wildcard they can now see warned about would be the worse trade.

The warning was keyed on whether a condition row exists on screen; the payload
is keyed on whether a row contributes a value. Add a condition and leave it
blank and the two disagree: the warning hides itself, and Save writes a
name-only rule that matches every aircraft and shadows every rule below it.
Before this branch the same mis-step was a 400, so it was at least loud.

Reading the warning off readRule — the function that builds the payload — is
what keeps them from disagreeing again. Reworded to be true of a rule that has
rows but states nothing, and to say why it matters.

Also the third copy of the Mode-S claim removed in 4872088, in the altitude
and distance hint.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S5ML1fYwLryQsR5FXAQHNr
@grahamwetzler
grahamwetzler merged commit f03c9a2 into main Sep 12, 2026
2 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