From 4e2183d9a3fb6aafbe2c7470e767f71d8a03551d Mon Sep 17 00:00:00 2001 From: Timothy Date: Thu, 23 Jul 2026 18:40:12 +0200 Subject: [PATCH 01/19] =?UTF-8?q?design(434,435,438):=20RuleBuilder=20bund?= =?UTF-8?q?le=20spec=20=E2=80=94=20validation,=20relative-date=20operators?= =?UTF-8?q?,=20facet=20typeahead?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.8 (1M context) --- .../2026-07-23-rulebuilder-bundle-design.md | 125 ++++++++++++++++++ 1 file changed, 125 insertions(+) create mode 100644 docs/superpowers/specs/2026-07-23-rulebuilder-bundle-design.md diff --git a/docs/superpowers/specs/2026-07-23-rulebuilder-bundle-design.md b/docs/superpowers/specs/2026-07-23-rulebuilder-bundle-design.md new file mode 100644 index 000000000..bc4ad1ca6 --- /dev/null +++ b/docs/superpowers/specs/2026-07-23-rulebuilder-bundle-design.md @@ -0,0 +1,125 @@ +# RuleBuilder bundle — design (#438 + #435 + #434) + +**Date:** 2026-07-23 +**Issues:** #438 (validation & polish), #435 (relative-date operators), #434 (facet-value typeahead) +**Branch:** `feat/rulebuilder-bundle` +**Left open:** #436 (deeper nesting — self-labeled YAGNI), #437 (inline RuleBuilder adoption) + +The visual rule builder (`web/src/builder/rules/`, landed #176 / PR #433) compiles a visual rule tree +to a Lucene query string and parses it back, with a lossless-round-trip contract. It has one consumer +today: `CollectionsScreen.tsx` (SmartCollection create/edit, mounted at line ~347). Convention doc: +`docs/spa-conventions.md` **§12** (not §11 — #437's citation is wrong and will be corrected when #437 +is worked). + +## Slicing & workflow + +- **One feature branch / one PR** closing all three (`fixes #438 #435 #434`). They share + `RuleBuilder.tsx` (and #435/#438 share `types.ts`/`compile.ts`/`parse.ts`) too heavily to split + without rebase churn, and read as one cohesive polish sweep. +- **#434's backend endpoint is the only disjoint slice** — delegated to a parallel worktree agent + (branched off the feature branch, merged back early) so its C# + OpenAPI regen runs while the + frontend is built. **No parallel agents over the shared frontend module** (the one-file fan-out + trap). +- Everything else is **sequential, single committing agent** in the main worktree: + **#438 → #435 → #434-combobox** (combobox last, after the endpoint's generated API types exist). +- Independent adversarial review over the whole diff before push (new API endpoint + >~150 lines). + Live-E2E on the SmartCollection screen for the round-trip. + +## #438 — validation & polish (frontend only) + +Confirmed current behavior (compile.ts): +- `between` with empty `value2` emits `field:[v TO ]` (malformed). — compile.ts:45 +- empty `contains` → `field:**`; empty `startsWith` → `field:*`. — compile.ts:33–36 +- single-child group drops its connective (`join` over one element). — compile.ts:52–57 +- `isNot`/`notMatches` → `NOT field:"v"`, which matches nothing alone (no MatchAllDocs). — compile.ts:31 + +Fixes: +1. **`between` requires both bounds.** A `between` rule with empty `value` or `value2` is *invalid* — + surface a per-rule validation error and block compile/save. Never emit `field:[v TO ]`. +2. **Empty text values are invalid.** `contains`/`startsWith`/`is`/`isNot` with an empty `value` are + incomplete (not a wildcard) — same per-rule validation error; block compile/save. Never emit + `field:*` or `field:**`. +3. **Single-child group round-trips its connective.** Fix on the **parser** side: a compiled + single-child group parses back to the builder's default `match` so a 1-child `{match:'any'}` no + longer reopens as `'all'`. Output (compile) is unchanged; this is a parse-side alignment only. +4. **Pure-negative top group → non-blocking warning.** When every child of the *top* group is negative + (`isNot`/`notMatches`), show a warning (a `NOT`-only query matches nothing) and rely on the live + preview count. **Do not block** — a positive branch nested deeper can legitimately rescue it, so a + hard block would create false negatives. (Decision confirmed with user 2026-07-23.) + +Validation surface: a per-rule "incomplete" flag rendered inline in `RuleBuilder.tsx`, and an +aggregate "has-errors" boolean the consumer (`CollectionsScreen`) uses to disable Save. Compile of an +invalid tree returns a sentinel (empty / null) rather than a malformed string. + +Tests: extend `compile.test.ts` / `parse.test.ts` / `roundtrip.test.ts` for each case (cases 1–3 are +hard assertions; case 4 asserts the warning predicate, not a block). + +## #435 — relative-date operators (frontend only — backend macros already exist) + +The Lucene macros exist in `ErsatzTV.Infrastructure/Search/CustomMultiFieldQueryParser.cs`: +`released_inthelast` / `released_notinthelast` / `added_inthelast` / `added_notinthelast`, value form +`" day|week|month|year"` (unit substring-matched; negated from `DateTime.Today`). **No C# change** — +they are absent from `SearchFieldCatalog` only because they are query-time synthetic fields. (There is +no `inthenext` macro, so "in the next N" is out of scope — matches the issue.) + +- **types.ts:** add operators `inLast` and `notInLast`, offered only on `date` fields (extend + `OPERATORS_BY_TYPE.date`). Add optional `unit?: 'day' | 'week' | 'month' | 'year'` to `Rule` (cleaner + than overloading `value2`, which is the `between` upper bound). `value` holds the integer N. +- **RuleBuilder.tsx:** for `inLast`/`notInLast`, render a number input + a unit `` for an autocomplete combobox (debounced query to the endpoint). Purely + additive — the emitted value and compile/parse are unchanged. Falls back to free-text when the + endpoint returns nothing (so a typo still compiles, keeping the preview-count safety net). + +## Docs updated in-PR + +| Doc | Change | +|---|---| +| `api-conventions.md` | checklist entry for the new `search/fields/{name}/values` endpoint | +| `v1.json` / `v1.d.ts` / `endpoint-index.md` | regenerated (#434 endpoint) | +| `spa-conventions.md` §12 | relative-date operators (#435) + typeahead combobox (#434) behavior | +| `docs/decisions.md` | record for the #434 distinct-values endpoint; record for the #435 relative-date field-mapping convention | +| `docs/blazor-route-parity.md` | not required (no route change) | + +## Out of scope / deferred + +- **#436** (deeper group nesting): `compile.ts` already recurses arbitrarily deep; the cap is solely + `parse.ts:121` passing `false`. Left open — self-labeled YAGNI until a real query needs it. +- **#437** (inline RuleBuilder adoption in ChannelBuilder + Auto-Tune): larger integration + live-E2E, + its own session. Note for then: cite `spa-conventions.md` **§12**, not §11. +- **`does not contain` / date `is`**: these operators do not exist in the set and are not being added + (only #435's `inLast`/`notInLast`). -- 2.47.3 From 5618ad12eacaad8605c68691805a9b322d1e3905 Mon Sep 17 00:00:00 2001 From: Timothy Date: Thu, 23 Jul 2026 18:49:30 +0200 Subject: [PATCH 02/19] plan(434,435,438): RuleBuilder bundle implementation plan Co-Authored-By: Claude Opus 4.8 (1M context) --- .../plans/2026-07-23-rulebuilder-bundle.md | 655 ++++++++++++++++++ 1 file changed, 655 insertions(+) create mode 100644 docs/superpowers/plans/2026-07-23-rulebuilder-bundle.md diff --git a/docs/superpowers/plans/2026-07-23-rulebuilder-bundle.md b/docs/superpowers/plans/2026-07-23-rulebuilder-bundle.md new file mode 100644 index 000000000..b84c45175 --- /dev/null +++ b/docs/superpowers/plans/2026-07-23-rulebuilder-bundle.md @@ -0,0 +1,655 @@ +# RuleBuilder Bundle Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Ship #438 (validation & polish), #435 (relative-date operators), and #434 (facet-value typeahead) for the visual rule builder on one feature branch. + +**Architecture:** Pure-logic changes land in the small `web/src/builder/rules/` modules (types/compile/parse + two new helper modules `validation.ts`, `dateMacro.ts`) under TDD. UI changes to `RuleBuilder.tsx` consume those helpers. #434 adds a disjoint C# read endpoint (Lucene term enumeration) built in a parallel worktree; its combobox is wired last, after the generated API types exist. + +**Tech Stack:** React 18 + TypeScript + Vite (vitest for web tests); .NET 10 / C# / MediatR / Lucene.NET (`ISearchIndex`); NUnit + Shouldly + NSubstitute for C# tests. + +## Global Constraints + +- Work in worktree `feat/rulebuilder-bundle` off `origin/main`. Never commit in `/Users/timothy/ersatztv`. → `process.shared-tree-readonly` +- Backend #434 slice runs in its OWN worktree branched off `feat/rulebuilder-bundle`, merged back by fast-forward/plumbing. All frontend work is one committing agent, sequential (shared `RuleBuilder.tsx`). → `process.one-worktree-one-committing-agent`, `process.foreign-worktree-plumbing-merge` +- The compile↔parse **lossless round-trip contract** (spa-conventions §12) must hold: `parse(compile(g)) ≡ normalizeGroup(g)` for every valid `g`. Extend `roundtrip.test.ts` for every new operator. +- New REST response DTOs live in `ErsatzTV.Core/Api/Search/*ResponseModel.cs` with file-scoped `#nullable enable`. → `api.response-dtos` +- A PR touching `ErsatzTV/Controllers/Api/**` or `ErsatzTV.Core/Api/**` MUST ship regenerated `v1.json` / `v1.d.ts` / `endpoint-index.md` in the same diff. → `release.api-contract-ci-gate` +- Before any push touching `.cs`: BOM-check the touched set (`xxd -p | grep -c ^efbbbf`) and run the format gate under `bash -c`. → `process.bom-format-detection-recipe` +- Independent adversarial review over the whole diff before push (new API endpoint + >~150 lines). → `process.independent-review-rubric` +- Commit trailer on every commit: `Co-Authored-By: Claude Opus 4.8 (1M context) `. Worktree hooks: commit with `--no-verify` and run gates manually (worktree husky friction). + +**Test commands** (run from the worktree): +- Web unit: `cd web && npx vitest run src/builder/rules/.test.ts` +- Web all rules: `cd web && npx vitest run src/builder/rules` +- C#: `dotnet test ErsatzTV.Application.Tests --filter ` (or the relevant test project) + +--- + +## Task 1: `#438`/`#435` — extend types, then rule/group validation module (`validation.ts`) + +**Files:** +- Modify: `web/src/builder/rules/types.ts` (extend first, so validation typechecks) +- Create: `web/src/builder/rules/validation.ts` +- Test: `web/src/builder/rules/validation.test.ts` + +- [ ] **Step 0: Extend types.ts** (the new operators/unit are consumed first by validation's tests): + +```ts +export type Operator = + | 'is' | 'isNot' | 'contains' | 'startsWith' // text + | 'matches' | 'notMatches' // fulltext + | 'eq' | 'gt' | 'lt' | 'between' // number + | 'before' | 'after' // date (absolute) + | 'inLast' | 'notInLast'; // date (relative, #435) + +export type DateUnit = 'day' | 'week' | 'month' | 'year'; + +export interface Rule { + field: string; + operator: Operator; + value: string; + value2?: string; // upper bound for `between` + unit?: DateUnit; // unit for `inLast`/`notInLast` (#435) +} +``` + +and extend the date row of `OPERATORS_BY_TYPE`: + +```ts + date: ['before', 'after', 'between', 'inLast', 'notInLast'] +``` + +**Interfaces:** +- Produces: + - `ruleError(rule: Rule): string | null` — a human string when the rule is incomplete, else null. + - `groupHasErrors(group: Group): boolean` — true if any descendant rule has an error. + - `topGroupAllNegative(group: Group): boolean` — true when every direct child of the *top* group is a negative rule (`isNot`/`notMatches`) (warning predicate, non-blocking). + - `normalizeGroup(group: Group): Group` — returns a copy with `match` coerced to `'all'` for every group (recursively) that has fewer than 2 children (single-child connective is meaningless and cannot round-trip). + +- [ ] **Step 1: Write the failing test** + +```ts +// web/src/builder/rules/validation.test.ts +import { describe, expect, it } from 'vitest'; +import { groupHasErrors, normalizeGroup, ruleError, topGroupAllNegative } from './validation'; +import type { Group, Rule } from './types'; + +const r = (o: Partial): Rule => ({ field: 'title', operator: 'is', value: 'x', ...o }); + +describe('ruleError', () => { + it('flags between with a missing upper bound', () => { + expect(ruleError(r({ field: 'minutes', operator: 'between', value: '30', value2: '' }))).toBeTruthy(); + expect(ruleError(r({ field: 'minutes', operator: 'between', value: '', value2: '60' }))).toBeTruthy(); + expect(ruleError(r({ field: 'minutes', operator: 'between', value: '30', value2: '60' }))).toBeNull(); + }); + it('flags empty text values', () => { + for (const operator of ['is', 'isNot', 'contains', 'startsWith'] as const) { + expect(ruleError(r({ operator, value: '' }))).toBeTruthy(); + expect(ruleError(r({ operator, value: 'x' }))).toBeNull(); + } + }); + it('flags relative-date rule with a non-positive or non-integer N', () => { + expect(ruleError(r({ field: 'release_date', operator: 'inLast', value: '', unit: 'day' }))).toBeTruthy(); + expect(ruleError(r({ field: 'release_date', operator: 'inLast', value: '0', unit: 'day' }))).toBeTruthy(); + expect(ruleError(r({ field: 'release_date', operator: 'inLast', value: '7', unit: 'day' }))).toBeNull(); + }); +}); + +describe('groupHasErrors', () => { + it('walks nested groups', () => { + const g: Group = { match: 'all', children: [{ match: 'any', children: [r({ value: '' })] }] }; + expect(groupHasErrors(g)).toBe(true); + expect(groupHasErrors({ match: 'all', children: [r({ value: 'ok' })] })).toBe(false); + }); +}); + +describe('topGroupAllNegative', () => { + it('is true only when every top-level child is a negative rule', () => { + expect(topGroupAllNegative({ match: 'any', children: [r({ operator: 'isNot' }), r({ operator: 'notMatches', field: 'plot' })] })).toBe(true); + expect(topGroupAllNegative({ match: 'any', children: [r({ operator: 'isNot' }), r({ operator: 'is' })] })).toBe(false); + expect(topGroupAllNegative({ match: 'all', children: [] })).toBe(false); + }); +}); + +describe('normalizeGroup', () => { + it("coerces a single-child group's match to 'all'", () => { + const g: Group = { match: 'any', children: [r({})] }; + expect(normalizeGroup(g).match).toBe('all'); + }); + it('leaves a 2+ child group match unchanged and recurses', () => { + const g: Group = { match: 'any', children: [r({}), { match: 'any', children: [r({})] }] }; + const n = normalizeGroup(g); + expect(n.match).toBe('any'); + expect((n.children[1] as Group).match).toBe('all'); + }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cd web && npx vitest run src/builder/rules/validation.test.ts` +Expected: FAIL — cannot find module `./validation`. + +- [ ] **Step 3: Write minimal implementation** + +```ts +// web/src/builder/rules/validation.ts +import { isGroup, type Group, type Rule } from './types'; + +const NEGATIVE = new Set(['isNot', 'notMatches']); +const RELATIVE_DATE = new Set(['inLast', 'notInLast']); + +export function ruleError(rule: Rule): string | null { + if (rule.operator === 'between') { + if (rule.value.trim() === '' || (rule.value2 ?? '').trim() === '') return 'Both bounds are required.'; + return null; + } + if (RELATIVE_DATE.has(rule.operator)) { + const n = Number(rule.value); + if (!Number.isInteger(n) || n <= 0) return 'Enter a whole number greater than zero.'; + return null; + } + if (rule.value.trim() === '') return 'A value is required.'; + return null; +} + +export function groupHasErrors(group: Group): boolean { + return group.children.some((c) => (isGroup(c) ? groupHasErrors(c) : ruleError(c) !== null)); +} + +export function topGroupAllNegative(group: Group): boolean { + const rules = group.children.filter((c): c is Rule => !isGroup(c)); + if (rules.length === 0 || rules.length !== group.children.length) return false; + return rules.every((r) => NEGATIVE.has(r.operator)); +} + +export function normalizeGroup(group: Group): Group { + const children = group.children.map((c) => (isGroup(c) ? normalizeGroup(c) : c)); + return { match: children.length < 2 ? 'all' : group.match, children }; +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `cd web && npx vitest run src/builder/rules/validation.test.ts` +Expected: PASS (all cases). + +- [ ] **Step 5: Commit** + +```bash +git add web/src/builder/rules/types.ts web/src/builder/rules/validation.ts web/src/builder/rules/validation.test.ts +git commit --no-verify -m "feat(438,435): extend rule types; validation + single-child normalization helpers" +``` + +--- + +## Task 2: `#435` — dateMacro mapping module + +> `types.ts` was already extended in Task 1 Step 0 (operators `inLast`/`notInLast`, `DateUnit`, +> `Rule.unit`, `OPERATORS_BY_TYPE.date`). This task adds only the mapping module. + +**Files:** +- Create: `web/src/builder/rules/dateMacro.ts` +- Test: `web/src/builder/rules/dateMacro.test.ts` + +**Interfaces:** +- Produces: + - `dateMacro.ts`: + - `RELATIVE_FIELD_MAP: Record<'release_date' | 'added_date', 'released' | 'added'>` + - `compileRelative(rule: Rule): string | null` — e.g. `{release_date, inLast, '7', day}` → `released_inthelast:"7 day"`; null if not a relative-date rule or invalid. + - `parseRelative(field: string, quoted: string): Rule | null` — recognizes synthetic field + `" "`, returns a Rule on `release_date`/`added_date`; null otherwise. + - `RELATIVE_SYNTHETIC = /^(released|added)_(inthelast|notinthelast)$/` (exported for parse.ts field detection). + +- [ ] **Step 1: Write the failing test** + +```ts +// web/src/builder/rules/dateMacro.test.ts +import { describe, expect, it } from 'vitest'; +import { compileRelative, parseRelative } from './dateMacro'; +import type { Rule } from './types'; + +const r = (o: Partial): Rule => ({ field: 'release_date', operator: 'inLast', value: '7', unit: 'day', ...o }); + +describe('compileRelative', () => { + it('maps release_date/inLast → released_inthelast', () => { + expect(compileRelative(r({}))).toBe('released_inthelast:"7 day"'); + }); + it('maps added_date/notInLast → added_notinthelast', () => { + expect(compileRelative(r({ field: 'added_date', operator: 'notInLast', value: '3', unit: 'week' }))).toBe('added_notinthelast:"3 week"'); + }); + it('returns null for a non-relative rule', () => { + expect(compileRelative(r({ operator: 'before' }))).toBeNull(); + }); + it('returns null when N is invalid', () => { + expect(compileRelative(r({ value: '0' }))).toBeNull(); + }); +}); + +describe('parseRelative', () => { + it('round-trips released_inthelast', () => { + expect(parseRelative('released_inthelast', '7 day')).toEqual({ field: 'release_date', operator: 'inLast', value: '7', unit: 'day' }); + }); + it('round-trips added_notinthelast', () => { + expect(parseRelative('added_notinthelast', '3 week')).toEqual({ field: 'added_date', operator: 'notInLast', value: '3', unit: 'week' }); + }); + it('returns null for an unrelated field', () => { + expect(parseRelative('title', '7 day')).toBeNull(); + }); + it('returns null for a malformed value', () => { + expect(parseRelative('released_inthelast', 'soon')).toBeNull(); + }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cd web && npx vitest run src/builder/rules/dateMacro.test.ts` +Expected: FAIL — cannot find module `./dateMacro`. + +- [ ] **Step 3: Write dateMacro.ts** + +```ts +// web/src/builder/rules/dateMacro.ts +import type { DateUnit, Rule } from './types'; + +export const RELATIVE_FIELD_MAP = { release_date: 'released', added_date: 'added' } as const; +type RelField = keyof typeof RELATIVE_FIELD_MAP; + +const OP_TO_SUFFIX = { inLast: 'inthelast', notInLast: 'notinthelast' } as const; +const SUFFIX_TO_OP = { inthelast: 'inLast', notinthelast: 'notInLast' } as const; +const CATALOG_FROM_PREFIX = { released: 'release_date', added: 'added_date' } as const; +const UNITS: DateUnit[] = ['day', 'week', 'month', 'year']; + +export const RELATIVE_SYNTHETIC = /^(released|added)_(inthelast|notinthelast)$/; + +function isValidN(value: string): boolean { + const n = Number(value); + return Number.isInteger(n) && n > 0; +} + +export function compileRelative(rule: Rule): string | null { + if (rule.operator !== 'inLast' && rule.operator !== 'notInLast') return null; + const prefix = RELATIVE_FIELD_MAP[rule.field as RelField]; + if (!prefix) return null; + if (!isValidN(rule.value) || !rule.unit || !UNITS.includes(rule.unit)) return null; + return `${prefix}_${OP_TO_SUFFIX[rule.operator]}:"${rule.value} ${rule.unit}"`; +} + +export function parseRelative(field: string, quoted: string): Rule | null { + const m = RELATIVE_SYNTHETIC.exec(field); + if (!m) return null; + const [, prefix, suffix] = m; + const vm = /^(\d+)\s+(day|week|month|year)$/.exec(quoted.trim()); + if (!vm) return null; + const [, n, unit] = vm; + if (!isValidN(n)) return null; + return { + field: CATALOG_FROM_PREFIX[prefix as 'released' | 'added'], + operator: SUFFIX_TO_OP[suffix as 'inthelast' | 'notinthelast'], + value: n, + unit: unit as DateUnit + }; +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `cd web && npx vitest run src/builder/rules/dateMacro.test.ts` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add web/src/builder/rules/dateMacro.ts web/src/builder/rules/dateMacro.test.ts +git commit --no-verify -m "feat(435): dateMacro compile/parse mapping for relative-date operators" +``` + +--- + +## Task 3: `#438` + `#435` — compile.ts wiring + +**Files:** +- Modify: `web/src/builder/rules/compile.ts` +- Modify: `web/src/builder/rules/compile.test.ts` + +**Interfaces:** +- Consumes: `compileRelative` (Task 2), `ruleError` (Task 1). +- Produces: `compileRule` emits the relative-date macro for `inLast`/`notInLast`, and returns `''` for a structurally-invalid rule (incomplete `between`, empty text value) so malformed Lucene is never emitted (the empty string is already filtered by `compileGroup`). + +- [ ] **Step 1: Write the failing test** — append to `compile.test.ts`: + +```ts +import { compileRelative } from './dateMacro'; // ensure no import cycle; if compile imports dateMacro, this is fine + +describe('compile — #438 guards + #435 relative dates', () => { + it('emits the relative-date macro', () => { + expect(compile({ match: 'all', children: [{ field: 'release_date', operator: 'inLast', value: '7', unit: 'day' }] })).toBe('released_inthelast:"7 day"'); + }); + it('drops an incomplete between instead of emitting field:[v TO ]', () => { + const out = compile({ match: 'all', children: [{ field: 'minutes', operator: 'between', value: '30' }] }); + expect(out).not.toContain('TO ]'); + expect(out).toBe(''); // sole invalid child filtered out + }); + it('drops an empty text value instead of emitting field:* / field:**', () => { + expect(compile({ match: 'all', children: [{ field: 'title', operator: 'contains', value: '' }] })).toBe(''); + expect(compile({ match: 'all', children: [{ field: 'title', operator: 'startsWith', value: '' }] })).toBe(''); + }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cd web && npx vitest run src/builder/rules/compile.test.ts` +Expected: FAIL — relative macro not emitted; incomplete-between emits `minutes:[30 TO ]`. + +- [ ] **Step 3: Edit compile.ts** + +Add the import and two guards at the top of `compileRule`: + +```ts +import { isGroup, type Group, type Rule } from './types'; +import { compileRelative } from './dateMacro'; +import { ruleError } from './validation'; + +// ... quote/escapeWild/normalizeDate unchanged ... + +function compileRule(rule: Rule): string { + // #435: relative-date operators map to synthetic query fields. + const relative = compileRelative(rule); + if (relative !== null) return relative; + + // #438: never emit malformed Lucene for a structurally-invalid rule; compileGroup filters ''. + if (ruleError(rule) !== null) return ''; + + const f = rule.field; + const v = rule.value; + switch (rule.operator) { + // ... existing cases unchanged ... + default: + return ''; + } +} +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `cd web && npx vitest run src/builder/rules/compile.test.ts` +Expected: PASS (new + existing cases). + +- [ ] **Step 5: Commit** + +```bash +git add web/src/builder/rules/compile.ts web/src/builder/rules/compile.test.ts +git commit --no-verify -m "feat(438,435): compile relative-date macros; drop invalid rules instead of malformed Lucene" +``` + +--- + +## Task 4: `#435` — parse.ts relative-date recognition + round-trip + +**Files:** +- Modify: `web/src/builder/rules/parse.ts` +- Modify: `web/src/builder/rules/parse.test.ts` +- Modify: `web/src/builder/rules/roundtrip.test.ts` + +**Interfaces:** +- Consumes: `parseRelative`, `RELATIVE_SYNTHETIC` (Task 2); `normalizeGroup` (Task 1). +- Produces: `parseAtom` recognizes `(released|added)_(inthelast|notinthelast):"..."` and returns the relative Rule; the round-trip test asserts `parse(compile(g)) ≡ normalizeGroup(g)`. + +- [ ] **Step 1: Write the failing test** — append to `parse.test.ts`: + +```ts +describe('parse — #435 relative dates', () => { + const ft = { release_date: 'date', added_date: 'date' } as const; + it('parses released_inthelast back to a relative rule', () => { + expect(parse('released_inthelast:"7 day"', ft)).toEqual({ + match: 'all', + children: [{ field: 'release_date', operator: 'inLast', value: '7', unit: 'day' }] + }); + }); + it('parses added_notinthelast', () => { + expect(parse('added_notinthelast:"3 week"', ft)).toEqual({ + match: 'all', + children: [{ field: 'added_date', operator: 'notInLast', value: '3', unit: 'week' }] + }); + }); +}); +``` + +Note: the synthetic field is NOT in `fieldTypes`, so `parseAtom` must handle it BEFORE the `fieldTypes[field]` lookup (which would reject it). + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cd web && npx vitest run src/builder/rules/parse.test.ts` +Expected: FAIL — synthetic field returns null (unknown field type). + +- [ ] **Step 3: Edit parse.ts** + +At the top of `parseAtom`, before the `type` lookup, add relative-date recognition (only the quoted form is valid; `NOT` prefix does not apply): + +```ts +import type { FieldType, Group, Operator, Rule } from './types'; +import { parseRelative, RELATIVE_SYNTHETIC } from './dateMacro'; + +function parseAtom(atom: string, fieldTypes: Record): Rule | null { + const trimmed = atom.trim(); + + // #435: relative-date synthetic fields are not in the catalog; match them first. + const relColon = trimmed.indexOf(':'); + if (relColon > 0 && RELATIVE_SYNTHETIC.test(trimmed.slice(0, relColon))) { + const rawRel = trimmed.slice(relColon + 1); + if (rawRel.startsWith('"') && rawRel.endsWith('"') && rawRel.length >= 2) { + return parseRelative(trimmed.slice(0, relColon), rawRel.slice(1, -1)); + } + return null; + } + + const negate = trimmed.startsWith('NOT '); + // ... rest unchanged ... +} +``` + +- [ ] **Step 4: Update the round-trip test** — in `roundtrip.test.ts`, (a) extend the generator `makeRule` to sometimes emit `date` rules with `inLast`/`notInLast` + a unit, and (b) compare against `normalizeGroup(g)` rather than `g` so single-child groups match. Concretely, import `normalizeGroup` and change the assertion: + +```ts +import { normalizeGroup } from './validation'; +// ... +// inside the property loop, replace `expect(parse(compile(g), fieldTypes)).toEqual(g)` with: +expect(parse(compile(g), fieldTypes)).toEqual(normalizeGroup(g)); +``` + +For the generator, add a relative-date branch (guard so `value` is a positive integer string and `unit` is set): + +```ts +// where a date-field rule is generated: +if (rng() < 0.5) { + const op = rng() < 0.5 ? 'inLast' : 'notInLast'; + const unit = (['day', 'week', 'month', 'year'] as const)[Math.floor(rng() * 4)]; + return { field: dateField, operator: op, value: String(1 + Math.floor(rng() * 30)), unit }; +} +// else fall through to the existing before/after/between generation +``` + +(Read the actual `roundtrip.test.ts` generator first — match its existing rng/field-selection shape; the above is the required behavior, not a verbatim drop-in.) + +- [ ] **Step 5: Run tests to verify they pass** + +Run: `cd web && npx vitest run src/builder/rules` +Expected: PASS (parse + roundtrip + all prior). + +- [ ] **Step 6: Commit** + +```bash +git add web/src/builder/rules/parse.ts web/src/builder/rules/parse.test.ts web/src/builder/rules/roundtrip.test.ts +git commit --no-verify -m "feat(435): parse relative-date macros; round-trip against normalizeGroup" +``` + +--- + +## Task 5: `#438` + `#435` — RuleBuilder.tsx UI (validation surface, relative-date inputs, single-child toggle) + +**Files:** +- Modify: `web/src/builder/rules/RuleBuilder.tsx` +- Modify: `web/src/screens/CollectionsScreen.tsx` (consumer — disable Save on errors) +- Modify: `web/src/builder/rules/RuleBuilder.test.tsx` + +**Interfaces:** +- Consumes: `ruleError`, `groupHasErrors`, `topGroupAllNegative`, `normalizeGroup` (Task 1); `DateUnit` (Task 2). +- Produces: RuleBuilder renders a per-rule error message + an aggregate warning; for `inLast`/`notInLast` it renders a number input + unit ``, value-``, and group-toggle patterns. The required behaviors: + +- [ ] **Step 1: Relative-date inputs.** When `rule.operator` is `inLast`/`notInLast`, render (in place of the single date input) a numeric `` bound to `rule.value` and a ` { - const nextType = typeOf(fields, e.target.value); - onChange({ field: e.target.value, operator: OPERATORS_BY_TYPE[nextType][0], value: '' }); - }} - > - {fields.map((f) => ( - - ))} - - - - - {enumField ? ( - { + const nextType = typeOf(fields, e.target.value); + onChange({ field: e.target.value, operator: OPERATORS_BY_TYPE[nextType][0], value: '' }); + }} + > + {fields.map((f) => ( + ))} - ) : ( - onChange({ ...rule, value: e.target.value })} - type={type === 'number' ? 'number' : type === 'date' ? 'date' : 'text'} - /> - )} - {rule.operator === 'between' && ( - onChange({ ...rule, value2: e.target.value })} - type={type === 'number' ? 'number' : 'date'} - /> - )} + - - - + {enumField ? ( + + ) : isRelativeDate ? ( + <> + onChange({ ...rule, value: e.target.value })} + type="number" + /> + + + ) : ( + onChange({ ...rule, value: e.target.value })} + type={type === 'number' ? 'number' : type === 'date' ? 'date' : 'text'} + /> + )} + + {rule.operator === 'between' && ( + onChange({ ...rule, value2: e.target.value })} + type={type === 'number' ? 'number' : 'date'} + /> + )} + + + + + + {error && ( + + {error} + + )} ); } @@ -153,7 +199,9 @@ function GroupEditor({ marginBottom: 8 }} > - onChange({ ...group, match: m })} /> + {group.children.length >= 2 && ( + onChange({ ...group, match: m })} /> + )} {group.children.map((child, i) => isGroup(child) ? ( void; fields: RuleField[]; }) { - return ; + return ( +
+ onChange(normalizeGroup(g))} /> + {topGroupAllNegative(value) && ( + + This query is entirely negative and will match nothing on its own. + + )} +
+ ); } diff --git a/web/src/screens/CollectionsScreen.tsx b/web/src/screens/CollectionsScreen.tsx index 4943a0ae6..01eac32dc 100644 --- a/web/src/screens/CollectionsScreen.tsx +++ b/web/src/screens/CollectionsScreen.tsx @@ -53,6 +53,7 @@ import { useSearchFields } from '../builder/rules/fieldCatalog'; import { parse } from '../builder/rules/parse'; import { RuleBuilder } from '../builder/rules/RuleBuilder'; import type { Group } from '../builder/rules/types'; +import { groupHasErrors } from '../builder/rules/validation'; type Tab = 'manual' | 'smart'; @@ -261,7 +262,11 @@ function SmartDialog({ // compiled string is always what `query` — the single value submitted/previewed — holds. const handleGroupChange = (next: Group) => { setGroup(next); - setQuery(compile(next)); + // Skip compiling an errored tree — a partial rule can still compile to a non-empty (but + // meaningless) query, which would otherwise satisfy the Save button's non-empty check. + if (!groupHasErrors(next)) { + setQuery(compile(next)); + } }; const runPreview = async () => { @@ -287,6 +292,7 @@ function SmartDialog({ const trimmedName = name.trim(); const trimmedQuery = query.trim(); const builderUnavailable = query.trim().length > 0 && parse(query, fieldTypes) === null; + const groupInvalid = mode === 'builder' && groupHasErrors(group); return (