mirror of
https://github.com/cpvalente/ontime.git
synced 2026-08-18 13:44:12 +00:00
docs(spec): record delivered finder phases
Marks the correctness and search-depth phases as delivered, notes what is still outstanding inside them, and records the measured cost of widening the scan. Frames the review section as the state at the time of writing, since several findings no longer describe the code. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DzALEq9gGWwFmwTdgAiFcY
This commit is contained in:
+47
-29
@@ -2,8 +2,9 @@
|
||||
|
||||
## Context
|
||||
|
||||
The Finder is a `mod+F` modal that searches the rundown and jumps to an entry. Four files, ~370
|
||||
lines, essentially untouched since introduction, no tests, mounted only in the rundown editor.
|
||||
The Finder is a `mod+F` modal that searches the rundown and jumps to an entry. At the time of this
|
||||
review it was four files and ~370 lines, essentially untouched since introduction, with no tests and
|
||||
mounted only in the rundown editor.
|
||||
|
||||
This document reviews the feature and sets out a roadmap. The framing question was a
|
||||
business-development one: is it worth investing in, and can it beat the competition?
|
||||
@@ -22,6 +23,9 @@ entries, it does not act on them), and **results stay in rundown order** — no
|
||||
|
||||
## 1. Review findings
|
||||
|
||||
_This section records the feature as it stood when reviewed, and is kept as the rationale for the
|
||||
roadmap below. The findings in 1.1, 1.2 and 1.6 have since been addressed — see phases 1 and 2._
|
||||
|
||||
### 1.1 It doesn't search what people search for
|
||||
|
||||
A bare query is **title-only**. The two things an operator has in their head are the **cue** and
|
||||
@@ -113,7 +117,10 @@ Sequence accordingly.
|
||||
|
||||
Estimates assume one developer familiar with the codebase.
|
||||
|
||||
### Phase 1 — Correctness _(~0.5 day)_
|
||||
Phases 1 and 2 are **delivered**; the remaining phases are proposals. Items still outstanding inside
|
||||
a delivered phase are called out where they sit.
|
||||
|
||||
### Phase 1 — Correctness _(done)_
|
||||
|
||||
1. **Track `selectedId`, not `selectedIndex`** — derive the active row by id lookup, falling back to
|
||||
the first. Correct across updates, reordering and result changes; better than "reset to 0", which
|
||||
@@ -123,36 +130,47 @@ Estimates assume one developer familiar with the codebase.
|
||||
scrolls the list under a stationary cursor, fires a move event, and yanks the selection back. The
|
||||
12-result cap hides this today; a longer list won't.
|
||||
4. **Scroll the active row into view** on arrow navigation.
|
||||
5. **`mod+F` closes** — keep the global hotkey for opening only (its INPUT-ignoring default is right
|
||||
there), handle `mod+F` locally on the input to close. Drop the global `Escape` handler in
|
||||
`FinderPlacement`; Base UI's `Dialog` already handles it, and `preventDefault: true` globally is a
|
||||
latent conflict with inline field editing.
|
||||
5. **`mod+F` opens and closes from anywhere.** The hotkey hook skips input elements by default, so
|
||||
the shortcut was dead while editing an entry — exactly when a user reaches for it. Opting out of
|
||||
that lets one binding both open and close. The global `Escape` handler is gone; Base UI's
|
||||
`Dialog` already dismisses, and `preventDefault: true` document-wide conflicted with inline
|
||||
field editing.
|
||||
6. **Fix the `index <n>` bound** to use the event count.
|
||||
7. **Search milestones by cue.** The cue search only walked events, so a milestone could never be
|
||||
found by the cue it displays.
|
||||
|
||||
### Phase 2 — Search depth _(~1.5–2 days)_
|
||||
### Phase 2 — Search depth _(done)_
|
||||
|
||||
7. **Search cue + title + note + text custom fields** across events, groups and milestones on a bare
|
||||
query. Skip `image`-type custom fields. Cap indexed note length so one pasted script can't
|
||||
dominate memory.
|
||||
8. **Add filters** `note:`, `flag:`, `group:`, `<custom-field>:`, accepting both `cue:x` and `cue x`
|
||||
for every key so today's documented syntax keeps working. Drive the footer hint and the
|
||||
`EventEditorEmpty.tsx` cheat sheet from one filter-key constant so they can't drift.
|
||||
9. **Keep rundown order — no ranking.** Deterministic, simpler, and it preserves the early-exit scan
|
||||
(today's cap of 12 is a _scan_ cap, not a sort cap; ranking would force scanning everything).
|
||||
_One honest consequence:_ widening the searched fields while keeping position order means a
|
||||
distant cue match can be pushed off the list by nearer note matches. Two mitigations, both cheap
|
||||
— raise the cap and show "showing 12 of 47", and label which field matched so a note hit is
|
||||
obviously a note hit. If that still isn't enough in practice, group by matched field (cue block,
|
||||
then title, then note/custom) with rundown order inside each block — still fully deterministic.
|
||||
10. **Highlight the matched substring** and show the matching field's text.
|
||||
11. **Make results a pure derivation of (index, query)** — a controlled input removes the
|
||||
`useEffect`-replays-`lastSearchString` mechanism entirely, and with it the stale-index crash,
|
||||
rather than patching around it.
|
||||
12. **Unit tests** for the query parser (pattern: `features/rundown/__tests__/rundown.utils.test.ts`).
|
||||
Highest-value case: `index <n>` staying aligned with 1-based UI event indices when delays, groups
|
||||
and milestones interleave — the current code gets this right and a rewrite is likely to break it.
|
||||
8. **Search cue + title + note + text custom fields** across events, groups and milestones on a bare
|
||||
query. `image` custom fields are skipped — they hold a URL.
|
||||
9. **Filter badges.** The syntax was previously discoverable only through a line of footer text.
|
||||
A badge row now offers the fixed fields plus every project custom field, scoping the search while
|
||||
keeping what the user already typed. Both `cue x` and `cue:x` parse, so badges and typing agree.
|
||||
10. **Keep rundown order — no ranking.** Deterministic, and it keeps results predictable during a
|
||||
show. _The honest consequence:_ matching more fields while holding position order means a distant
|
||||
cue match can be pushed off by nearer note matches. Mitigated by raising the cap, reporting the
|
||||
true total, and naming the matched field per row.
|
||||
11. **Name the matching field** with an excerpt, so a hit inside a long note is legible.
|
||||
12. **Results are a pure derivation of (data, query)** — the controlled input removed the
|
||||
`useEffect` that replayed the last search on rundown changes, and with it the stale-index crash.
|
||||
|
||||
### Phase 3 — Visibility, UX and polish _(~1.5 days)_
|
||||
**Measured cost of widening the scan** — cue + title + note + 3 custom fields versus title alone:
|
||||
|
||||
| Rundown | Title only | All fields | All fields, prebuilt index |
|
||||
| ------- | ---------- | ---------- | -------------------------- |
|
||||
| 200 | 0.061 ms | 0.051 ms | 0.020 ms |
|
||||
| 1,000 | 0.149 ms | 0.112 ms | 0.020 ms |
|
||||
| 5,000 | 0.608 ms | 0.563 ms | 0.116 ms |
|
||||
|
||||
All-fields measures the same as title-only at every size — both are dominated by loop overhead
|
||||
rather than string comparison. Performance was never the reason to search one field.
|
||||
|
||||
**Still outstanding from this phase:** `flag:` and `group:` filters; highlighting the matched
|
||||
substring within the excerpt; unit tests for the query parser (covered by e2e today, but the parser
|
||||
is now a pure function and worth testing directly — the highest-value case is `index <n>` staying
|
||||
aligned with 1-based UI indices when delays, groups and milestones interleave).
|
||||
|
||||
### Phase 3 — Visibility, UX and polish _(~1 day, partly done)_
|
||||
|
||||
This is the phase that changes how many people ever use the feature.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user