Skip to content

Note list with filters, search, and sort - #3

Merged
unforced merged 1 commit into
mainfrom
note-list-filters
Apr 18, 2026
Merged

Note list with filters, search, and sort#3
unforced merged 1 commit into
mainfrom
note-list-filters

Conversation

@unforced

Copy link
Copy Markdown
Contributor

Summary

  • New /notes route is the default landing for a connected vault (/ redirects when a vault is active). Filter surface: full-text search, multi-select tag filter with any/all match toggle, path prefix, and createdAt asc/desc sort.
  • Search and path prefix inputs are debounced 300ms via a useDebouncedValue hook so each keystroke doesn't fire a request. Pagination is server-side limit=50 + offset; Next disables when a page returns fewer than limit rows.
  • Row layout shows path (or id fallback), preview, tag chips, and a relative-time stamp derived from updatedAt ?? createdAt.
  • Extended VaultClient with queryNotes(params) and listTags(). Added pure buildNoteQueryParams(state) helper and isFilteringActive(state) — together those are the testable query-layer core.
  • Empty state distinguishes "no notes yet" from "no notes match these filters". VaultAuthError routes through to a Reconnect button that sends the user to /add; skeleton rows on initial load; keepPreviousData keeps the previous page visible during refetch.

Latent bug squashed in passing

useActiveVaultClient() used a zustand selector that called loadToken(), which JSON.parses on every invocation and returns a new object identity each render. With no vault present (PR #1/#2 tests) this was harmless — the path returned null. With a token in the store, it pushed consumers into an infinite re-render loop. Moved the token lookup into useMemo keyed on activeVaultId.

Test plan

  • bun run typecheck clean
  • bun run lint clean
  • bun run test — 55/55 pass (10 query-builder, 4 relative-time, 6 client, 5 Notes route, 30 pre-existing)
  • bun run build succeeds (295 KB JS / 93 KB gzip)
  • bun run dev serves /notes at 200
  • Manual: point at a real vault and verify filter combinations, pagination, and the skeleton/empty/error states
  • Manual: expire a token and confirm the Reconnect flow

Out of scope

🤖 Generated with Claude Code

The default landing for a connected vault is the note list — everything
in Lens starts from here. The filter surface mirrors the vault's query
API (full-text search, tag filter with any/all match, path prefix, and
createdAt sort), debounced at 300ms so each keystroke doesn't fire a
request. Pagination is limit+offset at 50/page; next/previous disable
at the boundaries. Row layout leads with the path (or id fallback) and
shows preview, tags, and a relative-time stamp.

VaultAuthError bubbles through TanStack Query so an expired session
routes users back to /add with a Reconnect button instead of thrashing.
Skeleton rows on first load, placeholder-data retention while a filter
change refetches. Empty state distinguishes "no notes yet" from "no
notes match" so the copy is honest in both cases.

Fixed a latent instability in useActiveVaultClient: the zustand selector
was calling loadToken() which JSON.parses on every render, returning
a new object identity each time and pushing components into infinite
re-render loops when any token was present. Moved the token lookup
into useMemo keyed on activeVaultId.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@unforced
unforced merged commit b4883da into main Apr 18, 2026
@unforced
unforced deleted the note-list-filters branch April 18, 2026 21:04
unforced added a commit that referenced this pull request May 22, 2026
…ob onAuthRevoked detail

Two reviewer nits from notes#155:

1. `linkAttachment` now delegates to the inherited `addAttachment`
   rather than re-implementing the POST via `this.request(...)`. The
   wire shape is identical (POST `/api/notes/:id/attachments`); the
   thin wrapper preserves the Notes-facing name + semantic ("link an
   already-uploaded blob") while inheriting any future wire-shape
   changes from app-client automatically. Many callers reference
   `linkAttachment` directly (NoteNew, queue, queries, tests) so the
   alias stays.

2. `fetchBlobWithRetry` (the 401/403 path on attachment-blob loads)
   now parses the response body for `error_type` + `message` and
   forwards them to `onAuthRevoked` as the detail object, matching
   notes#150's enhanced-error shape and the base class's
   `requestWithRetry` behavior. Previously the blob path called
   `onAuthRevoked?.(res.status)` and dropped the detail, so banners
   triggered by attachment failures lost the parsed reason.

Body is read once with `res.text().catch(() => "")` before the
refresh-and-retry branch, mirroring the base class. Non-JSON bodies
leave detail undefined.

Reviewer nit #3 (a test specifically exercising the `setAccessToken`
override) is intentionally deferred — the dual-token sync is already
exercised indirectly by the `fetchAttachmentBlob` 401-retry test,
which verifies the rotated token reaches the second fetch call.

Tests: 768 passing (one new test added for the enhanced-detail blob
path). Typecheck + lint clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
unforced added a commit that referenced this pull request May 22, 2026
…request loop) (#155)

* refactor(notes-ui): subclass app-client VaultClient (drop duplicated request loop)

app-client 0.1.0-rc.3 lifted `request`, `requestWithRetry`, and
`requestCursorWithRetry` to `protected` (parachute-app#10), closing
the loop from notes#153's reviewer-flagged follow-up. Notes' VaultClient
now subclasses `@openparachute/app-client`'s base instead of cloning the
request/retry/error-classification loop.

- 219 lines deleted from `packages/notes-ui/src/lib/vault/client.ts`
- 691 lines deleted from `packages/notes-ui/src/lib/vault/client.test.ts`
  (base-class behaviors are covered by app-client's own suite; 14
  Notes-specific tests retained)

Notes-specific surface retained on the subclass:
  - `renameTag` / `mergeTags` / `deleteTag` — tag-curation endpoints
  - `listTagsWithSchema` — `/api/tags?include_schema=true` (schema-audit)
  - `linkAttachment` — alias of base `addAttachment` (preserves Notes'
    semantic + avoids a sweeping rename)
  - `fetchAttachmentBlob` — audio/image render path; carries its own
    retry loop because the base's `request*` always parses JSON

Notes' previous narrow `updateTag` override was dropped — the base
class's `TagUpsertPayload`-shaped `updateTag` accepts the same
`{description, parent_names}` shape `schema-ensure.ts` already passes,
and the caller ignores the return value.

Bundle delta: +3.8 kB raw / +0.9 kB gzip on the main chunk (the
subclass mirrors a handful of auth-callback fields on the instance so
the blob path can drive its own retry loop without reaching into the
base's `private` state).

Verification:
  - `bun run typecheck` clean
  - `bun run lint` clean
  - `bun run test` — 767/767 pass
  - `bun run build` — clean

Bumps `@openparachute/notes-ui` to 0.1.0-rc.3.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(notes-ui): #155 reviewer — linkAttachment delegates to super + blob onAuthRevoked detail

Two reviewer nits from notes#155:

1. `linkAttachment` now delegates to the inherited `addAttachment`
   rather than re-implementing the POST via `this.request(...)`. The
   wire shape is identical (POST `/api/notes/:id/attachments`); the
   thin wrapper preserves the Notes-facing name + semantic ("link an
   already-uploaded blob") while inheriting any future wire-shape
   changes from app-client automatically. Many callers reference
   `linkAttachment` directly (NoteNew, queue, queries, tests) so the
   alias stays.

2. `fetchBlobWithRetry` (the 401/403 path on attachment-blob loads)
   now parses the response body for `error_type` + `message` and
   forwards them to `onAuthRevoked` as the detail object, matching
   notes#150's enhanced-error shape and the base class's
   `requestWithRetry` behavior. Previously the blob path called
   `onAuthRevoked?.(res.status)` and dropped the detail, so banners
   triggered by attachment failures lost the parsed reason.

Body is read once with `res.text().catch(() => "")` before the
refresh-and-retry branch, mirroring the base class. Non-JSON bodies
leave detail undefined.

Reviewer nit #3 (a test specifically exercising the `setAccessToken`
override) is intentionally deferred — the dual-token sync is already
exercised indirectly by the `fetchAttachmentBlob` 401-retry test,
which verifies the rotated token reaches the second fetch call.

Tests: 768 passing (one new test added for the enhanced-detail blob
path). Typecheck + lint clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant