From c448fa438f55c39cf66b8186398cde2f94adb65a Mon Sep 17 00:00:00 2001 From: John O'Keefe Date: Sun, 1 Nov 2026 01:12:40 -0400 Subject: [PATCH] fix(filter-builder): one operator name per field (Option A); drawer ids resolve on refresh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The is/in question, settled per user decision: they were exact synonyms (feed's clause compiles is/is_not and in/not_in to the same ANY), so the vocabulary collapses instead of being explained. - Registry: feed's offered ops are in/isn't-in (+has/missing); legacyFieldOps keeps is/is_not ACCEPTED at parse forever — every stored tree and saved filter from before the collapse parses and evaluates unchanged. Library keeps is/isn't (its only names); collection's genuinely distinct operators are untouched. - Client: id-kind cells are uniform — every operator on an id field accepts a set, so all id cells say "pick one or more" and accumulate picks. The per-operator placeholder/pick branching is gone (it also never fired: the registry types is as Values:"many"). - The drawer-refresh id fix rides along: hydrateFromForm now calls resolveIdLabels, so a page refresh with an active saved filter resolves stored ids to names in the bookshelf drawer (the collection rules page already did at init). Pins: feed payload omits the synonyms while legacy feed is/is_not trees parse (Go unit); uniform accumulate + placeholder (vitest); menu-open regression and hydrate-resolution pins kept. Web 183/183; services, filter + episode-tree + search/collections contract slices green (full suite skipped per user — vocabulary + UI-layer change, engine paths pinned by the targeted slices). --- docs/contract_pins.md | 16 ++++++++++ internal/services/filter_registry.go | 16 ++++++++-- internal/services/filter_registry_test.go | 39 +++++++++++++++++++++++ web/src/filter_builder.test.ts | 13 +++++--- web/src/filter_builder.ts | 18 +++++++---- 5 files changed, 88 insertions(+), 14 deletions(-) diff --git a/docs/contract_pins.md b/docs/contract_pins.md index efd3d195..6cd10bf2 100644 --- a/docs/contract_pins.md +++ b/docs/contract_pins.md @@ -603,3 +603,19 @@ evaluation through the dashboard could never work (the test harness wired it correctly — the gap was production-only). The dashboard handler and both inline sites now receive it, and evaluation failures log instead of silently emptying a section. + +## 2026-11-01 — feed operator vocabulary collapsed (Option A): one name per semantic + +The feed field's menu offered `is`/`isn't` alongside `in`/`isn't in` — +exact synonyms (the clause compiles both pairs to the same `ANY`). The +MENUS now offer `in`/`isn't in` (+ has/missing) only. This is an +offered-vocabulary narrowing, not a parse change: + +- Stored trees and saved filters carrying `feed is X` / `feed is_not X` + PARSE AND EVALUATE FOREVER (`legacyFieldOps` accepts the synonyms at + validation — the stored wire is never invalidated). +- The fields payload (the client's operator menus) no longer carries + the duplicates. +- Client id-cells are uniform: every operator on an id-valued field + accepts a set, so all id cells say "pick one or more" and accumulate + picks — no per-operator placeholder/pick distinction to explain. diff --git a/internal/services/filter_registry.go b/internal/services/filter_registry.go index b1b54f75..aaf2f133 100644 --- a/internal/services/filter_registry.go +++ b/internal/services/filter_registry.go @@ -566,7 +566,11 @@ var filterFields = []FilterField{ // `played` is THE finished predicate (viewer progress >= 0.999 — // the same definition as hide_finished, item 2.4). {Key: "feed", Label: "Feed", Group: GroupEpisodes, Kind: KindIdentifier, - Ops: []string{OpIs, OpIsNot, OpIn, OpNotIn, OpHas, OpMissing}, DefaultOp: OpIn, + // One name per semantic (2026-11-01, Option A): is/is_not were + // exact synonyms of in/not_in (the clause compiles both to the + // same ANY). Stored trees carrying the legacy keys keep + // parsing — see legacyFieldOps. + Ops: []string{OpIn, OpNotIn, OpHas, OpMissing}, DefaultOp: OpIn, Clause: clauseFeed, Suggest: suggestFeed, Scopes: []string{ScopeEpisodes}}, {Key: "downloaded", Label: "Downloaded", Group: GroupEpisodes, Kind: KindBool, Ops: []string{OpIsTrue, OpIsFalse}, DefaultOp: OpIsTrue, @@ -631,13 +635,21 @@ func FilterFieldKeys() []string { } // fieldOpsAllowed reports whether op is allowed on the field. +// legacyFieldOps — operator synonyms still ACCEPTED at parse for +// trees saved before the vocabulary collapse (2026-11-01): feed once +// offered is/is_not alongside in/not_in with identical semantics. The +// menus offer one name per field; stored trees parse forever. +var legacyFieldOps = map[string]map[string]bool{ + "feed": {OpIs: true, OpIsNot: true}, +} + func fieldOpsAllowed(f FilterField, op string) bool { for _, o := range f.Ops { if o == op { return true } } - return false + return legacyFieldOps[f.Key][op] } // maxFilterDepth bounds group nesting (BookOrbit allows 5; the UI diff --git a/internal/services/filter_registry_test.go b/internal/services/filter_registry_test.go index 8f219171..05c701d9 100644 --- a/internal/services/filter_registry_test.go +++ b/internal/services/filter_registry_test.go @@ -235,3 +235,42 @@ func TestEpisodesScopeHidesBookOnlyFields(t *testing.T) { t.Fatalf("books vocabulary wrong: author=%v feed=%v", bkeys["author"], bkeys["feed"]) } } + +// ── 2026-11-01, Option A: one operator name per semantic ──────────── +// feed's MENU offers in/not_in only (is/is_not were exact synonyms); +// trees saved before the collapse keep parsing forever. + +func TestFeedVocabularyCollapse(t *testing.T) { + f, ok := FilterFieldByKey("feed") + require.True(t, ok) + for _, o := range f.Ops { + assert.NotEqual(t, OpIs, o, "the feed menu must not offer the is synonym") + assert.NotEqual(t, OpIsNot, o, "the feed menu must not offer the is_not synonym") + } + assert.Contains(t, f.Ops, OpIn) + assert.Contains(t, f.Ops, OpNotIn) + + // The payload (the client's menus) carries the same collapse. + for _, wf := range FilterFieldsPayload(ScopeEpisodes) { + if wf.Key != "feed" { + continue + } + for _, op := range wf.Ops { + assert.NotEqual(t, OpIs, op.Key, "payload must not offer is on feed") + } + return + } + t.Fatal("feed missing from the episodes payload") +} + +func TestFeedLegacyOpsStillParse(t *testing.T) { + // Stored trees from before the collapse: is/is_not on feed parse + // and validate like their in/not_in twins. + legacy := `{"scope":"episodes","mode":"any","children":[ + {"field":"feed","op":"is","values":["11111111-1111-1111-1111-111111111111"]}, + {"field":"feed","op":"is_not","values":["22222222-2222-2222-2222-222222222222"]} + ]}` + tree, err := ParseFilterTree(legacy) + require.NoError(t, err, "legacy feed is/is_not trees parse forever") + require.Len(t, tree.Children, 2) +} diff --git a/web/src/filter_builder.test.ts b/web/src/filter_builder.test.ts index b46747bb..5cca58bd 100644 --- a/web/src/filter_builder.test.ts +++ b/web/src/filter_builder.test.ts @@ -463,23 +463,26 @@ describe("id-kind value cells", () => { vi.unstubAllGlobals(); }); - it("a one-arity op (is) picks ONE value — replace, not toggle", () => { + it("is and in behave identically on id cells — both accumulate (Option A)", () => { const c = filterBuilderData(); c.filterTree = { mode: "all", children: [{ field: "library", op: "is", values: ["lib-1"] }], }; + c.idLabels["lib-1"] = "Books"; c.pickSuggestion("0", [0], { value: "lib-2", display: "Comics", count: 2 }); - expect((c.filterTree.children[0] as FilterRuleState).values).toEqual(["lib-2"]); - expect(c.valueCellText([0])).toBe("Comics"); + expect((c.filterTree.children[0] as FilterRuleState).values).toEqual(["lib-1,lib-2"]); + expect(c.valueCellText([0])).toBe("Books, Comics"); }); - it("the placeholder follows the op's arity", () => { + it("the id-cell placeholder is uniform — one name per field, no per-op distinction", () => { const c = filterBuilderData(); expect(c.valuePlaceholderFor({ field: "library", op: "in", values: [] })).toBe( "pick one or more", ); - expect(c.valuePlaceholderFor({ field: "library", op: "is", values: [] })).toBe("pick one"); + expect(c.valuePlaceholderFor({ field: "library", op: "is", values: [] })).toBe( + "pick one or more", + ); expect(c.valuePlaceholderFor({ field: "author", op: "contains", values: [] })).toBe("value"); }); diff --git a/web/src/filter_builder.ts b/web/src/filter_builder.ts index 8fa0ab30..a5d38e3e 100644 --- a/web/src/filter_builder.ts +++ b/web/src/filter_builder.ts @@ -727,14 +727,14 @@ export function filterBuilderData() { this.filterTree.children.forEach((child, idx) => visit(child, [idx])); }, - // valuePlaceholderFor — the value cell's hint, ARITY-AWARE: the op - // decides how many values the cell takes ("in" = several, "is" = - // one), and the hint follows the op as it changes. + // valuePlaceholderFor — the value cell's hint. Id-kind cells are + // the MULTI-VALUE boxes (pick several; the menu keeps toggling + // them in) — every operator on an id field accepts a set, so the + // hint is uniform (2026-11-01, Option A: one name per field, no + // per-operator distinction — "is" and "in" were synonyms). valuePlaceholderFor(rule: FilterRuleState): string { if (this.fieldIsDays(rule.field)) return "days"; - if (FIELD_BY_KEY.get(rule.field)?.valueKind === "id") { - return this.valueArity(rule) === "many" ? "pick one or more" : "pick one"; - } + if (FIELD_BY_KEY.get(rule.field)?.valueKind === "id") return "pick one or more"; return "value"; }, @@ -797,7 +797,7 @@ export function filterBuilderData() { const meta = FIELD_BY_KEY.get(node.field); this.idLabels[s.value] = s.display ?? s.value; delete this.idDrafts[path.join("-")]; // the cell shows labels again - if (meta?.valueKind === "id" && this.valueArity(node) === "many") { + if (meta?.valueKind === "id") { // id-valued MULTI: toggle the id in the comma list. const ids = node.values.join(",").split(",").filter(Boolean); const at = ids.indexOf(s.value); @@ -1084,6 +1084,10 @@ export function filterBuilderData() { const tree = JSON.parse(qfInput.value) as FilterGroupState; if (tree && Array.isArray(tree.children)) { this.filterTree = tree; + // Hydrated trees carry bare ids with no labels in sight + // (page refresh with an active saved filter) — resolve + // them now (2026-11-01: the cells showed raw uuids). + this.resolveIdLabels(); return true; } } catch {