Skip to content

feat: add reusable global miner search - #973

Draft
mcharles-square wants to merge 7 commits into
mainfrom
feat/809-global-miner-search
Draft

feat: add reusable global miner search#973
mcharles-square wants to merge 7 commits into
mainfrom
feat/809-global-miner-search

Conversation

@mcharles-square

Copy link
Copy Markdown
Collaborator

Reviewable diff: +185/-39 across 13 files (excludes generated, test, and story files).

Summary

Fleet operators can now find miners globally from one debounced search field across name, device identifier, serial number, MAC address, IP address, and worker name. The search is available on the fleet miner table and reusable miner-selection flows, while preserving URL filters, saved views, pagination, totals, and safe selection behavior. Closes #809.

How it works

The client debounces input for 250 ms and adds the query to the existing MinerListFilter. The Fleet service trims and bounds the query, then the SQL store applies a parameterized, literal substring ILIKE predicate across the searchable miner fields. The same filtered query drives paginated rows, totals, state counts, model groups, exports, and filtered identifier resolution.

In selection modals, the shared miner list sends the query through the same server-side filter. Modal Select All is disabled while searching so a search cannot be accidentally represented as an organization-wide allDevices selection.

Diagrams

flowchart LR
  A["Operator types search"] --> B["Debounced miner search input"]
  B --> C["MinerListFilter.search_query"]
  C --> D["Fleet management RPC"]
  D --> E["SQL search predicate"]
  E --> F["Paginated rows and matching totals"]
Loading
sequenceDiagram
  participant O as Operator
  participant UI as Miner list or picker
  participant API as Fleet API
  participant DB as Miner store
  O->>UI: Type partial identifier
  UI->>UI: Wait 250 ms
  UI->>API: List request with search_query
  API->>DB: Apply search AND existing filters
  DB-->>API: Rows, count, and cursor
  API-->>UI: Matching page
  UI-->>O: Display matching miners
Loading

Areas of the code involved

Area / package / file What changed Why it matters for review
proto/fleetmanagement Added MinerListFilter.search_query. Defines the API contract shared by list and bulk-selection flows.
server/internal/domain/fleetmanagement Parses, trims, and bounds the query. Establishes the request-validation boundary.
server/internal/domain/stores/sqlstores Adds escaped, parameterized multi-column search and routes search queries through dynamic counts. Ensures substring matching, pagination, totals, and org scoping remain consistent.
client/src/protoFleet/components Added reusable debounced search input and integrated it into MinerSelectionList. Covers rack, group, schedule, alert, and curtailment miner pickers.
client/src/protoFleet/features/fleetManagement Added All Miners URL state, saved-view persistence, and bulk-action copy handling. Keeps search shareable and prevents filtered selections from being described as fleet-wide.
client/src/shared/components/Search Added configurable IDs and labels. Allows multiple miner search controls without duplicate DOM IDs.
Generated protobuf files Regenerated client and server bindings — generated, skip. Required output for the proto contract change.

Key technical decisions & trade-offs

  • Extend the existing MinerListFilter instead of adding a search RPC, so pagination, counts, exports, and selection flows share one server-side implementation.
  • Use parameterized literal ILIKE matching instead of client-side filtering, so searches work across pages and do not expose cross-organization data.
  • Disable modal Select All during a search instead of broadening the generic selector to the whole fleet; explicit row selection remains available.
  • Start without trigram indexes because the expected 3,000+ fleet size is compatible with the existing filtered scan; query plans can justify a later index migration.

Testing & validation

  • just gen
  • Targeted Go parser and SQL filter tests passed.
  • Targeted Vitest tests passed: 84 tests.
  • npm run build:protoFleet passed.
  • TypeScript compilation and targeted ESLint passed.
  • Fleet miner list/action tests passed: 138 tests, 1 skipped.
  • Database integration tests could not run because the local Postgres instance rejected the configured fleet user password.

@github-actions github-actions Bot added the review-policy: needs-review Managed by the Review Policy workflow. label Aug 26, 2026
@github-actions github-actions Bot added javascript Pull requests that update javascript code client server shared labels Aug 26, 2026
@mcharles-square
mcharles-square requested a balanced review from Copilot August 26, 2026 17:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds reusable, debounced global miner search across fleet tables and selection flows, backed by server-side filtering.

Changes:

  • Adds validated multi-field substring search to the protobuf API and SQL store.
  • Integrates search with URLs, saved views, counts, exports, and bulk actions.
  • Adds reusable search UI and coverage for server/client behavior.

Reviewed changes

Copilot reviewed 22 out of 24 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
server/internal/domain/stores/sqlstores/device.go Routes searched counts through dynamic queries.
server/internal/domain/stores/sqlstores/device_filters.go Builds escaped multi-field search predicates.
server/internal/domain/stores/sqlstores/device_filters_test.go Tests search SQL and dynamic routing.
server/internal/domain/stores/sqlstores/device_filters_integration_test.go Tests database search and count consistency.
server/internal/domain/stores/interfaces/device.go Adds search to the domain filter.
server/internal/domain/fleetmanagement/service.go Parses and bounds search input.
server/internal/domain/fleetmanagement/parse_filter_test.go Tests search parsing and limits.
proto/fleetmanagement/v1/fleetmanagement.proto Adds the search API field.
client/src/shared/components/Search/Search.tsx Makes search IDs, labels, and sanitization configurable.
client/src/protoFleet/features/fleetManagement/views/viewSummary.ts Summarizes saved search filters.
client/src/protoFleet/features/fleetManagement/views/savedViews.ts Persists search in saved views.
client/src/protoFleet/features/fleetManagement/utils/fleetVisiblePairingFilter.ts Preserves search through pairing filters.
client/src/protoFleet/features/fleetManagement/utils/fleetVisiblePairingFilter.test.ts Tests search preservation.
client/src/protoFleet/features/fleetManagement/utils/filterUrlParams.ts Encodes and parses search URLs.
client/src/protoFleet/features/fleetManagement/utils/filterUrlParams.test.ts Tests search URL handling.
client/src/protoFleet/features/fleetManagement/components/MinerList/MinerList.tsx Adds search to the fleet table.
client/src/protoFleet/features/fleetManagement/components/MinerActionsMenu/useMinerActions.tsx Treats search as an active filter.
client/src/protoFleet/features/fleetManagement/components/MinerActionsMenu/useMinerActions.test.tsx Tests searched bulk-action copy.
client/src/protoFleet/components/MinerSelectionList.tsx Adds search to miner pickers.
client/src/protoFleet/components/MinerSelectionList.test.tsx Tests picker search and select-all behavior.
client/src/protoFleet/components/MinerSearchInput.tsx Implements reusable debounced search.
client/src/protoFleet/components/MinerSearchInput.test.tsx Tests debounce and input normalization.
client/src/protoFleet/api/generated/fleetmanagement/v1/fleetmanagement_pb.ts Regenerates the TypeScript protobuf binding.
Suppressed comments (2)

client/src/protoFleet/components/MinerSearchInput.tsx:56

  • This cleanup only handles unmounts, so a pending query survives an external initialValue change. For example, typing and then clearing filters, navigating back, or applying a saved view within 250 ms re-seeds the displayed value, but the stale timer subsequently fires and restores the old search. Cancel the pending timeout whenever initialValue changes as well.
  // Unmount-only: keying this on `onQueryChange` would cancel a pending search
  // whenever an unrelated navigation changed the callback's identity.
  useEffect(
    () => () => {
      if (timeoutRef.current) clearTimeout(timeoutRef.current);
    },
    [],
  );

client/src/protoFleet/components/MinerSearchInput.tsx:65

  • The API contract rejects search queries longer than 255 Unicode code points, but this input permits and emits arbitrary lengths. Entering 256+ characters therefore guarantees every list/count request will fail and surfaces only the generic “Failed to load miners” toast. Apply the same bound in the input sanitizer so invalid queries cannot be sent.
      sanitize={trimLeadingWhitespace}

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +42 to +46
const handleChange = useCallback((value: string) => {
if (timeoutRef.current) clearTimeout(timeoutRef.current);
timeoutRef.current = setTimeout(() => {
onQueryChangeRef.current(value);
}, SEARCH_DEBOUNCE_MS);
The all-mode bulk-action filter dropped the new searchQuery field, so
"Select all" under a search sent unpair/rename/worker-name/reparent at
the whole fleet while the confirmation copy claimed a filtered scope.

Also from review of the global miner search change:
- keep a pending search debounce alive across onQueryChange identity churn
- bound search_query by runes, matching the proto's max_len contract
- reuse the name sort expression so search-by-name and sort-by-name agree
- stop the search integration test from nulling NOT NULL mac_address
…icate

Three call sites that pair a count with a row set each carried their own
copy of the same ten-term boolean, and every new filter dimension had to
be added to all three in lockstep. The copies agreed, but nothing kept
them agreeing; the search field in the previous commit had to be added to
each one by hand.

Collapse them onto minerFilterParams.requiresDynamicQuery(). Cover every
dimension with a unit test, and assert end-to-end that a search-scoped
list total, state breakdown, and model-group counts all agree.
The debounced handler emitted value.trim(), the miner list persisted that
to the URL, and it came straight back as initialValue. Search and Input
both re-seed their displayed value from that prop, so the trimmed echo
overwrote the field while it still had focus.

Typing "rack ", pausing past the 250ms debounce, then typing "7" produced
"rack7": the persisted value came back without the space and the next
keystroke landed against the trimmed text. Any multi-word query typed at
human speed was affected.

Emit the query as typed and leave trimming to the consumers, which already
do it — the server trims search_query, the miner list trims when reading
the URL param, and the selection list trims before testing for an active
search. Leading whitespace is dropped via Input's sanitize hook, which
applies it to the displayed text and the emitted value together so the two
cannot disagree; trailing whitespace stays typable.
Search's compact mode is not a size modifier — it drops the border, the
focus ring and the clear button, and shrinks the field to 24px. Both real
usages had opted into it, so the miner list rendered 50%-opacity label text
on the page background next to bordered secondary buttons, with nothing to
signal it was a field at all.

Replace the compact boolean with an explicit variant:

  compact  bare, no container — only for callers supplying their own
  toolbar  bordered at control height, with a clear button
  default  the 56px field used in modals, with the Cmd-K hint

toolbar reuses the bare input and owns the container, so the field matches
the height of the compact buttons beside it rather than the 56px modal
field. It supplies its own clear button because Input renders one only at
the default height, and returns focus to the field after clearing.

Also widen the wrapper from w-24 to w-full on mobile: 96px fit about eight
characters.
…ounce

MinerSelectionList treats an active search and an all-mode selection as
mutually exclusive: canSelectAll requires an empty searchQuery, and an
effect force-clears allSelected when that stops holding. Both read the
applied filter, which the search input updates only after its 250ms
debounce, so the invariant had a 250ms hole. Consumers call getSelection()
straight from their submit handler with no second confirmation, so a
keystroke followed by a click inside that window committed all-mode with a
searchQuery the filter still reported as empty.

Add a synchronous onQueryInput alongside the debounced onQueryChange and
gate select-all on both the applied and the pending query. Gating on both
also covers the reverse race, where clearing the field would otherwise
re-offer select-all 250ms before the list stopped being filtered.

Reported by Copilot on #973.

The miner list is unaffected: it supports all-mode with a search by design,
reads currentFilter when the action is confirmed rather than when it is
opened, and resets the selection when the URL filter changes.
@mcharles-square
mcharles-square force-pushed the feat/809-global-miner-search branch from f55db6d to 659fcbe Compare August 26, 2026 21:00
@github-actions

github-actions Bot commented Aug 26, 2026

Copy link
Copy Markdown

🔐 Codex Security Review

Note: This is an automated security-focused code review generated by Codex.
It should be used as a supplementary check alongside human review.
False positives are possible - use your judgment.

Scope summary

  • Reviewed pull request diff only (c126c0b6bfa0947967caa5a04474503cdff5c200...dff82a94135fde43e57e507d0ba48896bf805a9e, exact PR three-dot diff)
  • Model: gpt-5.6-sol

💡 Click "edited" above to see previous reviews for this PR.


Review Summary

Overall Risk: HIGH

Findings

[HIGH] Automated review incomplete

  • Category: Other
  • Description: The automated review produced no usable result for c126c0b6bfa0947967caa5a04474503cdff5c200...dff82a94135fde43e57e507d0ba48896bf805a9e (workflow run 33168236407; reason: codex-job-timeout, elapsed: unknown, budget: 9 minutes).
  • Impact: The pull request has not received complete automated security, correctness, and reliability analysis.
  • Recommendation: Require human review before merging. Do not treat this result as approval-free or low risk.

Notes

Human review is required because the bounded automated review was incomplete.


Generated by Codex Security Review |
Triggered by: @mcharles-square |
Review workflow run

Disambiguate ProtoOS and single-miner log search selectors from the new accessible clear button. Keep stale debounced queries from surviving external value resets, and cap typed queries at the API's 255-Unicode-code-point boundary.

Co-authored-by: Jared Marrz <jmarr@squareup.com>
Signed-off-by: Jared Marrz <jmarr@squareup.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

client javascript Pull requests that update javascript code review-policy: needs-review Managed by the Review Policy workflow. server shared

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add search/filter functionality to the All Miners table (and other miner-selection surfaces)

3 participants