fix(filter-builder): one operator name per field (Option A); drawer ids resolve on refresh
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).
This commit is contained in:
1 parent
81e54f89b2
commit
c448fa438f
5 files changed
+88
-14
No files matched your search
@@ -603,3 +603,19 @@ evaluation through the dashboard could never work (the test harness
|
|||||||
wired it correctly — the gap was production-only). The dashboard
|
wired it correctly — the gap was production-only). The dashboard
|
||||||
handler and both inline sites now receive it, and evaluation failures
|
handler and both inline sites now receive it, and evaluation failures
|
||||||
log instead of silently emptying a section.
|
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.
|
||||||
@@ -566,7 +566,11 @@ var filterFields = []FilterField{
|
|||||||
// `played` is THE finished predicate (viewer progress >= 0.999 —
|
// `played` is THE finished predicate (viewer progress >= 0.999 —
|
||||||
// the same definition as hide_finished, item 2.4).
|
// the same definition as hide_finished, item 2.4).
|
||||||
{Key: "feed", Label: "Feed", Group: GroupEpisodes, Kind: KindIdentifier,
|
{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}},
|
Clause: clauseFeed, Suggest: suggestFeed, Scopes: []string{ScopeEpisodes}},
|
||||||
{Key: "downloaded", Label: "Downloaded", Group: GroupEpisodes, Kind: KindBool,
|
{Key: "downloaded", Label: "Downloaded", Group: GroupEpisodes, Kind: KindBool,
|
||||||
Ops: []string{OpIsTrue, OpIsFalse}, DefaultOp: OpIsTrue,
|
Ops: []string{OpIsTrue, OpIsFalse}, DefaultOp: OpIsTrue,
|
||||||
@@ -631,13 +635,21 @@ func FilterFieldKeys() []string {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// fieldOpsAllowed reports whether op is allowed on the field.
|
// 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 {
|
func fieldOpsAllowed(f FilterField, op string) bool {
|
||||||
for _, o := range f.Ops {
|
for _, o := range f.Ops {
|
||||||
if o == op {
|
if o == op {
|
||||||
return true
|
return true
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
return false
|
return legacyFieldOps[f.Key][op]
|
||||||
}
|
}
|
||||||
|
|
||||||
// maxFilterDepth bounds group nesting (BookOrbit allows 5; the UI
|
// maxFilterDepth bounds group nesting (BookOrbit allows 5; the UI
|
||||||
|
|||||||
@@ -235,3 +235,42 @@ func TestEpisodesScopeHidesBookOnlyFields(t *testing.T) {
|
|||||||
t.Fatalf("books vocabulary wrong: author=%v feed=%v", bkeys["author"], bkeys["feed"])
|
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)
|
||||||
|
}
|
||||||
@@ -463,23 +463,26 @@ describe("id-kind value cells", () => {
|
|||||||
vi.unstubAllGlobals();
|
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();
|
const c = filterBuilderData();
|
||||||
c.filterTree = {
|
c.filterTree = {
|
||||||
mode: "all",
|
mode: "all",
|
||||||
children: [{ field: "library", op: "is", values: ["lib-1"] }],
|
children: [{ field: "library", op: "is", values: ["lib-1"] }],
|
||||||
};
|
};
|
||||||
|
c.idLabels["lib-1"] = "Books";
|
||||||
c.pickSuggestion("0", [0], { value: "lib-2", display: "Comics", count: 2 });
|
c.pickSuggestion("0", [0], { value: "lib-2", display: "Comics", count: 2 });
|
||||||
expect((c.filterTree.children[0] as FilterRuleState).values).toEqual(["lib-2"]);
|
expect((c.filterTree.children[0] as FilterRuleState).values).toEqual(["lib-1,lib-2"]);
|
||||||
expect(c.valueCellText([0])).toBe("Comics");
|
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();
|
const c = filterBuilderData();
|
||||||
expect(c.valuePlaceholderFor({ field: "library", op: "in", values: [] })).toBe(
|
expect(c.valuePlaceholderFor({ field: "library", op: "in", values: [] })).toBe(
|
||||||
"pick one or more",
|
"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");
|
expect(c.valuePlaceholderFor({ field: "author", op: "contains", values: [] })).toBe("value");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -727,14 +727,14 @@ export function filterBuilderData() {
|
|||||||
this.filterTree.children.forEach((child, idx) => visit(child, [idx]));
|
this.filterTree.children.forEach((child, idx) => visit(child, [idx]));
|
||||||
},
|
},
|
||||||
|
|
||||||
// valuePlaceholderFor — the value cell's hint, ARITY-AWARE: the op
|
// valuePlaceholderFor — the value cell's hint. Id-kind cells are
|
||||||
// decides how many values the cell takes ("in" = several, "is" =
|
// the MULTI-VALUE boxes (pick several; the menu keeps toggling
|
||||||
// one), and the hint follows the op as it changes.
|
// 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 {
|
valuePlaceholderFor(rule: FilterRuleState): string {
|
||||||
if (this.fieldIsDays(rule.field)) return "days";
|
if (this.fieldIsDays(rule.field)) return "days";
|
||||||
if (FIELD_BY_KEY.get(rule.field)?.valueKind === "id") {
|
if (FIELD_BY_KEY.get(rule.field)?.valueKind === "id") return "pick one or more";
|
||||||
return this.valueArity(rule) === "many" ? "pick one or more" : "pick one";
|
|
||||||
}
|
|
||||||
return "value";
|
return "value";
|
||||||
},
|
},
|
||||||
|
|
||||||
@@ -797,7 +797,7 @@ export function filterBuilderData() {
|
|||||||
const meta = FIELD_BY_KEY.get(node.field);
|
const meta = FIELD_BY_KEY.get(node.field);
|
||||||
this.idLabels[s.value] = s.display ?? s.value;
|
this.idLabels[s.value] = s.display ?? s.value;
|
||||||
delete this.idDrafts[path.join("-")]; // the cell shows labels again
|
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.
|
// id-valued MULTI: toggle the id in the comma list.
|
||||||
const ids = node.values.join(",").split(",").filter(Boolean);
|
const ids = node.values.join(",").split(",").filter(Boolean);
|
||||||
const at = ids.indexOf(s.value);
|
const at = ids.indexOf(s.value);
|
||||||
@@ -1084,6 +1084,10 @@ export function filterBuilderData() {
|
|||||||
const tree = JSON.parse(qfInput.value) as FilterGroupState;
|
const tree = JSON.parse(qfInput.value) as FilterGroupState;
|
||||||
if (tree && Array.isArray(tree.children)) {
|
if (tree && Array.isArray(tree.children)) {
|
||||||
this.filterTree = tree;
|
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;
|
return true;
|
||||||
}
|
}
|
||||||
} catch {
|
} catch {
|
||||||
|
|||||||
Reference in new issue
Block a user