Lint warnings 44 → 25. The remaining 14 `any` warnings sit in
settings-provider.tsx (provider config form + test results dialog,
needs domain knowledge — deferred per scope). The other 11 warnings
are React Compiler "Compilation Skipped" notices, not code issues.
- lib/log.ts: any → unknown (5) — logger accepts arbitrary input
- shared/markdown.tsx: typed the recursive ReactNode walker with a
`hasChildrenProp` type guard around `isValidElement`; the components
map now uses react-markdown's `Components` type so `code` / `pre`
overrides infer their props (5)
- settings-prompt.tsx: `ControllerProps.control: Control<FieldValues>`,
`field: ControllerRenderProps<…>`, validation result typed against
`ValidatePromptMutation['validatePrompt']`, dropped a stray
`as any` on `removeEventListener` options (4)
- settings-prompts.tsx: row sub-component params typed via
`Row<T>` from @tanstack/react-table (2)
- settings-providers.tsx: provider icon map typed against
`SVGProps<SVGSVGElement>`, row sub-component via `Row<Provider>`,
recursive `getFields(obj)` narrowed from `any` to `unknown` with a
single boundary cast for `Object.entries` (3)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Baseline 8/10 fail (80% flake) → 20/20 pass. Root cause was `user.type`
racing the Radix Sheet focus trap, not the `useDebounce` macrotask
theory from the prior investigation: with `defaultOpen=true` Radix sets
`body { pointer-events: none }`, swallowing user-event's internal click,
so keystrokes route to whatever element currently holds focus (the
SheetContent close button) instead of the input. `inputValue` stays
empty and the listbox never narrows.
`fireEvent.change` mirrors the sidestep already used in the
URL-filter AND test in the same file.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Followed up on the "no war-story comments" rule (memory file added a
few commits ago, then promptly broken in the X-button addition).
Pass through every comment with the test "remove it — does a senior
reading the code for the first time understand less?".
Removed:
- "Same two-cell shape as DataTableFilter, see its docstring" — cross-
file rationale belongs in commit history.
- "Hint shown over the collapsed trigger. Kbd self-themes…" — Kbd
configuration detail, not part of this component's contract.
- "Mirror size='sm' button geometry used by HeaderButton" — micro
implementation detail.
- "Collapsed trigger sits flush… has-[>button]:ml-[-0.45rem] handles
the analogous adjustment" — CSS quirk story.
- "The trigger is a real <button> — never a <div> with onClick" —
generic HTML/a11y factoid, not specific to this file.
- "The component is controlled — parent owns searchQuery" — visible
from the props shape.
- "Belt-and-braces for programmatic clears" — defensive-style war
language, replaced with one line stating the effect's purpose.
- "Initialise expanded if the parent already has a non-empty query" —
duplicates `useState(() => searchQuery.trim().length > 0)`.
Kept (in single-line / sub-3-line form):
- `COMMIT_DEBOUNCE_MS` shares timing with DataTableFilter.
- `External → local sync` effect intent.
- `rAF` before focus so motion's transform has started.
- `handleInputBlur` reads `localValue`, not the prop (subtle invariant).
- Auto-collapse effect purpose.
- `inputRef.current?.focus()` in `handleClear` (otherwise auto-collapse
fires and the field disappears under the user).
- Top-level JSDoc shrunk from 28 lines to 5 — what it does plus the
two equivalent clear paths.
Behaviour unchanged. 15/15 tests still pass, lint clean. File: 340 → 260
lines (-24%).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`DataTableFilter` already ships a conditional trailing `<X>` clear
button; `InputSearch` (the collapsible variant used for /knowledges
semantic search and the same shape sitting in flow detail searches)
only offered keyboard Esc to clear. Unify so every search field across
the app has the same shape.
Implementation:
- Extract a shared `handleClear` used by both the new X button and the
first-Esc branch. The path cancels any pending debounce, emits `''`
upstream, and explicitly returns focus to the input — this matches
the Esc-clear path (where focus is already on the input) and
side-steps the programmatic-clear collapse effect, so the field
stays open and ready for fresh typing.
- Render the X only while `isExpanded && localValue` so it can never
appear on a collapsed/empty field. `aria-label` is contextual:
"Clear search knowledge documents" / "Clear semantic search" / etc.,
derived from the host page's `ariaLabel`.
- Update the JSDoc to describe both clear entry points.
Tests (input-search.test.tsx, 15 total — 10 baseline + 5 new):
- absent on empty mount
- present on deep-linked non-empty mount
- shows/hides as the user types and clears
- click clears value, emits `''` upstream, keeps focus on input,
prevents the trigger from re-appearing
- click mid-typing drops any pending debounce — no leaked emit
Browser verified on /knowledges semantic search:
beforeClear { value: 'jwt', url: '?qs=jwt', xPresent: true }
afterClear { value: '', url: '', xPresent: false, focusOnInput: true }
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
After the `chore(frontend): upgrade graphql-codegen toolchain` upgrade
(8a3d4e9) the generated `src/graphql/types.ts` carried every base type
twice — once as `export enum Foo` (from `typescript`) and once as
`export type Foo = 'a' | 'b'` (from `typescript-operations`). Vite's
dependency scan failed on the duplicate declarations with 29
`Identifier 'X' has already been declared` parse errors, which made
the dev server occasionally die on cold start.
Root cause: this was a stale v5-style config running on v6 plugins.
The v6 migration guide is explicit — `preResolveTypes` has been
removed, and `typescript-operations@6` now emits base types itself,
so the canonical setup is `plugins: ['typescript-operations']` rather
than `['typescript', 'typescript-operations']`. Keeping both meant the
two plugins independently emitted the full type set in different
shapes.
Bisect confirmed the split: `typescript` alone → 1327 lines, no
duplicates; `typescript-operations` alone → 1311 lines, no
duplicates; the pair → 2639 lines with 29 duplicate type names.
With `typescript-react-apollo` also in the mix the regenerated file
ballooned to 8819 lines, half of which was the shadow set.
Fix:
- Drop the `typescript` plugin from the codegen config.
- Drop `preResolveTypes: true` (removed in v6).
- Add `enumType: 'native'` so the call sites that use enum-style
access (`KnowledgeDocType.Answer`, `AgentConfigType.Adviser`, …)
keep compiling — without it, v6 defaults to string-literal unions.
Result:
- `src/graphql/types.ts` shrinks 8819 → 7586 lines, zero duplicate
declarations.
- `KnowledgeDocType`, `AgentConfigType`, etc. remain real TS `enum`s
so `KnowledgeDocType.Answer`-style access keeps working.
- `pnpm run build` clean, lint clean, 531/531 tests passing.
- Vite dev start no longer prints `[PARSE_ERROR] Identifier X has
already been declared`. `/knowledges` (the page that exercises
enum access) loads with 10 rows and zero console errors.
Migration guide:
https://the-guild.dev/graphql/codegen/docs/migration/operations-and-client-preset-from-5-0
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reviewing 17451e7: the dedupe was added in reaction to a runtime
`Cannot read properties of null (reading 'useRef')` that I attributed
to dual React copies via pnpm hoist. But I applied two changes at
once — clearing `node_modules/.vite` and adding dedupe — and never
isolated which one actually fixed it.
Empirical re-check just now: production build, dev server, and
the affected page all work with dedupe removed. The real cause was
the stale Vite pre-bundle (`.vite/deps/`), which doesn't auto-rebuild
after `pnpm add` in a running dev server. Cache clear alone fixes it.
That makes the dedupe defensive hygiene at best, and the 7-line
war-story comment alongside it actively misleads — it pins a problem
on a mechanism that wasn't the cause. Removing both keeps the config
honest. QA report updated to record the misdiagnosis as a process
note instead of an architectural rule.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Run `pnpm prettier:fix` brought three files into compliance:
- data-table.test.tsx — new race-scenario suite added in the previous
commit; prettier preferred a slightly different multi-line shape for a
couple of `render(<FilterHost />)` calls.
- input-search.test.tsx — new file in the previous commit; same minor
multi-line reshaping for `render(<SearchHost />)`.
- settings-api-tokens.tsx — pre-existing single-line nested ternary that
predates this branch, now collapsed to one line per prettier.
No behavior change. Format-only.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
After landing the imperative-timer fix and extracting `useDebouncedCallback`
into our own hook, the use-site count climbed past three and the case for
a single, battle-tested contract beat the case for in-tree code. Drops
both our `useDebouncedValue` and `useDebouncedCallback` and routes every
caller through `use-debounce@10.1.1` (1M+ weekly, ~3KB gzip).
API mapping at each use-site:
- value debounce: `useDebouncedValue(v, ms)` → `const [v] = useDebounce(v, ms)`
- callback debounce: `useDebouncedCallback(fn, ms)` → identical signature,
same `.cancel()`/`.isPending()` semantics we relied on, plus `.flush()`
and leading/trailing/maxWait options we now get for free.
Files touched (7 use-sites): data-table.tsx, input-search.tsx,
use-table-state.ts, use-table-query-filter.ts, knowledges-provider.tsx,
use-flow-files-search.ts, use-detail-navigation.ts.
pnpm + Vite gotcha: pnpm hoists into `.pnpm/<name>@<ver>/`, so any
transitive `import React from 'react'` from a third-party package can
resolve to its own physical copy. With one path through our code and
another through use-debounce, the runtime crashed at `useRef` with
"Cannot read properties of null (reading 'useRef')". Added
`resolve.dedupe: ['react', 'react-dom']` in vite.config.ts so the
bundle ships exactly one React.
Tests:
- new input-search.test.tsx (10 tests) covers collapsed/expanded
presentation, expand-on-click + rAF focus, burst-typing coalesce,
Esc-clear mid-debounce, two-stage Esc semantics, external-source-wins
race, Ctrl+F / Cmd+F global hotkey expansion.
- data-table.test.tsx +5 tests: deep-link initial filterValue,
6-keystroke burst coalesce, unmount-mid-debounce cleanup, two
independent DataTables on one page (settings/prompts shape),
back-button-style external clear, three-round type/clear stress.
- full suite: 541/541 passing, lint 0 errors.
Manual smoke via Chrome DevTools MCP on every page that mounts a
filter input — /flows, /templates, /settings/api-tokens,
/settings/prompts (both tables independent), /knowledges (filter
+ semantic search hitting the server through ?qs=). All scenarios
collapse to a single `[{v="", u=""}]` transition; no URL re-emit
after X, no leftover timer after Esc, no cross-table leakage.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous iteration moved DataTableFilter / InputSearch off
`useDebouncedValue` and onto an inline `setTimeout` + `pendingTimerRef`
+ `cancelPendingEmit` dance. That fixed all four race scenarios but
left imperative scheduling living inside components — a code smell
flagged in review. After surveying the React 19.2 surface area
(`useDeferredValue` is for *render* priority, not side-effect throttling;
`useTransition` doesn't apply to controlled inputs; `useEffectEvent` is
restricted to handlers called from within Effects) and `use-debounce`'s
API, the right shape is a small, dependency-free hook.
`useDebouncedCallback(fn, delayMs)` returns a stable callable with
`.cancel()` / `.isPending()`. Timer state lives in a closure variable
inside a `useState` lazy initialiser, so:
- the returned identity never changes (safe in effect deps and memoised
children);
- there are no refs to read during render (no React Compiler complaints);
- `fn` and `delayMs` go through `useLatestRef` so the dispatched call
always sees the freshest closure — inline arrows in parents are free.
DataTableFilter / InputSearch shrink to one useState + one useRef
(`lastEmittedReference` distinguishes our own router round-trip from a
true external change) + one useEffect (external sync). All four
regression tests added in the previous commit still pass; five new unit
tests pin `useDebouncedCallback` semantics (burst coalescing, cancel,
isPending, latest-closure read, unmount cleanup). Browser repro on
/flows for both X-after-flush and X-before-flush scenarios: a single
transition `v="" u=""`, no resurrection.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previously DataTableFilter (and InputSearch) layered four cells holding
the same logical value — `localValue` (sync useState), `debouncedValue`
(async useState inside `useDebouncedValue`), `lastEmittedReference`
(ref), and the parent's `query` prop (async round-trip) — and stitched
them together with two opposing useEffect's. The async `debouncedValue`
cell could be read out of phase from any other effect, which leaked
through as four distinct races: X clear after debounce flush (URL/state
snap back ~50–80 ms), X clear *before* debounce flush (pending timer
fires after clear), external `query` change mid-debounce (pending
typed value clobbers external), and tail-end character loss on fast
typing across the round-trip boundary.
Drop the extra storage cell. The debounce is now an imperative
`pendingTimerReference` mutated only by `handleChange`,
`cancelPendingEmit`, and the external-sync effect. All outbound
emission goes through one `emit(next)` function that synchronously
cancels any pending timer before sending. `handleClear` → `emit('')`.
External `query` change → cancel pending + accept. Unmount → cancel.
`onQueryChange` lives behind `useLatestRef` so effect deps stay
stable across parent re-renders.
Four regression tests in data-table.test.tsx pin all four scenarios.
Verified by rolling back the fix temporarily — the X-clear race test
fails with the exact `emitted = ['', 'alpha', '']` from the original
browser repro. With the fix in place all four pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
DataTableFilter / InputSearch listed `onQueryChange` (an inline arrow from
the parent — fresh reference per render) in the emit effect's deps. After
the X-clear handler set `lastEmitted=''` and propagated upstream, the
parent re-rendered and handed back a new handler reference, re-running
the effect while `debouncedValue` was still the pre-clear value
(`useState` inside `useDebouncedValue` hadn't yet been updated by its
trailing `setTimeout`). The effect saw `debouncedValue='alpha' !==
lastEmitted=''` and re-emitted `'alpha'`, snapping URL/state back for
~50–80 ms. JSDOM regression captured `emitted = ['', 'alpha', '']`.
Stash the handler in `useLatestRef` and drop it from deps so the effect
fires only when the observed value actually changes. Same fix in
InputSearch's Esc-clear path. New test in data-table.test.tsx pins the
behavior — fails with the previous code (`['', 'alpha', '']`), passes
now (`['', '']`).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The JSDoc blocks added in afb9b63 mostly paraphrased the 12 lines of
code below them. Reviewing line-by-line:
- "two axes" / "filter active vs no filter" / per-state copy — code
reads identically and is right next to the comment.
- "kept inline because tightly coupled to effectiveQuery" — every
helper in this file is inline, that's the existing convention.
- "colSpan={columns.length} parent" — the one call site is 10 lines
below in the same file.
- "for callers that have not migrated yet" — `empty?:` being optional
already says this.
- "so existing tests stay green" — the test itself documents the
contract; deleting the fallback fails it loudly.
Kept the one piece TypeScript can't express: a single-line hint that
`entityName` wants the plural lowercase form so the generated copy
reads naturally mid-sentence. That shows up in IDE autocomplete and
saves a "Flows" → "No Flows match" first-try mistake.
No behaviour change. 28/28 DataTable tests pass, 0 ESLint errors.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Resolves QA report observation O9. Every list page used to render the
bare "No results." label when its DataTable had no rows — both for a
truly empty dataset and when a filter narrowed the result set to zero.
The Resources page already had a better pattern (shadcn <Empty> block
with a filter-aware copy that cited the query), so propagate it to the
shared DataTable.
API
- New optional `empty?: { entityName?: string }` prop on `DataTable`.
- When `entityName` is provided, the body cell renders an `<Empty>`
block:
filter active → Search icon + "No matches" + "No <entity> match
<code>{query}</code>. Try a different query."
no filter → Inbox icon + "No <entity> yet"
and the pagination footer mirrors the entity name ("No <entity>"
instead of "No results").
- When `entityName` is omitted, the existing bare "No results." cell
and "No results" footer are kept verbatim — preserves backward-compat
for call sites that have not migrated and for the existing test that
asserts the legacy copy.
Per-page wiring
Plural lowercase names match how a screen reader reads them mid-
sentence:
flows → "flows"
templates → "templates"
knowledges → "knowledge documents"
/settings/providers → "providers"
/settings/api-tokens → "API tokens"
/settings/prompts ×2 → "agent prompts" / "tool prompts"
Tests
- Existing `'No results.'` assertion stays valid: it renders a
DataTable without the `empty` prop, exercising the legacy fallback
path.
- Added 3 new cases under `DataTable — empty results`:
1. legacy fallback when `empty.entityName` is omitted
2. data-empty "No <entity> yet" when no filter is active
3. filter-empty "No <entity> match <query>. Try a different
query." with the query cited verbatim
Verified
- 0 ESLint errors, 44 pre-existing warnings.
- 511/511 vitest pass in sequential mode and when the affected files
run isolated. A pre-existing concurrency flake in
`detail-navigation-sheet.test.tsx` (`typing narrows the listbox
immediately when searchDebounceMs=0`) occasionally surfaces under
vitest's default `--file-parallelism` — unrelated to this change,
passes deterministically with `--no-file-parallelism` and when the
file is run in isolation.
- Browser sweep across 7 tables (`/flows`, `/templates`,
`/knowledges`, `/settings/{providers,api-tokens,prompts(×2)}`) all
render the new copy with `?q=ZZZZZZ`.
References
- shadcn `Empty` component (already vendored in
`components/ui/empty.tsx`)
- shadcn.io `tables-empty` block FAQ (best-practice: distinguish
filter-empty vs data-empty via `hasActiveFilters` boolean, ground
the empty cell in a Lucide icon, surface a "next-step" hint).
- Existing precedent in `pages/resources/resources.tsx` —
`No resources match <code>{q}</code>. Try a different query.`
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reverts the O10 part of commit 04702c3. The original change renamed
the provider clone URL parameter from `?id=` to `?clone=`. On second
look the rename solved no real problem:
- No security issue: PentAGI is a single-tenant local admin tool, so
there is no cross-tenant info leak from a shared URL.
- No bug: the form behaviour was identical before and after.
- No UX impact: the URL is generated by the "Clone" action, not typed
by users; the bare `?id=` is also explicit enough in context
(settings-provider.tsx gates it on `isNew`, so the meaning is
"source provider to clone from").
What the rename did cost: it broke backward compatibility with any
bookmarks/links saved in `?id=` form. Restore `?id=` to remove the
needless churn.
The rest of commit 04702c3 (O7 dialog case, O8 "API Tokens" unify,
O11 button label + JSDoc, O12 italic placeholder) addressed real
inconsistencies and stays in place.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Resolves QA report observations O7, O8, O10, O11, O12.
O7 — Confirm dialog title case
The Resources page rendered "Delete Directory" / "Delete Resource" in
Title Case while every other delete confirm (template, knowledge,
token) used Sentence case ("Delete template", "Delete token"). Aligned
the resources copy to "Delete directory" / "Delete resource".
O8 — /settings/api-tokens identity mismatch
Three names referred to the same page:
- sidebar link / h1: "PentAGI API"
- tab title (route registry): "API tokens"
- menuItems entry: "PentAGI API"
Unified everything to "API Tokens". Also removed the now-redundant
api-tokens special case in settings-layout's SettingsHeader (it always
returned the same value the menuItems lookup would have produced).
O10 — Provider Clone URL parameter
`/settings/providers/new?id=5` collided semantically with the rest of
the codebase, where `?id=` usually means "this is the record being
edited". Renamed to `?clone=5` so the intent is explicit when the URL
is shared or shows up in logs. Updated the reader in
settings-provider.tsx (`formQueryParams.id` → `formQueryParams.clone`)
to match.
O11 — "Create Provider" button accessibility
The button is a DropdownMenuTrigger that opens a provider-type chooser
before navigating to /new — not a submit button. Sighted users get
this from `<ChevronDown />` and Radix's `aria-haspopup="menu"`, but
the bare visible text "Create Provider" reads as a confirmation
action to screen readers. Added `aria-label="Create provider — choose
type"` and a block comment explaining the pattern so future
maintainers don't try to "fix" the dropdown into a direct submit.
O12 — `(unnamed)` placeholder rendering
Tokens with empty name rendered as literal `(unnamed)` with
text-muted-foreground only. Added `font-normal italic` so the
placeholder reads as missing data rather than a real name (the column
otherwise uses font-medium).
Verified: 0 ESLint errors, 508/508 tests pass.
Browser regression sweep across 9 pages (/dashboard, /flows,
/templates, /knowledges, /resources, /flows/:id, /settings/{providers,
api-tokens,prompts}) still returns 0 icon-only buttons without an
accessible name.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Resolves QA report BUG #16 and BUG #17.
BUG #16 — /settings/prompts/<invalid> tab title fallback
formatPromptId() blindly capitalized whatever the URL passed in, so a
nonexistent id like /settings/prompts/99999 produced the tab title
"99999 — PentAGI" (and an "Edit Prompt" header with a literal "99999"
in the error message). Valid prompt ids are camelCase identifiers
(e.g. "agentSelector"). Reject anything that does not match
/^[a-z][a-zA-Z]*$/ and fall back to a generic "Prompt", so the title
becomes "Prompt — PentAGI" for invalid ids; valid ids keep their
existing "Agent Selector — PentAGI" rendering.
BUG #17 — Chrome DevTools "form field needs id/name" + label-for
Two separate issues on /flows and /flows/🆔
1) Four hidden <input type="file"> elements driven programmatically
by use-resources-upload / use-flow-files-upload / flow-form lacked
`name`. Added descriptive names (`resource-upload`, `flow-file-upload`,
`flow-form-attachment`) plus `aria-hidden="true"` and `tabIndex={-1}`
— these inputs are never user-focused (the button triggers .click()
via a ref), so excluding them from the accessibility tree is correct.
2) <FormControl><Switch/></FormControl> + <FormLabel> for the "Use Agents"
field on the assistant flow form was rendered without a wrapping
<FormItem>, so useFormField() read the default empty context and
formItemId became "undefined-form-item". Chrome then flagged "label
for=undefined-form-item" as an orphan. Wrapped the cluster in
<FormItem className="flex flex-row items-center gap-0"> so useId()
generates a real id and the label binds to the Switch trigger
button correctly.
Verified: 0 ESLint errors, 508/508 tests pass.
On /flows the page now has 0 form fields without id/name.
On /flows/:id the only remaining unlabeled fields are third-party
internals we do not own (xterm.js helper textarea, Radix Switch
primitive checkbox, react-textarea-autosize ghost) — Chrome's count
dropped accordingly.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Page-level row-action triggers were split between two conventions:
- 4 pages used <span className="sr-only">Open menu</span> inside <Button>
- 3 pages (touched in commit 9eee2b4) followed the same pattern for consistency
Project has no i18n pipeline that benefits from sr-only text-node
translation, and the rest of the codebase already uses aria-label for
icon-only buttons (pagination, forms, theme tabs, toggles). Unify all
6 page-level row-action triggers to aria-label="Open menu":
- pages/flows/flows.tsx
- pages/knowledges/knowledges.tsx
- pages/templates/templates.tsx
- pages/settings/settings-providers.tsx
- pages/settings/settings-prompts.tsx (×2 — agent + tool tables)
- pages/settings/settings-api-tokens.tsx
shadcn UI library components (sheet, dialog, sidebar, breadcrumb) keep
their sr-only spans untouched — that is upstream shadcn convention and
not in scope here.
Verified: 0 ESLint errors, 508/508 tests pass, browser sweep across
6 list pages confirms aria-label="Open menu" rendered correctly and
no icon-only buttons remain without an accessible name.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The clamping effect compared the URL to a `safePageIndex` derived from the
internal mirror. After popstate the URL refreshes one render before the
mirror catches up; after a page-size change the mirror refreshes one
render before the URL — so each side echoed the stale source back over
the fresh one, oscillating `/flows` ↔ `/flows?page=2` indefinitely.
Now clamping only fires when both URL and mirror agree out-of-range, and
goes through a new `setPage(idx, { replace: true })` opt-in so a typed
`?page=999` no longer leaves a history entry the next back-press would
re-trigger.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Radix ScrollArea wraps children in a display:table viewport that grows
to intrinsic content, so renderItem rows with white-space:nowrap text
pushed each option past the sheet's right edge. Swap to plain
overflow-y-auto and add min-w-0 to the flex chain so truncate finally
takes effect on long flow / template / knowledge titles.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Strip section banners, "Helper function to X" labels above self-describing
identifiers, "GraphQL query/mutation/subscription" markers, and the
copy-pasted `// Create debounced function` / `// Filter by search` blocks
that lined every flow feature. Where a comment hid a non-obvious reason
(history blocker pattern, watch() vs getValues(), CJK font fallbacks),
rewrite it as a WHY comment instead of dropping it.
Net diff: -571/+78 across 57 files. Lint clean (pre-existing `any`
warnings unchanged), `tsc` errors unchanged, 506/506 tests still pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Terser strips the inner function-expression name in `apolloTitle()` during
the production build, leaving an anonymous `function({params})` whose
`fn.name === ''`. The old PascalCase heuristic in `DocumentTitle` then
misclassified the component as a `(params) => string` resolver, called it
with raw `match.params`, and crashed `variables({flowId})` with
"Cannot destructure property 'flowId' of undefined" on every /flows/:id
view in production.
Attach an explicit `Symbol.for('pentagi.apolloTitle')` marker via
`Object.assign` in the factory and expose `isApolloTitle()` as the only
public guard. Symbol-keyed properties survive Terser regardless of
`mangle.properties` settings, and `Symbol.for` keeps the marker stable
across accidental double-bundles (workers, SSR boundaries). The
`document-title` consumer now imports the guard, not the symbol, so the
marker stays an implementation detail of the apollo-title module.
Cover the regression with a dedicated `apollo-title.test.tsx` that
simulates name stripping via `Object.defineProperty(fn, 'name', { value: '' })`
and asserts the guard still recognizes the component.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
HTML's default `type` for a `<button>` inside a `<form>` is `submit`,
so every Button without an explicit `type` silently posted the parent
form on click — repro on knowledge detail: clicking the
`DetailNavigationButtons` Prev/Position/Next cluster (or the Anonymize
button) fired the form's submit handler and kicked off an
`updateKnowledge` mutation that 502'd. Mirrors the existing convention
in `InputGroupButton`. `asChild` still defers to the consumer's
element; the one genuine submit caller (`FormSubmitButton`) is
explicit. Audit confirms no other Button uses default-submit
intentionally.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`useDetailNavigation` now owns a controllable local search (debounced,
ANDed with the URL `?q=` filter) and `DetailNavigationSheet` surfaces it
as a header input by default (`hasSearch={true}`). Sheet auto-focus no
longer steals focus from the search input when `filteredItems` shrinks
on a debounce flush — gate the option-focus effect on `document.activeElement`
so typing past the boundary keeps the caret in place. ArrowDown from the
input synchronously promotes focus into the listbox.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Now that `useTableState` no longer touches storage for filter values, the
adjacent read-only API has dead optionality:
- `useTableQueryFilterReader` accepted a `storageKey` that was already
documented as "ignored here — only for API symmetry with the mutate
variant." With the symmetry gone (the mutate variant no longer takes
one), keep the reader pure-URL and drop the prop entirely.
- `useDetailNavigation` used to compute that storage key via
`usePageStorageKeys` just to pass it into the reader. The whole
`parentPath` override existed solely to feed that computation. Drop
`parentPath`, the `ParentPath` template-literal type, and the
`usePageStorageKeys` import — no caller passes `parentPath`, so the
cleanup is API-compatible.
- `useResourcesSearch` JSDoc still claimed a "localStorage fallback";
reword to describe the current URL-only behavior so the comment stops
contradicting the code.
Manually verified `/knowledges?q=Jinja` → click a row → the detail page
shows `1/11` navigation, Next steps through siblings, and `?q=Jinja`
stays in the URL — the headless controller still reads the live filter
through the reader.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous `useTableState` mirrored `?q=` into per-page localStorage
and replayed it back into the URL on mount when the URL came in clean.
The intent was "fresh tab resumes the prior search," but in practice
the storage→URL replay leaked the user's most recent filter into
unrelated visits: navigating away from a filtered `/flows?q=` and
returning via the sidebar would silently land on the same filter, often
with an empty result set the user had no reason to expect.
Filter is an ad-hoc query, not a preference. Real preferences (sorting,
column visibility, page size, search columns) still persist via
`lib/table-state` — only `filter` is dropped from the storage roundtrip:
- Remove the storage→URL restore effect.
- Remove the URL→storage write effect for `filter`.
- Drop the `skipRestore` option (no restore left to suppress) and the
`storageKey` override that only existed to scope the restore.
- Drop the conditional `skipRestore` plumbing on `/knowledges` — the
page no longer needs to defend against a stale `?q=` being replayed
on top of a fresh `?qs=` link.
- Rewrite the "storage roundtrip" test block as
"filter is URL-only": assert the storage entry is ignored on mount,
the URL still wins when both are set, and typed filter never lands
in storage.
Manually verified in the browser: `/flows?q=IDOR` → navigate away →
return to `/flows` shows the unfiltered table; sorting a column still
persists across reloads (`table_4_/flows` keeps the `sorting` field).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Flows, Providers, and API tokens pages each defined a local
`formatDateTime`/`formatFullDateTime` pair (~16 lines × 3 files) doing
essentially the same trimming-by-recency job as `formatDate` from
`@/lib/utils/format`, and each wrapped the cell in a Tooltip that
repeated the same text more verbosely.
After the previous date-format unification commit (1806956), the
trimmed cell already conveys enough context for every recency band:
- today → HH:mm:ss
- this year → HH:mm, d MMM
- older → HH:mm, d MMM yyyy
So the Tooltip+full-timestamp pair no longer adds information for the
date columns. Replace the three local copies with a direct
`formatDate(new Date(dateString))` call and drop the Tooltip wrapper
where it only mirrors the cell.
The Tooltip on the Terminals column in flows.tsx is kept — it expands
the list of terminals with per-row connection status and is not a
duplicate.
Net delta: -115 / +10 lines.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- providers/flow-provider: map "no rows in result set" to a friendly
"Flow not found" toast with a stable id so repeated sibling-query
errors no longer stack raw SQL into the UI
- components/ui/sidebar: skip the global Ctrl+B shortcut when focus is
in an input, textarea, select, or contenteditable — was hijacking the
Tiptap Bold shortcut in the markdown editor
- components/ui/sidebar: mark the mobile drawer with group + data-state
"expanded" so `group-data-[state=...]` selectors collapse correctly
and flow id badges stop rendering twice in the drawer
- pages/templates/template: add aria-label + title to the icon-only Save
button and mark the icon aria-hidden
- pages/settings/settings-api-tokens: render the date-picker label with
date-fns format "d MMM yyyy" to match the table's date format
- pages/settings/settings-providers: drop the fixed Name column width so
the Type column stays visible on narrow viewports; Badge truncates
cleanly with whitespace-nowrap and shrink-0 on the provider icon
- components/shared/confirmation-dialog: derive the title from
`\${confirmText} \${itemType}` so dialogs surface "Delete template"
instead of a generic "Confirm Action", and use the verb in the
description for consistency
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- DataTableFilter no longer renders a leading `<Search>` addon. With
`InputSearch` now living in the same page header as the trigger for
semantic search, two magnifier icons next to each other read as
visual clutter — the placeholder text alone already cues the input's
purpose. Drops the unused `Search` import too.
- The "Flags" column header span gets `w-full` so its
`justify-end` actually pushes the label flush against the right edge
of the cell. Without the explicit width it collapsed to content
size and the alignment had no axis to act on.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Typing fast into the knowledges header search dropped characters: each
keystroke went straight into the parent's `onSearchChange`, which on
this page funnels through `setSearchParams` → router re-render → the
controlled `searchQuery` prop snapped back through React. When the
round-trip outran the user's fingers, the input's visible value rolled
back and chars between commits were lost ("find" → "fn").
Mirrored the `DataTableFilter` pattern: hold a `localValue` state that
keystrokes update synchronously, debounce the commit upstream
(`useDebouncedValue`, 150 ms — same constant as the table filter so
both inputs on this page feel uniform), and reconcile external resets
(Esc, programmatic URL wipe, back button) via a `lastEmittedReference`
guard that ignores our own round-trips.
Blur and Escape now also read `localValue` instead of `searchQuery` so
keystrokes pending in the debounce window aren't treated as "nothing
typed yet". Escape additionally fast-paths an empty commit upstream so
the clear hits the parent without waiting for the debounce.
Measurements (chrome-devtools, 19-keystroke burst): pre-fix typing
dropped characters, post-fix avg keystroke 2.07 ms, p95 6 ms, 0
keystrokes over the 16 ms 60-fps budget. Server `searchKnowledge`
still fires exactly once per typing pause.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Kbd inside tooltip-content now uses the `accent` token family
(`bg-accent/20`, `text-accent-foreground`, dark `bg-accent/50`) for
better legibility against the popover background. The previous
`bg-background/20` chips read as thin film on dark tooltips.
- The empty-state "New Knowledge" button on the knowledges page passes
`<Plus />` without an explicit `size-4` — the Button variant already
sizes its icon slot, the override was duplicating defaults.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Children like Kbd target tooltip surfaces via
\`[[data-slot=tooltip-content]_&]:bg-background/20\` selectors so the
chips can re-tone themselves for the dark popover background. Our
TooltipContent was missing that attribute (the file shipped from a
pre-v4 cut of shadcn), so the override never matched and Kbd kept its
muted page-surface styling — invisible on the tooltip.
Set \`data-slot\` on every Tooltip primitive (provider / root /
trigger / content) to match the v4 contract. Verified in DevTools:
Kbd inside tooltip-content now computes the inverted background and
foreground colors.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
shadcn ships the canonical Kbd-inside-Tooltip pattern (per the
upstream kbd-tooltip demo and our existing components/ui/kbd.tsx).
The Kbd component already self-themes for tooltip-content surfaces
via `[[data-slot=tooltip-content]_&]` overrides, so swapping the
plain "(⌘ K)" suffix for `<KbdGroup><Kbd>⌘</Kbd><Kbd>K</Kbd></KbdGroup>`
matches the rest of the app and renders proper keycap chips against
the tooltip background.
Verified the accessibility-description path: Radix still surfaces the
flattened "Semantic search... ⌘ K" string to screen readers (we saw
it in the a11y snapshot), so the keyboard hint stays announced.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The collapsed trigger is just a magnifier icon — there's no visible
hint about what it does or which key reveals it. Added a tooltip that
combines the `placeholder` with the platform-appropriate hotkey glyph
(⌘ on Apple, Ctrl elsewhere) using the existing `isMac()` helper. When
`hotkey` is `null` the suffix drops and the tooltip shows just the
placeholder.
Tooltip wraps only the trigger button: once the input is expanded the
caret is in it and a hovering tooltip would obscure the very thing the
user is typing into.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The original sofy-front version collapsed the control whenever focus
left an empty input. When the duplicate blur+effect pair was folded
into a single effect during the port, that behaviour silently went
with it — opening the trigger without typing and then clicking
elsewhere left the box expanded forever, because `searchQuery` never
changed and the effect never re-ran.
Re-added a dedicated `onBlur` handler that collapses on focus loss
when the value is empty. The effect stays as the safety net for the
programmatic-clear case (parent wipes `?qs=` from the URL while focus
is elsewhere) — different trigger, different path, neither path can
cover the other.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`HeaderButton` defaults to `size='sm'` (h-8 px-3 rounded-md text-xs)
and pages mount InputSearch right next to it. The component previously
shipped with h-7 / icon-xs (28 px) so the icon-only trigger sat a
notch shorter than its sibling action — visible in the knowledges
header.
Bumped the group to h-8, the trigger to size='icon-sm', and pinned
the inner input to h-8 (it inherits `Input`'s default h-9 otherwise,
which pokes 4 px past the bordered group). Dropped the duplicate
negative margins on the button — the addon's
`has-[>button]:ml-[-0.45rem]` already handles flush-left alignment
when expanded; only the collapsed state needs the explicit `-mx-1.5`
to claw back the addon's default padding around the lone icon.
Verified: trigger 32×32, group 32 px, input 32 px, all matching the
neighbouring `HeaderButton` baseline.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
New `components/ui/input-search.tsx`: a controlled, collapsible search
control that idles as just a `<Search>` icon button and expands into a
text input with a motion-driven width animation. Adapted from the
sofy-front version with:
- Trigger is a real `<button>` (via `InputGroupButton`) instead of a
`<div onClick>` — keyboard reachable, screen-reader announced, and
avoids the invalid-HTML nesting that putting an `<input>` inside a
`<button>` would create.
- Configurable Mod+`hotkey` (default `'f'`, `null` to disable). The
knowledges page uses `'k'` so it doesn't fight the browser's own
Ctrl+F find-in-page.
- Escape clears the value on the first press, blurs + collapses on the
second — matches native search-input muscle memory.
- Single collapse effect on `searchQuery` change instead of the
duplicate blur+effect pair.
- `aria-label` prop wired to both the trigger button and the input.
The knowledges header now mounts this control bound to `?qs=` via
`useSearchParams`. The provider already debounces the URL value before
hitting `searchKnowledge`, so the input commits keystrokes immediately
(instant UI feedback) while the server roundtrip waits for the 400 ms
debounce. URL writes use `replace` to avoid history-stack churn.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When `/knowledges` is opened via a `?qs=…` link, `useTableState`'s
mount-time restore was injecting the previous session's `?q=` from
storage on top of it — so a clean semantic-search URL silently became
`?qs=foo&q=python` and the user got a narrowed result set they didn't
ask for.
Added `skipRestore` to `useTableState` (defaults to `false`, captured
via a ref at mount so a later toggle doesn't replay a restore the user
has worked past). `knowledges.tsx` passes `searchParams.has('qs')` so
the restore is skipped exactly when the parameter is part of the
landing URL.
Mirror-to-storage on subsequent filter changes is unaffected — a
client filter the user actually types still persists to storage for
the next session.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Knowledges list previously sourced everything from `knowledgeDocuments`.
When `?qs=` is present the provider now switches to `searchKnowledge`
(debounced 400ms, capped at 100 results) and feeds the resulting subset
through the same public `knowledges` array — so DataTable, the existing
`?q=` client filter, and DetailNavigation continue to walk it without
any consumer-side change.
Subscription cache merge in `lib/apollo.ts` only covers the
`knowledgeDocuments` root field; in search mode CRUD events trigger a
`refetchQueries(['searchKnowledge'])` via `useLatestRef` so the relevance
ordering stays consistent without freezing the boolean at mount.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mirrors the entry already present in the right-click context menu so
both menus expose the same set of actions (Edit / Copy Token ID /
Delete) — the dropdown was previously missing the copy shortcut.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Long-press on iOS/Android starts native text selection before Radix
ContextMenu reacts, so the row appears highlighted from the top of the
table down to the finger. Apply `user-select: none` and disable the iOS
callout on those rows only, and only on coarse pointers — desktop users
keep the ability to select cell text with the mouse.
Pattern matches the workaround discussed in radix-ui/primitives#930.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Hover relies on the cursor staying over the <tr>, but Radix portals the
overlay so the row dims as soon as the menu opens. Match the same
background via `data-[state=open]` (ContextMenuTrigger forwards
`data-state="open"` onto the row through `asChild`) and
`has-[[data-state=open]]` (covers any Dropdown/Popover triggered from a
cell). Switch the selection prop to a conditional spread so the prior
`data-state={false}` no longer wins over Radix Slot's merged props.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Single source of truth for every route's tab title. All title knowledge —
static strings, params-derived resolvers, and Apollo-cache-driven reactive
components — now lives in src/lib/route-titles/, and app.tsx imports only
the `routeTitles` object. The router file no longer pulls Apollo hooks
or formatters; it just spreads handles onto routes.
Handle API collapsed from two keys to one. Following the canonical
`useMatches` example from React Router docs, `handle.title` is now a
single field that accepts three forms:
• `string` — static, known at build time
• `(params) => string` — synchronous resolver from URL params
• `ComponentType<{ params }>` — reactive (e.g. Apollo cache subscription)
DocumentTitle distinguishes resolver vs component by the PascalCase
convention (component names start with an uppercase letter), so the
choice is implicit at the call site — no `titleComponent` field, no
discriminated union, no marker property.
Boilerplate consolidation:
- 4 near-clone resource-title components (Flow/Knowledge/Provider/
Template) collapse into 4 declarative `apolloTitle({ useQuery,
variables, select })` calls in the registry. The factory owns the
`fetchPolicy: 'cache-only'` plumbing, skip semantics, and component
naming.
- renderTitle/APP_NAME/RouteParams were duplicated between
document-title.tsx and resource-titles.tsx — now imported from one
render-title.tsx.
- formatPromptId moves into the route-titles module since the prompt
detail page and the route handle are its only consumers.
References to React docs added to document-title.tsx: only one <title>
should be rendered from React at a time (multiple → undefined browser/
SEO behavior). The convention is now enforceable from one place.
All 482 unit tests pass; lint, tsc, prettier clean. Verified in the
browser across every route — static (Dashboard, Flows, Resources, …),
/new variants (New flow, New provider, …), params resolver
(/settings/prompts/agentSelector → "Agent Selector"), and Apollo
reactive (/flows/491 → "Flow #491 — Login Beta Program"), including
sibling navigation between detail routes which previously motivated
moving DocumentTitle into the shell.
Net diff: -171 / +66 across the touched files plus the new registry.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Match the redirect pattern used on the flow page: instead of rendering
a dedicated "not found" card with a manual back button, show a toast
and replace the route with /knowledges so the user lands on the list
the same way other resources behave.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Single mechanism for the document <title>. Every route now declares its
title in the react-router handle:
• handle: { title: 'Dashboard' } — static listing pages
• handle: { title: (params) => formatPromptId(params.promptId) } —
params-derived (e.g. /settings/prompts/:promptId)
• handle: { titleComponent: TemplateTitle } — Apollo cache-driven,
reactive to cache updates after the page mounts
A single <DocumentTitle/> in RootLayout walks useMatches() and picks
the deepest match. PageTitle is gone — 13 call sites across 13 page
components migrated to handle, the component deleted.
Other improvements:
- Split document-title.tsx so the shell-only <DocumentTitle/> has zero
graphql imports — the resource title components (FlowTitle,
KnowledgeTitle, ProviderTitle, TemplateTitle) live in
resource-titles.tsx. This lets the unit tests render DocumentTitle
without dragging the codegenerated graphql/types.ts through Vite.
- String(p.id) === providerId replaces sloppy == in ProviderTitle.
- matches.findLast(...) replaces the imperative for-loop.
- Extracted formatPromptId into lib/utils/format-prompt-id.ts so the
prompt detail page and the route handle resolver share one formatter.
7 unit tests added for DocumentTitle covering: static title, derived
title from params, deepest-match-wins, titleComponent rendering,
titleComponent precedence over static title, empty-string fallback,
and no-handle fallback.
19 test files / 482 tests pass; lint + tsc clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Move <title> ownership out of detail-page components into the app
shell. The four detail routes — templates/:templateId,
knowledges/:knowledgeId, settings/providers/:providerId,
flows/:flowId — now expose a titleComponent via react-router
handle. A new <DocumentTitle/> in RootLayout walks useMatches()
deepest-first and renders the matched title component. Each one
subscribes to its resource via Apollo with fetchPolicy:
'cache-only', so it reacts to the destination page's own fetch
without issuing a duplicate request.
Fixes the root cause behind bbf943e's sticky workaround: navigation
between sibling documents (DetailNavigation prev/next) tore down
the page-level state that PageTitle held, flashing a generic
fallback during data fetch. The shell-level <DocumentTitle/>
survives the remount, so the title resolves from cache before the
new page has finished mounting.
- Add src/components/shared/document-title.tsx with DocumentTitle
plus FlowTitle / KnowledgeTitle / ProviderTitle / TemplateTitle.
- Wire <DocumentTitle/> into RootLayout and attach
handle={{ titleComponent: ... }} on the four detail routes.
- Remove <PageTitle> calls and now-redundant <> wrappers from the
four page components.
- Drop the sticky setState-during-render hack from PageTitle —
listing pages that still use it never flickered, so the simple
passthrough is enough.
Lint + tsc clean, all 475 tests pass. Listing routes (Dashboard,
Flows, Templates, Knowledges, etc.) keep using PageTitle and can
migrate route-by-route without coordination.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace the two ad-hoc useState form objects (createFormData /
editFormData) in settings-api-tokens.tsx with parent-level useForm
hooks per inline row, each backed by a zod schema. Controller in
each DataTable cell re-subscribes to the parent form state on
remount, so subscription-driven row remounts no longer drop user
input — same root cause that 04c6825 fixed, but solved at the
architectural level instead of controlling each Input manually.
- tokenNameSchema (trim + max 255), createTokenFormSchema with
required expiresAt via refine, editTokenFormSchema with status.
- <Input>, <Select> and <Calendar> cells rendered through
<Controller>; RHF owns the values, not React state in the parent.
- CreateRowActions / EditRowActions subcomponents own the
useFormState({ control }).isValid subscription so the Save button
re-renders only the small action subtree on keystrokes instead of
the whole table.
- Ad-hoc required/trim checks dropped from handlers; the schema +
isValid gate handle them. Submit still applies trim() || null
because the GraphQL mutation treats null as "no name."
All 18 test files / 475 tests pass; lint + tsc clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sergey Kozyrenko