chore: import upstream snapshot with attribution
Backend Tests / Tests (other) (push) Failing after 1s
Backend Tests / Static Checks (push) Failing after 2s
Backend Tests / Tests (internal) (push) Failing after 2s
Backend Tests / Tests (server) (push) Failing after 1s
Backend Tests / Tests (store) (push) Failing after 1s
Build Canary Image / build-frontend (push) Failing after 1s
Frontend Tests / Lint (push) Failing after 1s
Build Canary Image / build-push (linux/amd64) (push) Has been skipped
Build Canary Image / build-push (linux/arm64) (push) Has been skipped
Build Canary Image / merge (push) Has been skipped
Frontend Tests / Build (push) Failing after 1s
Release Please / release-please (push) Failing after 0s
Proto Linter / Lint Protos (push) Failing after 2s
Backend Tests / Tests (other) (push) Failing after 1s
Backend Tests / Static Checks (push) Failing after 2s
Backend Tests / Tests (internal) (push) Failing after 2s
Backend Tests / Tests (server) (push) Failing after 1s
Backend Tests / Tests (store) (push) Failing after 1s
Build Canary Image / build-frontend (push) Failing after 1s
Frontend Tests / Lint (push) Failing after 1s
Build Canary Image / build-push (linux/amd64) (push) Has been skipped
Build Canary Image / build-push (linux/arm64) (push) Has been skipped
Build Canary Image / merge (push) Has been skipped
Frontend Tests / Build (push) Failing after 1s
Release Please / release-please (push) Failing after 0s
Proto Linter / Lint Protos (push) Failing after 2s
This commit is contained in:
File diff suppressed because it is too large
Load Diff
@@ -0,0 +1,738 @@
|
||||
# Calendar-Date Memo Prefill — 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:** When the user picks a date in the activity calendar, the home memo editor pre-fills its `createTime`/`updateTime` to that date and exposes the existing `TimestampPopover` so the user can adjust before saving. Empty calendar dates also become clickable.
|
||||
|
||||
**Architecture:** Frontend-only. A new pure helper derives a `Date` from the `displayTime` filter; `PagedMemoList` reads `MemoFilterContext` and passes the derived `Date` into `MemoEditor` via a new `defaultCreateTime` prop; `MemoEditor` seeds its reducer state from the prop, renders the popover when the prop is set in create mode, and re-syncs on prop change. `CalendarCell` drops the `count > 0` gate on click handling.
|
||||
|
||||
**Tech Stack:** React 18 + TypeScript, Vite 7, Vitest + jsdom + @testing-library/react, Tailwind v4, react-router. State via React Context + `useReducer`.
|
||||
|
||||
**Spec:** `docs/superpowers/specs/2026-05-02-calendar-date-prefill-design.md`
|
||||
|
||||
---
|
||||
|
||||
## File map
|
||||
|
||||
**Created**
|
||||
|
||||
- `web/src/components/MemoEditor/utils/deriveDefaultCreateTime.ts` — pure helper `(filters, now?) => Date | undefined`.
|
||||
- `web/tests/derive-default-create-time.test.ts` — Vitest unit tests for the helper.
|
||||
- `web/tests/calendar-cell-empty-clickable.test.tsx` — RTL test that count=0 in-month cells are clickable.
|
||||
|
||||
**Modified**
|
||||
|
||||
- `web/src/components/ActivityCalendar/CalendarCell.tsx` — drop `day.count > 0` gate from click/interactivity/tooltip.
|
||||
- `web/src/components/MemoEditor/types/components.ts` — add `defaultCreateTime?: Date` to `MemoEditorProps`.
|
||||
- `web/src/components/MemoEditor/hooks/useMemoInit.ts` — accept optional `defaultCreateTime`; in create mode, dispatch `SET_TIMESTAMPS` to seed `{ createTime, updateTime }`.
|
||||
- `web/src/components/MemoEditor/index.tsx` — accept the new prop, pass it to `useMemoInit`, add a `useEffect` that re-syncs timestamps when the prop changes in create mode, render `TimestampPopover` when `(!memo && state.timestamps.createTime)`.
|
||||
- `web/src/components/PagedMemoList/PagedMemoList.tsx` — read `useMemoFilterContext`, derive `defaultCreateTime`, pass to `<MemoEditor>` at line ~155.
|
||||
|
||||
---
|
||||
|
||||
## Task 1: Pure helper `deriveDefaultCreateTimeFromFilters`
|
||||
|
||||
**Files:**
|
||||
- Create: `web/src/components/MemoEditor/utils/deriveDefaultCreateTime.ts`
|
||||
- Test: `web/tests/derive-default-create-time.test.ts`
|
||||
|
||||
The helper takes the `filters` array from `MemoFilterContext` plus an injectable `now`, finds any `displayTime` filter (value format `YYYY-MM-DD`, local-date — produced by `useDateFilterNavigation` which forwards the `dayjs().format("YYYY-MM-DD")` string from `CalendarDayCell.date`), and returns a `Date` of `selected_date + now's hh:mm:ss`. Returns `undefined` if no `displayTime` filter or the value is malformed.
|
||||
|
||||
- [ ] **Step 1: Write the failing tests**
|
||||
|
||||
Create `web/tests/derive-default-create-time.test.ts`:
|
||||
|
||||
```ts
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { deriveDefaultCreateTimeFromFilters } from "@/components/MemoEditor/utils/deriveDefaultCreateTime";
|
||||
import type { MemoFilter } from "@/contexts/MemoFilterContext";
|
||||
|
||||
describe("deriveDefaultCreateTimeFromFilters", () => {
|
||||
const now = new Date(2026, 4, 2, 14, 32, 10); // 2026-05-02 14:32:10 local
|
||||
|
||||
it("returns undefined when no filters are set", () => {
|
||||
expect(deriveDefaultCreateTimeFromFilters([], now)).toBeUndefined();
|
||||
});
|
||||
|
||||
it("returns undefined when no displayTime filter is present", () => {
|
||||
const filters: MemoFilter[] = [
|
||||
{ factor: "tagSearch", value: "work" },
|
||||
{ factor: "pinned", value: "true" },
|
||||
];
|
||||
expect(deriveDefaultCreateTimeFromFilters(filters, now)).toBeUndefined();
|
||||
});
|
||||
|
||||
it("merges the displayTime date with the current local hh:mm:ss", () => {
|
||||
const filters: MemoFilter[] = [{ factor: "displayTime", value: "2025-05-01" }];
|
||||
const result = deriveDefaultCreateTimeFromFilters(filters, now);
|
||||
expect(result).toBeDefined();
|
||||
expect(result!.getFullYear()).toBe(2025);
|
||||
expect(result!.getMonth()).toBe(4); // May (0-indexed)
|
||||
expect(result!.getDate()).toBe(1);
|
||||
expect(result!.getHours()).toBe(14);
|
||||
expect(result!.getMinutes()).toBe(32);
|
||||
expect(result!.getSeconds()).toBe(10);
|
||||
});
|
||||
|
||||
it("ignores extra non-displayTime filters", () => {
|
||||
const filters: MemoFilter[] = [
|
||||
{ factor: "tagSearch", value: "work" },
|
||||
{ factor: "displayTime", value: "2025-05-01" },
|
||||
{ factor: "pinned", value: "true" },
|
||||
];
|
||||
const result = deriveDefaultCreateTimeFromFilters(filters, now);
|
||||
expect(result?.getDate()).toBe(1);
|
||||
});
|
||||
|
||||
it("returns undefined for a malformed YYYY-MM-DD value", () => {
|
||||
const cases: MemoFilter[][] = [
|
||||
[{ factor: "displayTime", value: "not-a-date" }],
|
||||
[{ factor: "displayTime", value: "2025-13-40" }],
|
||||
[{ factor: "displayTime", value: "" }],
|
||||
[{ factor: "displayTime", value: "2025-5-1" }], // single-digit month/day
|
||||
];
|
||||
for (const filters of cases) {
|
||||
expect(deriveDefaultCreateTimeFromFilters(filters, now)).toBeUndefined();
|
||||
}
|
||||
});
|
||||
|
||||
it("uses real `new Date()` when `now` is omitted", () => {
|
||||
const filters: MemoFilter[] = [{ factor: "displayTime", value: "2025-05-01" }];
|
||||
const before = new Date();
|
||||
const result = deriveDefaultCreateTimeFromFilters(filters);
|
||||
const after = new Date();
|
||||
expect(result).toBeDefined();
|
||||
// Time-of-day should fall between before and after (within 1s tolerance).
|
||||
const resultTimeOnly = result!.getHours() * 3600 + result!.getMinutes() * 60 + result!.getSeconds();
|
||||
const beforeTimeOnly = before.getHours() * 3600 + before.getMinutes() * 60 + before.getSeconds();
|
||||
const afterTimeOnly = after.getHours() * 3600 + after.getMinutes() * 60 + after.getSeconds();
|
||||
// Handle midnight rollover by allowing any value if before > after.
|
||||
if (beforeTimeOnly <= afterTimeOnly) {
|
||||
expect(resultTimeOnly).toBeGreaterThanOrEqual(beforeTimeOnly);
|
||||
expect(resultTimeOnly).toBeLessThanOrEqual(afterTimeOnly);
|
||||
}
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run tests to verify they fail**
|
||||
|
||||
Run: `cd web && pnpm test derive-default-create-time`
|
||||
Expected: FAIL — module `@/components/MemoEditor/utils/deriveDefaultCreateTime` does not exist.
|
||||
|
||||
- [ ] **Step 3: Implement the helper**
|
||||
|
||||
Create `web/src/components/MemoEditor/utils/deriveDefaultCreateTime.ts`:
|
||||
|
||||
```ts
|
||||
import type { MemoFilter } from "@/contexts/MemoFilterContext";
|
||||
|
||||
const DATE_RE = /^(\d{4})-(\d{2})-(\d{2})$/;
|
||||
|
||||
/**
|
||||
* Derive a default `createTime` for a new memo from the active memo filters.
|
||||
* If a `displayTime:YYYY-MM-DD` filter is present, returns that local date
|
||||
* combined with `now`'s wall-clock hh:mm:ss. Returns undefined otherwise or
|
||||
* when the filter value is malformed.
|
||||
*/
|
||||
export function deriveDefaultCreateTimeFromFilters(
|
||||
filters: MemoFilter[],
|
||||
now: Date = new Date(),
|
||||
): Date | undefined {
|
||||
const dateFilter = filters.find((f) => f.factor === "displayTime");
|
||||
if (!dateFilter) return undefined;
|
||||
const match = DATE_RE.exec(dateFilter.value);
|
||||
if (!match) return undefined;
|
||||
const year = Number(match[1]);
|
||||
const month = Number(match[2]);
|
||||
const day = Number(match[3]);
|
||||
// Construct a local-time Date and verify the components round-trip
|
||||
// (catches things like 2025-13-40 that JS would silently roll forward).
|
||||
const candidate = new Date(year, month - 1, day, now.getHours(), now.getMinutes(), now.getSeconds());
|
||||
if (
|
||||
candidate.getFullYear() !== year ||
|
||||
candidate.getMonth() !== month - 1 ||
|
||||
candidate.getDate() !== day
|
||||
) {
|
||||
return undefined;
|
||||
}
|
||||
return candidate;
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run tests to verify they pass**
|
||||
|
||||
Run: `cd web && pnpm test derive-default-create-time`
|
||||
Expected: PASS — all 6 cases.
|
||||
|
||||
- [ ] **Step 5: Run lint**
|
||||
|
||||
Run: `cd web && pnpm lint`
|
||||
Expected: PASS (TypeScript + Biome happy).
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add web/src/components/MemoEditor/utils/deriveDefaultCreateTime.ts web/tests/derive-default-create-time.test.ts
|
||||
git commit -m "feat(memo-editor): add deriveDefaultCreateTimeFromFilters helper"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 2: Make empty calendar cells clickable
|
||||
|
||||
**Files:**
|
||||
- Modify: `web/src/components/ActivityCalendar/CalendarCell.tsx`
|
||||
- Test: `web/tests/calendar-cell-empty-clickable.test.tsx`
|
||||
|
||||
`CalendarCell` currently gates `handleClick`, `isInteractive`, and `shouldShowTooltip` on `day.count > 0`. Drop those gates so any in-month cell is clickable when `onClick` is provided. Out-of-month cells (early-returned at line ~38) stay unclickable.
|
||||
|
||||
- [ ] **Step 1: Write the failing test**
|
||||
|
||||
Create `web/tests/calendar-cell-empty-clickable.test.tsx`:
|
||||
|
||||
```tsx
|
||||
import { fireEvent, render, screen } from "@testing-library/react";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { CalendarCell } from "@/components/ActivityCalendar/CalendarCell";
|
||||
import type { CalendarDayCell } from "@/components/ActivityCalendar/types";
|
||||
|
||||
const makeDay = (overrides: Partial<CalendarDayCell> = {}): CalendarDayCell => ({
|
||||
date: "2025-05-01",
|
||||
label: "1",
|
||||
count: 0,
|
||||
isCurrentMonth: true,
|
||||
isToday: false,
|
||||
isSelected: false,
|
||||
...overrides,
|
||||
});
|
||||
|
||||
describe("CalendarCell empty-day clickability", () => {
|
||||
it("fires onClick for an in-month day with count=0", () => {
|
||||
const onClick = vi.fn();
|
||||
render(<CalendarCell day={makeDay()} maxCount={5} tooltipText="May 1, 2025" onClick={onClick} />);
|
||||
|
||||
const button = screen.getByRole("button", { name: /May 1, 2025/ });
|
||||
fireEvent.click(button);
|
||||
|
||||
expect(onClick).toHaveBeenCalledWith("2025-05-01");
|
||||
});
|
||||
|
||||
it("renders an empty in-month day as interactive (tabIndex 0, not aria-disabled)", () => {
|
||||
render(<CalendarCell day={makeDay()} maxCount={5} tooltipText="May 1, 2025" onClick={() => {}} />);
|
||||
|
||||
const button = screen.getByRole("button", { name: /May 1, 2025/ });
|
||||
expect(button).toHaveAttribute("tabindex", "0");
|
||||
expect(button).toHaveAttribute("aria-disabled", "false");
|
||||
});
|
||||
|
||||
it("still renders a populated in-month day as interactive", () => {
|
||||
const onClick = vi.fn();
|
||||
render(<CalendarCell day={makeDay({ count: 3 })} maxCount={5} tooltipText="May 1, 2025" onClick={onClick} />);
|
||||
|
||||
fireEvent.click(screen.getByRole("button", { name: /May 1, 2025/ }));
|
||||
expect(onClick).toHaveBeenCalledWith("2025-05-01");
|
||||
});
|
||||
|
||||
it("does not render out-of-month days as interactive (no role=button)", () => {
|
||||
render(
|
||||
<CalendarCell
|
||||
day={makeDay({ isCurrentMonth: false })}
|
||||
maxCount={5}
|
||||
tooltipText="May 1, 2025"
|
||||
onClick={() => {}}
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(screen.queryByRole("button")).toBeNull();
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run test to verify it fails**
|
||||
|
||||
Run: `cd web && pnpm test calendar-cell-empty-clickable`
|
||||
Expected: FAIL — first two tests fail because the empty cell currently has `tabindex="-1"`, `aria-disabled="true"`, and `onClick` is not invoked.
|
||||
|
||||
- [ ] **Step 3: Edit `CalendarCell.tsx` to drop the count gate**
|
||||
|
||||
Modify `web/src/components/ActivityCalendar/CalendarCell.tsx`. Three edits:
|
||||
|
||||
(a) Replace `handleClick`:
|
||||
|
||||
```tsx
|
||||
const handleClick = () => {
|
||||
if (onClick) {
|
||||
onClick(day.date);
|
||||
}
|
||||
};
|
||||
```
|
||||
|
||||
(b) Replace `isInteractive`:
|
||||
|
||||
```tsx
|
||||
const isInteractive = Boolean(onClick);
|
||||
```
|
||||
|
||||
(c) Replace `shouldShowTooltip`:
|
||||
|
||||
```tsx
|
||||
const shouldShowTooltip = tooltipText && !disableTooltip;
|
||||
```
|
||||
|
||||
Leave the out-of-month early return (`if (!day.isCurrentMonth) { ... }`) untouched — out-of-month cells remain non-buttons.
|
||||
|
||||
- [ ] **Step 4: Run test to verify it passes**
|
||||
|
||||
Run: `cd web && pnpm test calendar-cell-empty-clickable`
|
||||
Expected: PASS — all 4 cases.
|
||||
|
||||
- [ ] **Step 5: Run the full test suite to catch regressions**
|
||||
|
||||
Run: `cd web && pnpm test`
|
||||
Expected: PASS. The existing `activity-calendar-tooltip.test.ts` covers `getTooltipText` (a separate utility) and shouldn't be affected.
|
||||
|
||||
- [ ] **Step 6: Run lint**
|
||||
|
||||
Run: `cd web && pnpm lint`
|
||||
Expected: PASS.
|
||||
|
||||
- [ ] **Step 7: Commit**
|
||||
|
||||
```bash
|
||||
git add web/src/components/ActivityCalendar/CalendarCell.tsx web/tests/calendar-cell-empty-clickable.test.tsx
|
||||
git commit -m "feat(activity-calendar): allow clicking empty in-month dates"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 3: Add `defaultCreateTime` prop to `MemoEditorProps`
|
||||
|
||||
**Files:**
|
||||
- Modify: `web/src/components/MemoEditor/types/components.ts`
|
||||
|
||||
Type-only change. No tests at this step — TypeScript compiler is the gate. Subsequent tasks consume the prop.
|
||||
|
||||
- [ ] **Step 1: Add the prop**
|
||||
|
||||
Modify `web/src/components/MemoEditor/types/components.ts`. Replace the `MemoEditorProps` interface (lines 6–16) with:
|
||||
|
||||
```ts
|
||||
export interface MemoEditorProps {
|
||||
className?: string;
|
||||
cacheKey?: string;
|
||||
placeholder?: string;
|
||||
/** Existing memo to edit. When provided, the editor initializes from it without fetching. */
|
||||
memo?: Memo;
|
||||
parentMemoName?: string;
|
||||
autoFocus?: boolean;
|
||||
/**
|
||||
* Default `createTime` for a *new* memo (create mode only). When set, the
|
||||
* editor seeds both `createTime` and `updateTime` to this value and renders
|
||||
* the timestamp popover so the user can adjust before saving. Tracked live:
|
||||
* if the prop changes after mount, the editor's timestamps re-sync. Ignored
|
||||
* in edit mode (when `memo` is set).
|
||||
*/
|
||||
defaultCreateTime?: Date;
|
||||
onConfirm?: (memoName: string) => void;
|
||||
onCancel?: () => void;
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Verify compilation**
|
||||
|
||||
Run: `cd web && pnpm lint`
|
||||
Expected: PASS (no consumer changes yet, prop is optional).
|
||||
|
||||
- [ ] **Step 3: Commit**
|
||||
|
||||
```bash
|
||||
git add web/src/components/MemoEditor/types/components.ts
|
||||
git commit -m "feat(memo-editor): add defaultCreateTime prop type"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 4: Seed editor timestamps in `useMemoInit` (create mode)
|
||||
|
||||
**Files:**
|
||||
- Modify: `web/src/components/MemoEditor/hooks/useMemoInit.ts`
|
||||
|
||||
`useMemoInit` currently handles edit mode (`if (memo)`) by calling `memoService.fromMemo(memo)` and `actions.initMemo(...)`. In create mode it only restores cached content and sets default visibility — it never touches timestamps. Extend the create branch so that when `defaultCreateTime` is set, it dispatches `SET_TIMESTAMPS` with `{ createTime: defaultCreateTime, updateTime: defaultCreateTime }`. This handles the *initial mount* case.
|
||||
|
||||
- [ ] **Step 1: Update `UseMemoInitOptions` and `useMemoInit`**
|
||||
|
||||
Modify `web/src/components/MemoEditor/hooks/useMemoInit.ts`. Replace the entire file with:
|
||||
|
||||
```ts
|
||||
import { useEffect, useRef, useState } from "react";
|
||||
import type { Memo, Visibility } from "@/types/proto/api/v1/memo_service_pb";
|
||||
import type { EditorRefActions } from "../Editor";
|
||||
import { cacheService, memoService } from "../services";
|
||||
import { useEditorContext } from "../state";
|
||||
|
||||
interface UseMemoInitOptions {
|
||||
editorRef: React.RefObject<EditorRefActions | null>;
|
||||
memo?: Memo;
|
||||
cacheKey?: string;
|
||||
username: string;
|
||||
autoFocus?: boolean;
|
||||
defaultVisibility?: Visibility;
|
||||
defaultCreateTime?: Date;
|
||||
}
|
||||
|
||||
export const useMemoInit = ({
|
||||
editorRef,
|
||||
memo,
|
||||
cacheKey,
|
||||
username,
|
||||
autoFocus,
|
||||
defaultVisibility,
|
||||
defaultCreateTime,
|
||||
}: UseMemoInitOptions) => {
|
||||
const { actions, dispatch } = useEditorContext();
|
||||
const initializedRef = useRef(false);
|
||||
const [isInitialized, setIsInitialized] = useState(false);
|
||||
|
||||
useEffect(() => {
|
||||
if (initializedRef.current) return;
|
||||
initializedRef.current = true;
|
||||
const key = cacheService.key(username, cacheKey);
|
||||
|
||||
if (memo) {
|
||||
const initialState = memoService.fromMemo(memo);
|
||||
cacheService.clear(key);
|
||||
dispatch(actions.initMemo(initialState));
|
||||
} else {
|
||||
const cachedContent = cacheService.load(key);
|
||||
if (cachedContent) {
|
||||
dispatch(actions.updateContent(cachedContent));
|
||||
}
|
||||
if (defaultVisibility !== undefined) {
|
||||
dispatch(actions.setMetadata({ visibility: defaultVisibility }));
|
||||
}
|
||||
if (defaultCreateTime) {
|
||||
dispatch(actions.setTimestamps({ createTime: defaultCreateTime, updateTime: defaultCreateTime }));
|
||||
}
|
||||
}
|
||||
|
||||
if (autoFocus) {
|
||||
setTimeout(() => editorRef.current?.focus(), 100);
|
||||
}
|
||||
|
||||
setIsInitialized(true);
|
||||
}, [memo, cacheKey, username, autoFocus, defaultVisibility, defaultCreateTime, actions, dispatch, editorRef]);
|
||||
|
||||
return { isInitialized };
|
||||
};
|
||||
```
|
||||
|
||||
Notes:
|
||||
- The `defaultCreateTime` dependency is added to the effect's deps to satisfy the linter, but `initializedRef` ensures the body runs only once. Live re-sync after mount is handled by a separate effect in Task 5.
|
||||
- Edit mode is unchanged — `defaultCreateTime` is intentionally ignored when `memo` is set.
|
||||
|
||||
- [ ] **Step 2: Verify compilation**
|
||||
|
||||
Run: `cd web && pnpm lint`
|
||||
Expected: PASS.
|
||||
|
||||
- [ ] **Step 3: Commit**
|
||||
|
||||
```bash
|
||||
git add web/src/components/MemoEditor/hooks/useMemoInit.ts
|
||||
git commit -m "feat(memo-editor): seed timestamps from defaultCreateTime on init"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 5: Wire `defaultCreateTime` through `MemoEditor` and render popover
|
||||
|
||||
**Files:**
|
||||
- Modify: `web/src/components/MemoEditor/index.tsx`
|
||||
|
||||
Three changes:
|
||||
1. Destructure `defaultCreateTime` from props.
|
||||
2. Pass it into `useMemoInit`.
|
||||
3. Add a `useEffect` that dispatches `setTimestamps` whenever `defaultCreateTime` changes after mount (live re-sync per design Q3-A). Skip when `memo` is set.
|
||||
4. Update the popover render condition so it shows in create mode too when timestamps are seeded.
|
||||
|
||||
- [ ] **Step 1: Destructure the prop and pass it to `useMemoInit`**
|
||||
|
||||
In `web/src/components/MemoEditor/index.tsx`, update the `MemoEditorImpl` destructuring (around line 42):
|
||||
|
||||
```tsx
|
||||
const MemoEditorImpl: React.FC<MemoEditorProps> = ({
|
||||
className,
|
||||
cacheKey,
|
||||
memo,
|
||||
parentMemoName,
|
||||
autoFocus,
|
||||
placeholder,
|
||||
defaultCreateTime,
|
||||
onConfirm,
|
||||
onCancel,
|
||||
}) => {
|
||||
```
|
||||
|
||||
And update the `useMemoInit` call (around line 71):
|
||||
|
||||
```tsx
|
||||
const { isInitialized } = useMemoInit({
|
||||
editorRef,
|
||||
memo,
|
||||
cacheKey,
|
||||
username: currentUser?.name ?? "",
|
||||
autoFocus,
|
||||
defaultVisibility,
|
||||
defaultCreateTime,
|
||||
});
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Add the live re-sync effect**
|
||||
|
||||
In the same file, add a new `useEffect` after `useMemoInit` (and after `useAutoSave`) that re-syncs timestamps when `defaultCreateTime` changes in create mode. Place it just before the existing `useEffect` that fetches AI settings (around line 80):
|
||||
|
||||
```tsx
|
||||
// Live-sync the draft's createTime/updateTime to the calendar-derived prop.
|
||||
// Only applies in create mode; edit mode owns its own timestamps. Runs after
|
||||
// initial mount (the seed value is set in useMemoInit), and again whenever
|
||||
// the prop changes — e.g., when the user picks a different calendar date
|
||||
// while the editor is open.
|
||||
useEffect(() => {
|
||||
if (memo) return;
|
||||
if (!isInitialized) return;
|
||||
dispatch(
|
||||
actions.setTimestamps({
|
||||
createTime: defaultCreateTime,
|
||||
updateTime: defaultCreateTime,
|
||||
}),
|
||||
);
|
||||
}, [defaultCreateTime, memo, isInitialized, actions, dispatch]);
|
||||
```
|
||||
|
||||
Notes:
|
||||
- We pass `undefined` through when the prop becomes undefined (filter cleared) — this resets timestamps to undefined so the editor falls back to "server-stamped now" on save, exactly the pre-feature behavior.
|
||||
- The `isInitialized` guard avoids racing with `useMemoInit`'s one-shot seed.
|
||||
|
||||
- [ ] **Step 3: Update the popover render condition**
|
||||
|
||||
In the same file, find the existing block (around line 294):
|
||||
|
||||
```tsx
|
||||
{memoName && (
|
||||
<div className="w-full -mb-1">
|
||||
<TimestampPopover />
|
||||
</div>
|
||||
)}
|
||||
```
|
||||
|
||||
Replace with:
|
||||
|
||||
```tsx
|
||||
{(memoName || (!memo && state.timestamps.createTime)) && (
|
||||
<div className="w-full -mb-1">
|
||||
<TimestampPopover />
|
||||
</div>
|
||||
)}
|
||||
```
|
||||
|
||||
Now the popover renders in edit mode (unchanged) AND in create mode whenever a default timestamp has been seeded.
|
||||
|
||||
- [ ] **Step 4: Verify compilation**
|
||||
|
||||
Run: `cd web && pnpm lint`
|
||||
Expected: PASS.
|
||||
|
||||
- [ ] **Step 5: Run all tests**
|
||||
|
||||
Run: `cd web && pnpm test`
|
||||
Expected: PASS (no editor-specific tests added; existing tests continue to pass).
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add web/src/components/MemoEditor/index.tsx
|
||||
git commit -m "feat(memo-editor): live-sync timestamps and reveal popover from defaultCreateTime"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 6: Wire calendar selection through `PagedMemoList`
|
||||
|
||||
**Files:**
|
||||
- Modify: `web/src/components/PagedMemoList/PagedMemoList.tsx`
|
||||
|
||||
The home memo editor is rendered at line ~155 of `PagedMemoList.tsx`. Read the current `MemoFilterContext` filters, derive the `defaultCreateTime`, and pass it to `<MemoEditor>`. Wrap in `useMemo` so the reference stays stable when filters don't change (avoids re-firing the live-sync effect).
|
||||
|
||||
- [ ] **Step 1: Add the imports and derivation**
|
||||
|
||||
Modify `web/src/components/PagedMemoList/PagedMemoList.tsx`. Add to the existing imports near the top:
|
||||
|
||||
```tsx
|
||||
import { useMemo } from "react";
|
||||
import { useMemoFilterContext } from "@/contexts/MemoFilterContext";
|
||||
import { deriveDefaultCreateTimeFromFilters } from "@/components/MemoEditor/utils/deriveDefaultCreateTime";
|
||||
```
|
||||
|
||||
If `useMemo` is already imported from `react` in this file, merge into the existing import rather than duplicating.
|
||||
|
||||
Inside the component body (above the `children` JSX, near the top of the function), add:
|
||||
|
||||
```tsx
|
||||
const { filters } = useMemoFilterContext();
|
||||
const defaultCreateTime = useMemo(
|
||||
() => deriveDefaultCreateTimeFromFilters(filters),
|
||||
[filters],
|
||||
);
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Pass the prop to `<MemoEditor>`**
|
||||
|
||||
Replace the existing line ~155:
|
||||
|
||||
```tsx
|
||||
{showMemoEditor ? <MemoEditor className="mb-2" cacheKey="home-memo-editor" placeholder={t("editor.any-thoughts")} /> : null}
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```tsx
|
||||
{showMemoEditor ? (
|
||||
<MemoEditor
|
||||
className="mb-2"
|
||||
cacheKey="home-memo-editor"
|
||||
placeholder={t("editor.any-thoughts")}
|
||||
defaultCreateTime={defaultCreateTime}
|
||||
/>
|
||||
) : null}
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Verify compilation**
|
||||
|
||||
Run: `cd web && pnpm lint`
|
||||
Expected: PASS.
|
||||
|
||||
- [ ] **Step 4: Run all tests**
|
||||
|
||||
Run: `cd web && pnpm test`
|
||||
Expected: PASS.
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add web/src/components/PagedMemoList/PagedMemoList.tsx
|
||||
git commit -m "feat(home): pass calendar-selected date as default createTime to memo editor"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 7: Manual smoke test
|
||||
|
||||
No code changes. Per `CLAUDE.md`: "For UI or frontend changes, start the dev server and use the feature in a browser before reporting the task as complete." Walk through the user-visible flows.
|
||||
|
||||
- [ ] **Step 1: Start the backend and frontend**
|
||||
|
||||
In one terminal:
|
||||
```bash
|
||||
go run ./cmd/memos --port 8081
|
||||
```
|
||||
|
||||
In another:
|
||||
```bash
|
||||
cd web && pnpm dev
|
||||
```
|
||||
|
||||
Open `http://localhost:3001`.
|
||||
|
||||
- [ ] **Step 2: Smoke — populated past date**
|
||||
|
||||
1. Sign in. Confirm the activity calendar is visible (statistics view).
|
||||
2. Click a *past* date that already has memos.
|
||||
3. Confirm the URL gains `?filter=displayTime:YYYY-MM-DD`.
|
||||
4. Confirm the home memo editor shows the timestamp popover above the textarea, populated with the selected date.
|
||||
5. Type a memo and click Save.
|
||||
6. Clear the date filter (chip X). Reapply by clicking the same date.
|
||||
7. Confirm the new memo appears under that date.
|
||||
|
||||
- [ ] **Step 3: Smoke — empty past date**
|
||||
|
||||
1. Pick a past date with **zero memos** in the calendar (lightest cell).
|
||||
2. Confirm it is now clickable, the URL filter applies, and the empty-state shows.
|
||||
3. Type and save a memo.
|
||||
4. Confirm the memo appears for that date and the calendar cell tints up.
|
||||
|
||||
- [ ] **Step 4: Smoke — future date**
|
||||
|
||||
1. Click a future date in the current month.
|
||||
2. Confirm the popover shows that date.
|
||||
3. Save a memo. Confirm it appears under that date.
|
||||
|
||||
- [ ] **Step 5: Smoke — clear filter mid-draft**
|
||||
|
||||
1. Pick May 1 (or any non-today date). Type some content, do **not** save.
|
||||
2. Click the filter chip X to clear the date filter.
|
||||
3. Confirm the popover disappears and the draft content is preserved (autoSave behavior).
|
||||
4. Save and confirm the memo gets a server-stamped "now" timestamp (i.e., appears under today).
|
||||
|
||||
- [ ] **Step 6: Smoke — change filter mid-draft**
|
||||
|
||||
1. Pick May 1. Type content.
|
||||
2. Without saving, click May 3.
|
||||
3. Confirm the popover updates to May 3.
|
||||
4. Save. Confirm the memo appears under May 3.
|
||||
|
||||
- [ ] **Step 7: Smoke — comment editor unaffected**
|
||||
|
||||
1. Open any memo's detail view (or open the comments thread).
|
||||
2. Confirm the reply editor does **not** show a timestamp popover.
|
||||
3. Confirm the date filter has no visible effect on the reply editor.
|
||||
|
||||
- [ ] **Step 8: Smoke — edit mode unaffected**
|
||||
|
||||
1. Edit an existing memo (pencil icon).
|
||||
2. Confirm the existing timestamp popover still works exactly as before, regardless of any active calendar filter.
|
||||
|
||||
- [ ] **Step 9: Smoke — empty-date click on Explore page**
|
||||
|
||||
1. Navigate to the Explore page (which also renders the calendar).
|
||||
2. Click an empty date.
|
||||
3. Confirm the URL filter applies and the empty-state shows. (No editor on Explore — that's correct.)
|
||||
|
||||
- [ ] **Step 10: Record results**
|
||||
|
||||
Note any unexpected behavior (especially: selection-ring contrast on the lowest-intensity background, mentioned as a flag in the spec). If the ring is too subtle, file a follow-up — *not* part of this plan.
|
||||
|
||||
---
|
||||
|
||||
## Self-review
|
||||
|
||||
**Spec coverage check:**
|
||||
|
||||
| Spec requirement | Task |
|
||||
|---|---|
|
||||
| Empty calendar dates clickable | Task 2 |
|
||||
| Editor shows TimestampPopover in create mode when filter active | Task 5 (popover condition) |
|
||||
| `createTime` = selected date + current local hh:mm:ss | Task 1 (helper) + Task 4 (seed) |
|
||||
| `updateTime` mirrored to same value | Task 4 (seed) + Task 5 (live sync) |
|
||||
| Live-derived: filter change re-syncs timestamps | Task 5 (live-sync `useEffect`) |
|
||||
| Filter cleared → undefined → server-stamped "now" | Task 5 (passes `undefined` through) + Task 6 (helper returns undefined) |
|
||||
| Future dates allowed (no clamp) | Task 1 (no clamp in helper); confirmed in Task 7 step 4 |
|
||||
| Comment editor unaffected | Task 6 wires only `PagedMemoList`; confirmed in Task 7 step 7 |
|
||||
| Edit mode unaffected | Task 4 + 5 explicitly guard on `memo`; confirmed in Task 7 step 8 |
|
||||
| Empty-date click on Explore/Archived/Profile | Task 2 (calendar-side change); confirmed in Task 7 step 9 |
|
||||
| DST/timezone uses local time | Task 1 (`new Date(y, m-1, d, h, mi, s)`) |
|
||||
| Helper unit tests | Task 1 |
|
||||
| `CalendarCell` empty-cell test | Task 2 |
|
||||
| Manual smoke | Task 7 |
|
||||
|
||||
All spec requirements have a task. No gaps.
|
||||
|
||||
**Placeholder scan:** No "TBD"/"TODO" steps. Every code step shows the actual code.
|
||||
|
||||
**Type/name consistency check:**
|
||||
- `MemoFilter` and `useMemoFilterContext` exist at `@/contexts/MemoFilterContext` (verified during exploration).
|
||||
- `editorActions.setTimestamps` exists in `state/actions.ts:75` and accepts `Partial<EditorState["timestamps"]>` (verified). Calls in Tasks 4 and 5 match.
|
||||
- `state.timestamps.createTime` is `Date | undefined` (verified `state/types.ts:27-29`). The popover render condition uses it as a truthy guard — `Date` instances are truthy, `undefined` is falsy.
|
||||
- `useMemoFilterContext` (alias used in PagedMemoList) is exported from `MemoFilterContext.tsx:151` (verified).
|
||||
- `deriveDefaultCreateTimeFromFilters` signature is identical between Task 1 (definition) and Task 6 (consumer).
|
||||
- `defaultCreateTime: Date | undefined` flows consistently through `MemoEditorProps` (Task 3) → `MemoEditorImpl` destructuring (Task 5) → `useMemoInit` options (Task 4) → reducer payloads.
|
||||
File diff suppressed because it is too large
Load Diff
File diff suppressed because it is too large
Load Diff
@@ -0,0 +1,774 @@
|
||||
# First Screen Lazy Heavy Dependencies 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:** Keep Mermaid, Leaflet, React Leaflet, marker cluster, and feature CSS out of the auth/signup initial screen while preserving diagram, math, and map behavior when those features are used.
|
||||
|
||||
**Architecture:** Move runtime Leaflet objects behind plain coordinate data at parent boundaries, then lazy-load map implementations from small wrapper components. Replace Mermaid's static import with effect-time dynamic import, and move KaTeX/Leaflet CSS from the app entry into feature modules.
|
||||
|
||||
**Tech Stack:** React 19, TypeScript 6, Vite 8/Rolldown, React Router 7, React Query 5, Mermaid, KaTeX, Leaflet, React Leaflet, pnpm.
|
||||
|
||||
---
|
||||
|
||||
## File Structure
|
||||
|
||||
- Create `web/src/components/map/types.ts`
|
||||
- Defines a lightweight `MapPoint` interface used by parents that must not import Leaflet.
|
||||
- Create `web/src/components/map/LazyLocationPicker.tsx`
|
||||
- Provides a `React.lazy` boundary and map-sized fallback for `LocationPicker`.
|
||||
- Modify `web/src/main.tsx`
|
||||
- Removes global Leaflet and KaTeX CSS imports.
|
||||
- Modify `web/src/router/index.tsx`
|
||||
- Lazy-loads the Home route so memo/editor/markdown modules are not part of the auth/signup entry graph.
|
||||
- Modify `web/vite.config.mts`
|
||||
- Keeps optional heavy dependency split groups from becoming entry preloads.
|
||||
- Modify `web/src/components/map/LocationPicker.tsx`
|
||||
- Imports Leaflet CSS inside the lazy implementation path.
|
||||
- Accepts and emits plain `MapPoint` values at the public component boundary.
|
||||
- Keeps `LatLng` runtime use internal to the map implementation.
|
||||
- Modify `web/src/components/map/index.ts`
|
||||
- Stops exporting `LocationPicker` and map utility helpers from the barrel so non-map imports do not pull Leaflet into parent chunks.
|
||||
- Keeps exporting `useReverseGeocoding` because it has no Leaflet dependency.
|
||||
- Modify `web/src/components/MemoEditor/types/insert-menu.ts`
|
||||
- Replaces `LatLng` in editor state with `MapPoint`.
|
||||
- Modify `web/src/components/MemoEditor/hooks/useLocation.ts`
|
||||
- Removes runtime Leaflet import and stores plain coordinates.
|
||||
- Modify `web/src/components/MemoEditor/Toolbar/InsertMenu.tsx`
|
||||
- Removes runtime Leaflet import.
|
||||
- Imports `useReverseGeocoding` directly from its file.
|
||||
- Constructs plain coordinates from geolocation.
|
||||
- Modify `web/src/components/MemoMetadata/Location/LocationDialog.tsx`
|
||||
- Uses `LazyLocationPicker` and `MapPoint`.
|
||||
- Only mounts the lazy picker while the dialog is open.
|
||||
- Modify `web/src/components/MemoMetadata/Location/LocationDisplayView.tsx`
|
||||
- Removes runtime Leaflet import and uses `LazyLocationPicker` inside the popover.
|
||||
- Modify `web/src/pages/UserProfile.tsx`
|
||||
- Lazy-loads `UserMemoMap` only when the map tab is active.
|
||||
- Modify `web/src/components/MemoContent/MemoMarkdownRenderer.tsx`
|
||||
- Imports KaTeX CSS from the markdown-rendering path.
|
||||
- Modify `web/src/components/MemoContent/MermaidBlock.tsx`
|
||||
- Dynamically imports Mermaid only when rendering a Mermaid block.
|
||||
- Modify `web/src/components/UserMemoMap/UserMemoMap.tsx`
|
||||
- Keeps marker cluster CSS in the lazy map implementation path.
|
||||
- Verify with `pnpm lint`, `pnpm build`, and a production preview/browser network check.
|
||||
|
||||
## Task 1: Remove Leaflet From Editor Location State
|
||||
|
||||
**Files:**
|
||||
- Create: `web/src/components/map/types.ts`
|
||||
- Modify: `web/src/components/MemoEditor/types/insert-menu.ts`
|
||||
- Modify: `web/src/components/MemoEditor/hooks/useLocation.ts`
|
||||
- Modify: `web/src/components/MemoEditor/Toolbar/InsertMenu.tsx`
|
||||
|
||||
- [x] **Step 1: Add a Leaflet-free map coordinate type**
|
||||
|
||||
Create `web/src/components/map/types.ts`:
|
||||
|
||||
```ts
|
||||
export interface MapPoint {
|
||||
lat: number;
|
||||
lng: number;
|
||||
}
|
||||
```
|
||||
|
||||
- [x] **Step 2: Replace editor location state type**
|
||||
|
||||
In `web/src/components/MemoEditor/types/insert-menu.ts`, replace the entire file with:
|
||||
|
||||
```ts
|
||||
import type { MapPoint } from "@/components/map/types";
|
||||
|
||||
export interface LocationState {
|
||||
placeholder: string;
|
||||
position?: MapPoint;
|
||||
latInput: string;
|
||||
lngInput: string;
|
||||
}
|
||||
```
|
||||
|
||||
- [x] **Step 3: Remove Leaflet runtime usage from `useLocation`**
|
||||
|
||||
In `web/src/components/MemoEditor/hooks/useLocation.ts`, remove `import { LatLng } from "leaflet";`, add a type import, and use plain coordinate objects:
|
||||
|
||||
```ts
|
||||
import { create } from "@bufbuild/protobuf";
|
||||
import { useCallback, useMemo, useRef, useState } from "react";
|
||||
import type { MapPoint } from "@/components/map/types";
|
||||
import { Location, LocationSchema } from "@/types/proto/api/v1/memo_service_pb";
|
||||
import { LocationState } from "../types/insert-menu";
|
||||
|
||||
export const useLocation = (initialLocation?: Location) => {
|
||||
const [locationInitialized, setLocationInitialized] = useState(false);
|
||||
const locationInitializedRef = useRef(locationInitialized);
|
||||
locationInitializedRef.current = locationInitialized;
|
||||
|
||||
const [state, setState] = useState<LocationState>({
|
||||
placeholder: initialLocation?.placeholder || "",
|
||||
position: initialLocation ? { lat: initialLocation.latitude, lng: initialLocation.longitude } : undefined,
|
||||
latInput: initialLocation ? String(initialLocation.latitude) : "",
|
||||
lngInput: initialLocation ? String(initialLocation.longitude) : "",
|
||||
});
|
||||
|
||||
const stateRef = useRef(state);
|
||||
stateRef.current = state;
|
||||
|
||||
const updatePosition = useCallback((position?: MapPoint) => {
|
||||
setState((prev) => ({
|
||||
...prev,
|
||||
position,
|
||||
latInput: position ? String(position.lat) : "",
|
||||
lngInput: position ? String(position.lng) : "",
|
||||
}));
|
||||
}, []);
|
||||
|
||||
const handlePositionChange = useCallback(
|
||||
(position: MapPoint) => {
|
||||
if (!locationInitializedRef.current) setLocationInitialized(true);
|
||||
updatePosition(position);
|
||||
},
|
||||
[updatePosition],
|
||||
);
|
||||
|
||||
const updateCoordinate = useCallback((type: "lat" | "lng", value: string) => {
|
||||
const num = parseFloat(value);
|
||||
const isValid = type === "lat" ? !isNaN(num) && num >= -90 && num <= 90 : !isNaN(num) && num >= -180 && num <= 180;
|
||||
setState((prev) => {
|
||||
const next = { ...prev, [type === "lat" ? "latInput" : "lngInput"]: value };
|
||||
if (isValid && prev.position) {
|
||||
const newPos = type === "lat" ? { lat: num, lng: prev.position.lng } : { lat: prev.position.lat, lng: num };
|
||||
return { ...next, position: newPos, latInput: String(newPos.lat), lngInput: String(newPos.lng) };
|
||||
}
|
||||
return next;
|
||||
});
|
||||
}, []);
|
||||
|
||||
const setPlaceholder = useCallback((placeholder: string) => {
|
||||
setState((prev) => ({ ...prev, placeholder }));
|
||||
}, []);
|
||||
|
||||
const reset = useCallback(() => {
|
||||
setState({
|
||||
placeholder: "",
|
||||
position: undefined,
|
||||
latInput: "",
|
||||
lngInput: "",
|
||||
});
|
||||
setLocationInitialized(false);
|
||||
}, []);
|
||||
|
||||
const getLocation = useCallback((): Location | undefined => {
|
||||
const { position, placeholder } = stateRef.current;
|
||||
if (!position || !placeholder.trim()) {
|
||||
return undefined;
|
||||
}
|
||||
return create(LocationSchema, {
|
||||
latitude: position.lat,
|
||||
longitude: position.lng,
|
||||
placeholder,
|
||||
});
|
||||
}, []);
|
||||
|
||||
return useMemo(
|
||||
() => ({ state, locationInitialized, handlePositionChange, updateCoordinate, setPlaceholder, reset, getLocation }),
|
||||
[state, locationInitialized, handlePositionChange, updateCoordinate, setPlaceholder, reset, getLocation],
|
||||
);
|
||||
};
|
||||
```
|
||||
|
||||
- [x] **Step 4: Remove Leaflet import from `InsertMenu`**
|
||||
|
||||
In `web/src/components/MemoEditor/Toolbar/InsertMenu.tsx`:
|
||||
|
||||
Remove:
|
||||
|
||||
```ts
|
||||
import { LatLng } from "leaflet";
|
||||
```
|
||||
|
||||
Replace:
|
||||
|
||||
```ts
|
||||
import { useReverseGeocoding } from "@/components/map";
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```ts
|
||||
import { useReverseGeocoding } from "@/components/map/useReverseGeocoding";
|
||||
```
|
||||
|
||||
Replace the geolocation success handler body:
|
||||
|
||||
```ts
|
||||
handleLocationPositionChange(new LatLng(position.coords.latitude, position.coords.longitude));
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```ts
|
||||
handleLocationPositionChange({ lat: position.coords.latitude, lng: position.coords.longitude });
|
||||
```
|
||||
|
||||
- [x] **Step 5: Run focused type check**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cd web && pnpm lint
|
||||
```
|
||||
|
||||
Expected: TypeScript may still fail because map component props have not been converted yet. If the only failures are `LatLng`/`MapPoint` mismatches in map/location components, continue to Task 2. If unrelated failures appear, stop and inspect before editing further.
|
||||
|
||||
## Task 2: Lazy-Load Location Picker And Keep Leaflet Inside Map Modules
|
||||
|
||||
**Files:**
|
||||
- Create: `web/src/components/map/LazyLocationPicker.tsx`
|
||||
- Modify: `web/src/components/map/LocationPicker.tsx`
|
||||
- Modify: `web/src/components/map/index.ts`
|
||||
- Modify: `web/src/components/MemoMetadata/Location/LocationDialog.tsx`
|
||||
- Modify: `web/src/components/MemoMetadata/Location/LocationDisplayView.tsx`
|
||||
|
||||
- [x] **Step 1: Create lazy location picker wrapper**
|
||||
|
||||
Create `web/src/components/map/LazyLocationPicker.tsx`:
|
||||
|
||||
```tsx
|
||||
import { lazy, Suspense } from "react";
|
||||
import { cn } from "@/lib/utils";
|
||||
import type { MapPoint } from "./types";
|
||||
|
||||
interface LazyLocationPickerProps {
|
||||
readonly?: boolean;
|
||||
latlng?: MapPoint;
|
||||
onChange?: (position: MapPoint) => void;
|
||||
className?: string;
|
||||
}
|
||||
|
||||
const LocationPicker = lazy(() => import("./LocationPicker"));
|
||||
|
||||
export const LazyLocationPicker = ({ className, ...props }: LazyLocationPickerProps) => {
|
||||
return (
|
||||
<Suspense
|
||||
fallback={
|
||||
<div
|
||||
className={cn(
|
||||
"memo-location-map relative isolate h-72 w-full overflow-hidden rounded-xl border border-border bg-muted/30 shadow-sm",
|
||||
className,
|
||||
)}
|
||||
/>
|
||||
}
|
||||
>
|
||||
<LocationPicker className={className} {...props} />
|
||||
</Suspense>
|
||||
);
|
||||
};
|
||||
```
|
||||
|
||||
- [x] **Step 2: Convert `LocationPicker` public props to `MapPoint`**
|
||||
|
||||
In `web/src/components/map/LocationPicker.tsx`:
|
||||
|
||||
Add CSS and type imports near the top:
|
||||
|
||||
```ts
|
||||
import "leaflet/dist/leaflet.css";
|
||||
import type { MapPoint } from "./types";
|
||||
```
|
||||
|
||||
Keep Leaflet runtime import:
|
||||
|
||||
```ts
|
||||
import L, { LatLng } from "leaflet";
|
||||
```
|
||||
|
||||
Add helper functions after imports:
|
||||
|
||||
```ts
|
||||
const toLatLng = (point: MapPoint): LatLng => new LatLng(point.lat, point.lng);
|
||||
const fromLatLng = (latlng: LatLng): MapPoint => ({ lat: latlng.lat, lng: latlng.lng });
|
||||
```
|
||||
|
||||
Update public-facing props:
|
||||
|
||||
```ts
|
||||
interface LocationMarkerProps {
|
||||
position: LatLng | undefined;
|
||||
onChange: (position: MapPoint) => void;
|
||||
readonly?: boolean;
|
||||
}
|
||||
```
|
||||
|
||||
In `LocationMarker`, replace:
|
||||
|
||||
```ts
|
||||
onChange(e.latlng);
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```ts
|
||||
onChange(fromLatLng(e.latlng));
|
||||
```
|
||||
|
||||
Update `MapControlsProps`:
|
||||
|
||||
```ts
|
||||
interface MapControlsProps {
|
||||
position: MapPoint | undefined;
|
||||
}
|
||||
```
|
||||
|
||||
Update `LocationPickerProps`:
|
||||
|
||||
```ts
|
||||
interface LocationPickerProps {
|
||||
readonly?: boolean;
|
||||
latlng?: MapPoint;
|
||||
onChange?: (position: MapPoint) => void;
|
||||
className?: string;
|
||||
}
|
||||
```
|
||||
|
||||
Replace:
|
||||
|
||||
```ts
|
||||
const DEFAULT_CENTER_LAT_LNG = new LatLng(48.8584, 2.2945);
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```ts
|
||||
const DEFAULT_CENTER: MapPoint = { lat: 48.8584, lng: 2.2945 };
|
||||
```
|
||||
|
||||
Inside `LocationPicker`, replace:
|
||||
|
||||
```ts
|
||||
const position = latlng || DEFAULT_CENTER_LAT_LNG;
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```ts
|
||||
const position = latlng || DEFAULT_CENTER;
|
||||
const mapCenter = toLatLng(position);
|
||||
const markerPosition = latlng ? toLatLng(latlng) : mapCenter;
|
||||
```
|
||||
|
||||
Replace the `MapContainer` props and marker call:
|
||||
|
||||
```tsx
|
||||
<MapContainer
|
||||
className="h-full w-full !bg-muted"
|
||||
center={mapCenter}
|
||||
zoom={13}
|
||||
scrollWheelZoom={false}
|
||||
zoomControl={false}
|
||||
attributionControl={false}
|
||||
>
|
||||
<ThemedTileLayer />
|
||||
<LocationMarker position={markerPosition} readonly={readOnly} onChange={onChange} />
|
||||
<MapControls position={latlng} />
|
||||
<MapCleanup />
|
||||
</MapContainer>
|
||||
```
|
||||
|
||||
- [x] **Step 3: Keep the map barrel Leaflet-free**
|
||||
|
||||
Replace `web/src/components/map/index.ts` with:
|
||||
|
||||
```ts
|
||||
export { useReverseGeocoding } from "./useReverseGeocoding";
|
||||
```
|
||||
|
||||
Do not export `LocationPicker`, `LazyLocationPicker`, `map-utils`, or Leaflet helpers from this barrel. Import map UI directly from `@/components/map/LazyLocationPicker` or the implementation file.
|
||||
|
||||
- [x] **Step 4: Use lazy picker in `LocationDialog`**
|
||||
|
||||
In `web/src/components/MemoMetadata/Location/LocationDialog.tsx`:
|
||||
|
||||
Remove:
|
||||
|
||||
```ts
|
||||
import type { LatLng } from "leaflet";
|
||||
import { LocationPicker } from "@/components/map";
|
||||
```
|
||||
|
||||
Add:
|
||||
|
||||
```ts
|
||||
import { LazyLocationPicker } from "@/components/map/LazyLocationPicker";
|
||||
import type { MapPoint } from "@/components/map/types";
|
||||
```
|
||||
|
||||
Change the prop type:
|
||||
|
||||
```ts
|
||||
onPositionChange: (position: MapPoint) => void;
|
||||
```
|
||||
|
||||
Replace:
|
||||
|
||||
```tsx
|
||||
<LocationPicker className="h-full" latlng={position} onChange={onPositionChange} />
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```tsx
|
||||
{open && <LazyLocationPicker className="h-full" latlng={position} onChange={onPositionChange} />}
|
||||
```
|
||||
|
||||
- [x] **Step 5: Use lazy picker in `LocationDisplayView`**
|
||||
|
||||
In `web/src/components/MemoMetadata/Location/LocationDisplayView.tsx`:
|
||||
|
||||
Remove:
|
||||
|
||||
```ts
|
||||
import { LatLng } from "leaflet";
|
||||
import { LocationPicker } from "@/components/map";
|
||||
```
|
||||
|
||||
Add:
|
||||
|
||||
```ts
|
||||
import { LazyLocationPicker } from "@/components/map/LazyLocationPicker";
|
||||
```
|
||||
|
||||
Replace:
|
||||
|
||||
```tsx
|
||||
<LocationPicker latlng={new LatLng(location.latitude, location.longitude)} readonly={true} />
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```tsx
|
||||
{popoverOpen && <LazyLocationPicker latlng={{ lat: location.latitude, lng: location.longitude }} readonly={true} />}
|
||||
```
|
||||
|
||||
- [x] **Step 6: Run lint**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cd web && pnpm lint
|
||||
```
|
||||
|
||||
Expected: PASS for the files changed so far, or only failures unrelated to this work. Fix any `MapPoint`/`LatLng` type errors before continuing.
|
||||
|
||||
## Task 3: Lazy-Load Profile Map
|
||||
|
||||
**Files:**
|
||||
- Modify: `web/src/pages/UserProfile.tsx`
|
||||
- Modify: `web/src/components/UserMemoMap/UserMemoMap.tsx`
|
||||
|
||||
- [x] **Step 1: Move `UserMemoMap` behind `React.lazy`**
|
||||
|
||||
In `web/src/pages/UserProfile.tsx`, replace the React import section:
|
||||
|
||||
```ts
|
||||
import copy from "copy-to-clipboard";
|
||||
import { ExternalLinkIcon, LayoutListIcon, type LucideIcon, MapIcon } from "lucide-react";
|
||||
import { toast } from "react-hot-toast";
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```ts
|
||||
import copy from "copy-to-clipboard";
|
||||
import { lazy, Suspense } from "react";
|
||||
import { ExternalLinkIcon, LayoutListIcon, type LucideIcon, MapIcon } from "lucide-react";
|
||||
import { toast } from "react-hot-toast";
|
||||
```
|
||||
|
||||
Remove:
|
||||
|
||||
```ts
|
||||
import UserMemoMap from "@/components/UserMemoMap";
|
||||
```
|
||||
|
||||
Add after the `type TabView = "memos" | "map";` line:
|
||||
|
||||
```ts
|
||||
const UserMemoMap = lazy(() => import("@/components/UserMemoMap"));
|
||||
```
|
||||
|
||||
Replace:
|
||||
|
||||
```tsx
|
||||
<div className="">
|
||||
<UserMemoMap creator={user.name} className="h-[60dvh] sm:h-[500px] rounded-xl" />
|
||||
</div>
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```tsx
|
||||
<div className="">
|
||||
<Suspense fallback={<div className="h-[60dvh] sm:h-[500px] rounded-xl border border-border bg-muted/30" />}>
|
||||
<UserMemoMap creator={user.name} className="h-[60dvh] sm:h-[500px] rounded-xl" />
|
||||
</Suspense>
|
||||
</div>
|
||||
```
|
||||
|
||||
- [x] **Step 2: Keep Leaflet CSS in the lazy profile map implementation**
|
||||
|
||||
In `web/src/components/UserMemoMap/UserMemoMap.tsx`, add the Leaflet CSS import above marker cluster CSS:
|
||||
|
||||
```ts
|
||||
import "leaflet/dist/leaflet.css";
|
||||
import "leaflet.markercluster/dist/MarkerCluster.css";
|
||||
```
|
||||
|
||||
The file should continue to import Leaflet, React Leaflet, marker clustering, and `map-utils` directly because this component is now loaded only from a lazy boundary.
|
||||
|
||||
- [x] **Step 3: Run lint**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cd web && pnpm lint
|
||||
```
|
||||
|
||||
Expected: PASS, or only unrelated pre-existing failures.
|
||||
|
||||
## Task 4: Move KaTeX CSS And Dynamically Import Mermaid
|
||||
|
||||
**Files:**
|
||||
- Modify: `web/src/main.tsx`
|
||||
- Modify: `web/src/components/MemoContent/MemoMarkdownRenderer.tsx`
|
||||
- Modify: `web/src/components/MemoContent/MermaidBlock.tsx`
|
||||
|
||||
- [x] **Step 1: Remove feature CSS from app entry**
|
||||
|
||||
In `web/src/main.tsx`, remove:
|
||||
|
||||
```ts
|
||||
import "leaflet/dist/leaflet.css";
|
||||
import "katex/dist/katex.min.css";
|
||||
```
|
||||
|
||||
- [x] **Step 2: Load KaTeX CSS from markdown renderer**
|
||||
|
||||
In `web/src/components/MemoContent/MemoMarkdownRenderer.tsx`, add this import with the existing dependency imports:
|
||||
|
||||
```ts
|
||||
import "katex/dist/katex.min.css";
|
||||
```
|
||||
|
||||
- [x] **Step 3: Remove static Mermaid import**
|
||||
|
||||
In `web/src/components/MemoContent/MermaidBlock.tsx`, remove:
|
||||
|
||||
```ts
|
||||
import mermaid from "mermaid";
|
||||
```
|
||||
|
||||
- [x] **Step 4: Replace Mermaid initialization/render effects with dynamic import**
|
||||
|
||||
In `web/src/components/MemoContent/MermaidBlock.tsx`, remove the existing Mermaid initialization effect:
|
||||
|
||||
```ts
|
||||
useEffect(() => {
|
||||
mermaid.initialize({
|
||||
startOnLoad: false,
|
||||
theme: toMermaidTheme(currentTheme),
|
||||
securityLevel: "strict",
|
||||
fontFamily: "inherit",
|
||||
suppressErrorRendering: true,
|
||||
});
|
||||
}, [currentTheme]);
|
||||
```
|
||||
|
||||
Replace the existing render effect with:
|
||||
|
||||
```ts
|
||||
useEffect(() => {
|
||||
if (!codeContent) return;
|
||||
|
||||
let cancelled = false;
|
||||
|
||||
const renderDiagram = async () => {
|
||||
try {
|
||||
const { default: mermaid } = await import("mermaid");
|
||||
if (cancelled) return;
|
||||
|
||||
mermaid.initialize({
|
||||
startOnLoad: false,
|
||||
theme: toMermaidTheme(currentTheme),
|
||||
securityLevel: "strict",
|
||||
fontFamily: "inherit",
|
||||
suppressErrorRendering: true,
|
||||
});
|
||||
|
||||
const id = `mermaid-${Math.random().toString(36).substring(7)}`;
|
||||
const { svg: renderedSvg } = await mermaid.render(id, codeContent);
|
||||
if (cancelled) return;
|
||||
|
||||
setSvg(renderedSvg);
|
||||
setError("");
|
||||
} catch (err) {
|
||||
if (cancelled) return;
|
||||
console.error("Failed to render mermaid diagram:", err);
|
||||
setSvg("");
|
||||
setError(formatErrorMessage(err));
|
||||
}
|
||||
};
|
||||
|
||||
renderDiagram();
|
||||
|
||||
return () => {
|
||||
cancelled = true;
|
||||
};
|
||||
}, [codeContent, currentTheme]);
|
||||
```
|
||||
|
||||
- [x] **Step 5: Run lint**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cd web && pnpm lint
|
||||
```
|
||||
|
||||
Expected: PASS, or only unrelated pre-existing failures.
|
||||
|
||||
## Task 4.5: Remove Remaining Entry Preloads Found During Verification
|
||||
|
||||
**Files:**
|
||||
- Modify: `web/src/router/index.tsx`
|
||||
- Modify: `web/vite.config.mts`
|
||||
|
||||
- [x] **Step 1: Lazy-load the Home route**
|
||||
|
||||
`web/src/router/index.tsx` now uses `lazyWithReload(() => import("@/pages/Home"))` instead of a static Home import. This prevents Home, `PagedMemoList`, `MemoEditor`, `MemoContent`, KaTeX, and Mermaid code from entering the auth/signup app entry graph.
|
||||
|
||||
- [x] **Step 2: Tighten optional vendor split groups**
|
||||
|
||||
`web/vite.config.mts` no longer defines a manual `mermaid-vendor` group, because Rolldown emitted the preload helper from that group and forced an entry preload. The Leaflet group now matches only the `leaflet` package, not `react-leaflet`, so React does not get bundled into a Leaflet-named entry preload.
|
||||
|
||||
- [x] **Step 3: Re-run lint and build**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cd web && pnpm lint && pnpm build
|
||||
```
|
||||
|
||||
Expected: PASS.
|
||||
|
||||
## Task 5: Build And Verify Initial Network Behavior
|
||||
|
||||
**Files:**
|
||||
- No source edits expected unless verification finds a regression.
|
||||
|
||||
- [x] **Step 1: Build production frontend**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cd web && pnpm build
|
||||
```
|
||||
|
||||
Expected: PASS. Build output should still contain separate Mermaid and Leaflet chunks, but they should not be required by the auth/signup initial route.
|
||||
|
||||
- [x] **Step 2: Inspect build output for heavy chunks**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cd web && find dist/assets -maxdepth 1 -type f \( -name '*mermaid*' -o -name '*leaflet*' -o -name '*katex*' \) -print | sort
|
||||
```
|
||||
|
||||
Expected: Mermaid and Leaflet assets may exist as lazy chunks. Their existence is fine; the goal is that auth/signup does not request them initially.
|
||||
|
||||
- [x] **Step 3: Start production preview**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cd web && pnpm exec vite preview --host 127.0.0.1 --port 4173
|
||||
```
|
||||
|
||||
Expected: Preview server starts on `http://127.0.0.1:4173/`. Keep this session running until browser verification is complete.
|
||||
|
||||
- [x] **Step 4: Verify `/auth/signup` network with browser tooling**
|
||||
|
||||
Open:
|
||||
|
||||
```text
|
||||
http://127.0.0.1:4173/auth/signup
|
||||
```
|
||||
|
||||
Expected initial document and asset requests do not include filenames containing:
|
||||
|
||||
```text
|
||||
mermaid
|
||||
leaflet
|
||||
```
|
||||
|
||||
If KaTeX CSS still appears on `/auth/signup`, inspect the chunk initiator and remove any remaining static import path that reaches `MemoMarkdownRenderer` from auth/signup.
|
||||
|
||||
- [ ] **Step 5: Smoke test feature lazy loading**
|
||||
|
||||
Use the running app or a local backend/dev setup to verify:
|
||||
|
||||
```text
|
||||
1. A memo containing a Mermaid code block renders the diagram and requests the Mermaid chunk only when the memo content appears.
|
||||
2. A memo containing inline or block math displays KaTeX styling when memo content appears.
|
||||
3. Opening the location picker loads Leaflet assets and the picker remains interactive.
|
||||
4. Opening a memo location popover loads Leaflet assets and shows the pinned map.
|
||||
5. Opening `/u/:username?view=map` loads the profile map and marker cluster behavior still works.
|
||||
```
|
||||
|
||||
Expected: Features behave as before after their lazy chunks load.
|
||||
|
||||
Result in this session: live feature smoke was not completed because the production preview has no authenticated backend session or seeded memo data. Static verification confirmed the relevant feature chunks still exist and load paths are behind lazy imports.
|
||||
|
||||
- [x] **Step 6: Stop preview server**
|
||||
|
||||
Stop the preview command from Step 3 with `Ctrl-C`.
|
||||
|
||||
## Task 6: Commit Implementation
|
||||
|
||||
**Files:**
|
||||
- All source files modified by Tasks 1-4.
|
||||
|
||||
- [x] **Step 1: Review final diff**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
git diff -- web/src/main.tsx web/src/components/map web/src/components/MemoEditor web/src/components/MemoMetadata/Location web/src/pages/UserProfile.tsx web/src/components/MemoContent web/src/components/UserMemoMap/UserMemoMap.tsx
|
||||
```
|
||||
|
||||
Expected: Diff is limited to lazy-loading heavy optional dependencies and plain coordinate type changes.
|
||||
|
||||
- [x] **Step 2: Run final verification**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cd web && pnpm lint && pnpm build
|
||||
```
|
||||
|
||||
Expected: PASS.
|
||||
|
||||
- [x] **Step 3: Commit**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
git add web/src/main.tsx web/src/components/map web/src/components/MemoEditor web/src/components/MemoMetadata/Location web/src/pages/UserProfile.tsx web/src/components/MemoContent web/src/components/UserMemoMap/UserMemoMap.tsx
|
||||
git commit -m "perf: lazy load heavy first-screen dependencies"
|
||||
```
|
||||
|
||||
Expected: Commit succeeds with only the intended source changes.
|
||||
|
||||
## Self-Review
|
||||
|
||||
- Spec coverage: The plan removes global Leaflet/KaTeX CSS, dynamically imports Mermaid, lazy-loads map UI, keeps Leaflet types out of parent boundaries, and verifies auth/signup network behavior.
|
||||
- Placeholder scan: No placeholder markers, unresolved decisions, or vague generic handling steps remain.
|
||||
- Type consistency: `MapPoint` is the shared parent-facing coordinate type; `LatLng` remains internal to `LocationPicker` and map implementation files only.
|
||||
@@ -0,0 +1,776 @@
|
||||
# Placeholder Component 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:** Replace `Empty.tsx` with a reusable `<Placeholder>` component covering empty / loading / noResults / notFound states, each rendering a hand-curated ASCII bird from a pool-shaped data file with subtle CSS-only animation.
|
||||
|
||||
**Architecture:** Single component (`Placeholder/index.tsx`) reads from a co-located `ascii-pool.ts` data file via a `pickPiece(variant)` picker. Motion is CSS keyframes in `Placeholder.css`, gated by `prefers-reduced-motion`. Default messages live in a `messages.ts` seam ready for future i18n. Integration is narrow: only `Inboxes.tsx` is rewired in this PR; `Empty.tsx` is deleted.
|
||||
|
||||
**Tech Stack:** React 19 · TypeScript · Tailwind v4 (via `@tailwindcss/vite`) · Vitest + `@testing-library/react` + jsdom · Biome for lint/format · `cn` helper from `@/lib/utils` for class composition.
|
||||
|
||||
---
|
||||
|
||||
## File Structure
|
||||
|
||||
**Create:**
|
||||
- `web/src/components/Placeholder/index.tsx` — public component (default export)
|
||||
- `web/src/components/Placeholder/Placeholder.css` — keyframes + motion classes
|
||||
- `web/src/components/Placeholder/ascii-pool.ts` — types, `ASCII_POOL` array, `pickPiece()`
|
||||
- `web/src/components/Placeholder/messages.ts` — `DEFAULT_MESSAGES` map
|
||||
- `web/src/components/Placeholder/CREDITS.md` — Joan Stark attribution
|
||||
- `web/tests/placeholder-pool.test.ts` — picker + pool integrity tests
|
||||
- `web/tests/placeholder-component.test.tsx` — component render tests
|
||||
|
||||
**Modify:**
|
||||
- `web/src/pages/Inboxes.tsx` — replace `<Empty />` with `<Placeholder variant="empty" message={…} />`
|
||||
|
||||
**Delete:**
|
||||
- `web/src/components/Empty.tsx`
|
||||
|
||||
---
|
||||
|
||||
## Task 0: Commit this plan
|
||||
|
||||
**Files:**
|
||||
- Add: `docs/superpowers/plans/2026-05-12-placeholder-component.md`
|
||||
|
||||
- [ ] **Step 1: Commit the plan document on its own**
|
||||
|
||||
```bash
|
||||
git add docs/superpowers/plans/2026-05-12-placeholder-component.md
|
||||
git commit -m "docs: add Placeholder component implementation plan"
|
||||
```
|
||||
|
||||
This keeps the planning artifact separate from feature commits.
|
||||
|
||||
---
|
||||
|
||||
## Task 1: Scaffold pool types and picker (TDD)
|
||||
|
||||
**Files:**
|
||||
- Create: `web/src/components/Placeholder/ascii-pool.ts`
|
||||
- Test: `web/tests/placeholder-pool.test.ts`
|
||||
|
||||
- [ ] **Step 1: Write the failing tests for `pickPiece` and pool shape**
|
||||
|
||||
Create `web/tests/placeholder-pool.test.ts`:
|
||||
|
||||
```ts
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { ASCII_POOL, pickPiece, type PlaceholderVariant } from "@/components/Placeholder/ascii-pool";
|
||||
|
||||
const VARIANTS: PlaceholderVariant[] = ["empty", "loading", "noResults", "notFound"];
|
||||
|
||||
describe("ASCII_POOL integrity", () => {
|
||||
it("contains at least one piece per variant", () => {
|
||||
for (const variant of VARIANTS) {
|
||||
const matches = ASCII_POOL.filter((p) => p.variant === variant);
|
||||
expect(matches.length, `variant=${variant}`).toBeGreaterThanOrEqual(1);
|
||||
}
|
||||
});
|
||||
|
||||
it("uses unique ids", () => {
|
||||
const ids = ASCII_POOL.map((p) => p.id);
|
||||
expect(new Set(ids).size).toBe(ids.length);
|
||||
});
|
||||
|
||||
it("preserves the jgs credit on every piece", () => {
|
||||
for (const piece of ASCII_POOL) {
|
||||
expect(piece.credit, `piece=${piece.id}`).toMatch(/jgs/);
|
||||
}
|
||||
});
|
||||
|
||||
it("uses a known motion style on every piece", () => {
|
||||
for (const piece of ASCII_POOL) {
|
||||
expect(["bob", "flutter", "none"]).toContain(piece.motion);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("pickPiece", () => {
|
||||
it("returns a piece matching the requested variant", () => {
|
||||
for (const variant of VARIANTS) {
|
||||
const piece = pickPiece(variant);
|
||||
expect(piece.variant).toBe(variant);
|
||||
}
|
||||
});
|
||||
|
||||
it("returns a non-empty ascii string", () => {
|
||||
const piece = pickPiece("empty");
|
||||
expect(piece.ascii.length).toBeGreaterThan(0);
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run the tests and verify they fail**
|
||||
|
||||
```bash
|
||||
cd web && pnpm test placeholder-pool
|
||||
```
|
||||
|
||||
Expected: FAIL — module `@/components/Placeholder/ascii-pool` not found.
|
||||
|
||||
- [ ] **Step 3: Implement `ascii-pool.ts` with types, an empty pool, and the picker**
|
||||
|
||||
Create `web/src/components/Placeholder/ascii-pool.ts`:
|
||||
|
||||
```ts
|
||||
export type PlaceholderVariant = "empty" | "loading" | "noResults" | "notFound";
|
||||
|
||||
export type MotionStyle = "bob" | "flutter" | "none";
|
||||
|
||||
export interface AsciiPiece {
|
||||
/** Stable identifier — used as React key and for debugging. */
|
||||
id: string;
|
||||
/** Which placeholder state this piece is shown for. */
|
||||
variant: PlaceholderVariant;
|
||||
/** ASCII art preserved verbatim — must keep every space. */
|
||||
ascii: string;
|
||||
/** Attribution shown beneath the bird, e.g. "jgs · 4/97". */
|
||||
credit: string;
|
||||
/** Motion hint applied to the <pre>. */
|
||||
motion: MotionStyle;
|
||||
}
|
||||
|
||||
export const ASCII_POOL: AsciiPiece[] = [];
|
||||
|
||||
export function pickPiece(variant: PlaceholderVariant): AsciiPiece {
|
||||
const matches = ASCII_POOL.filter((p) => p.variant === variant);
|
||||
if (matches.length === 0) {
|
||||
throw new Error(`No ASCII piece registered for variant "${variant}"`);
|
||||
}
|
||||
return matches[Math.floor(Math.random() * matches.length)];
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run the tests and verify they still fail (pool is empty)**
|
||||
|
||||
```bash
|
||||
cd web && pnpm test placeholder-pool
|
||||
```
|
||||
|
||||
Expected: FAIL — "contains at least one piece per variant" expectations not met (because `ASCII_POOL` is `[]`).
|
||||
|
||||
This is the expected red — pool gets seeded in the next task.
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add web/src/components/Placeholder/ascii-pool.ts web/tests/placeholder-pool.test.ts
|
||||
git commit -m "feat(placeholder): scaffold ASCII pool types and picker"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 2: Seed the four ASCII pieces
|
||||
|
||||
**Files:**
|
||||
- Modify: `web/src/components/Placeholder/ascii-pool.ts`
|
||||
|
||||
- [ ] **Step 1: Replace the empty `ASCII_POOL` array with the four seed entries**
|
||||
|
||||
In `web/src/components/Placeholder/ascii-pool.ts`, replace `export const ASCII_POOL: AsciiPiece[] = [];` with the four entries below. Preserve every space and newline in the `ascii` strings exactly — they are template literals with escaped backslashes/backticks per JS rules.
|
||||
|
||||
```ts
|
||||
export const ASCII_POOL: AsciiPiece[] = [
|
||||
{
|
||||
id: "jgs-crested-parrot",
|
||||
variant: "empty",
|
||||
credit: "jgs · 4/97",
|
||||
motion: "bob",
|
||||
ascii: ` .---.
|
||||
/ 6_6
|
||||
\\_ (__\\
|
||||
// \\\\
|
||||
(( ))
|
||||
=====""===""=====
|
||||
|||
|
||||
|`,
|
||||
},
|
||||
{
|
||||
id: "jgs-hummingbird-sm",
|
||||
variant: "loading",
|
||||
credit: "jgs · 7/98",
|
||||
motion: "flutter",
|
||||
ascii: ` , _
|
||||
{ \\/\`o;====-
|
||||
.----'-/\`-/
|
||||
\`'-..-| /
|
||||
/\\/\\
|
||||
\`--\``,
|
||||
},
|
||||
{
|
||||
id: "jgs-wide-eyed-owl",
|
||||
variant: "noResults",
|
||||
credit: "jgs · 2/01",
|
||||
motion: "bob",
|
||||
ascii: ` __ __
|
||||
\\ \`-'"'-\` /
|
||||
/ \\_ _/ \\
|
||||
| d\\_/b |
|
||||
.'\\ V /'.
|
||||
/ '-...-' \\
|
||||
| / \\ |
|
||||
\\/\\ /\\/
|
||||
==(||)---(||)==`,
|
||||
},
|
||||
{
|
||||
id: "jgs-bird-flown-away",
|
||||
variant: "notFound",
|
||||
credit: "jgs · 7/96",
|
||||
motion: "flutter",
|
||||
ascii: ` ___
|
||||
_,-' ______
|
||||
.' .-' ____7
|
||||
/ / ___7
|
||||
_| / ___7
|
||||
>(')\\ | ___7
|
||||
\\\\/ \\_______
|
||||
' _======>
|
||||
\`'----\\\\\``,
|
||||
},
|
||||
];
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run the pool tests and verify they pass**
|
||||
|
||||
```bash
|
||||
cd web && pnpm test placeholder-pool
|
||||
```
|
||||
|
||||
Expected: PASS for all six assertions.
|
||||
|
||||
- [ ] **Step 3: Commit**
|
||||
|
||||
```bash
|
||||
git add web/src/components/Placeholder/ascii-pool.ts
|
||||
git commit -m "feat(placeholder): seed pool with four jgs ASCII bird pieces"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 3: Default messages
|
||||
|
||||
**Files:**
|
||||
- Create: `web/src/components/Placeholder/messages.ts`
|
||||
|
||||
- [ ] **Step 1: Add a tiny test for the messages map**
|
||||
|
||||
Append to `web/tests/placeholder-pool.test.ts`:
|
||||
|
||||
```ts
|
||||
import { DEFAULT_MESSAGES } from "@/components/Placeholder/messages";
|
||||
|
||||
describe("DEFAULT_MESSAGES", () => {
|
||||
it("provides a non-empty message for every variant", () => {
|
||||
for (const variant of VARIANTS) {
|
||||
expect(DEFAULT_MESSAGES[variant], `variant=${variant}`).toBeTruthy();
|
||||
expect(DEFAULT_MESSAGES[variant].trim().length).toBeGreaterThan(0);
|
||||
}
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run the test and verify it fails**
|
||||
|
||||
```bash
|
||||
cd web && pnpm test placeholder-pool
|
||||
```
|
||||
|
||||
Expected: FAIL — module `@/components/Placeholder/messages` not found.
|
||||
|
||||
- [ ] **Step 3: Create `messages.ts`**
|
||||
|
||||
```ts
|
||||
import type { PlaceholderVariant } from "./ascii-pool";
|
||||
|
||||
/**
|
||||
* Default copy shown beneath the ASCII art when no `message` prop is supplied.
|
||||
*
|
||||
* Future i18n: swap these strings for `t("placeholder.<variant>")` lookups via
|
||||
* `react-i18next` without touching the component.
|
||||
*/
|
||||
export const DEFAULT_MESSAGES: Record<PlaceholderVariant, string> = {
|
||||
empty: "No memos yet",
|
||||
loading: "Loading…",
|
||||
noResults: "Nothing matches that search",
|
||||
notFound: "This page flew the coop",
|
||||
};
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run the test and verify it passes**
|
||||
|
||||
```bash
|
||||
cd web && pnpm test placeholder-pool
|
||||
```
|
||||
|
||||
Expected: PASS — all variants have a non-empty default message.
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add web/src/components/Placeholder/messages.ts web/tests/placeholder-pool.test.ts
|
||||
git commit -m "feat(placeholder): add DEFAULT_MESSAGES map"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 4: Animation keyframes
|
||||
|
||||
**Files:**
|
||||
- Create: `web/src/components/Placeholder/Placeholder.css`
|
||||
|
||||
- [ ] **Step 1: Create the stylesheet**
|
||||
|
||||
```css
|
||||
/*
|
||||
* Animations for <Placeholder>.
|
||||
*
|
||||
* All keyframes are wrapped in a prefers-reduced-motion guard so users who
|
||||
* opt out of motion see a static bird and an instantly-visible message.
|
||||
*/
|
||||
|
||||
@media (prefers-reduced-motion: no-preference) {
|
||||
.placeholder-motion-bob {
|
||||
animation: placeholder-bob 3.4s ease-in-out infinite;
|
||||
}
|
||||
|
||||
.placeholder-motion-flutter {
|
||||
animation: placeholder-flutter 0.7s ease-in-out infinite;
|
||||
}
|
||||
|
||||
.placeholder-fade-in {
|
||||
animation: placeholder-fade 1s ease-out 0.3s both;
|
||||
opacity: 0;
|
||||
}
|
||||
}
|
||||
|
||||
@keyframes placeholder-bob {
|
||||
0%, 100% { transform: translateY(0); }
|
||||
50% { transform: translateY(-4px); }
|
||||
}
|
||||
|
||||
@keyframes placeholder-flutter {
|
||||
0%, 100% { transform: translate(0, 0); }
|
||||
50% { transform: translate(2px, -1px); }
|
||||
}
|
||||
|
||||
@keyframes placeholder-fade {
|
||||
to { opacity: 1; }
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Commit**
|
||||
|
||||
```bash
|
||||
git add web/src/components/Placeholder/Placeholder.css
|
||||
git commit -m "feat(placeholder): add motion keyframes with reduced-motion guard"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 5: Placeholder component (TDD)
|
||||
|
||||
**Files:**
|
||||
- Create: `web/src/components/Placeholder/index.tsx`
|
||||
- Test: `web/tests/placeholder-component.test.tsx`
|
||||
|
||||
- [ ] **Step 1: Write the failing component tests**
|
||||
|
||||
Create `web/tests/placeholder-component.test.tsx`:
|
||||
|
||||
```tsx
|
||||
import { render, screen } from "@testing-library/react";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import Placeholder from "@/components/Placeholder";
|
||||
import { DEFAULT_MESSAGES } from "@/components/Placeholder/messages";
|
||||
|
||||
describe("<Placeholder>", () => {
|
||||
it("renders the default message for variant=empty", () => {
|
||||
render(<Placeholder variant="empty" />);
|
||||
expect(screen.getByText(DEFAULT_MESSAGES.empty)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("renders the default message for variant=loading", () => {
|
||||
render(<Placeholder variant="loading" />);
|
||||
expect(screen.getByText(DEFAULT_MESSAGES.loading)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("renders the default message for variant=noResults", () => {
|
||||
render(<Placeholder variant="noResults" />);
|
||||
expect(screen.getByText(DEFAULT_MESSAGES.noResults)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("renders the default message for variant=notFound", () => {
|
||||
render(<Placeholder variant="notFound" />);
|
||||
expect(screen.getByText(DEFAULT_MESSAGES.notFound)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("overrides the default message when `message` prop is passed", () => {
|
||||
render(<Placeholder variant="empty" message="Custom copy goes here" />);
|
||||
expect(screen.getByText("Custom copy goes here")).toBeInTheDocument();
|
||||
expect(screen.queryByText(DEFAULT_MESSAGES.empty)).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("renders the ASCII art inside a <pre> with aria-hidden", () => {
|
||||
const { container } = render(<Placeholder variant="empty" />);
|
||||
const pre = container.querySelector("pre");
|
||||
expect(pre).not.toBeNull();
|
||||
expect(pre).toHaveAttribute("aria-hidden", "true");
|
||||
expect(pre!.textContent!.length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it("renders a jgs credit string", () => {
|
||||
render(<Placeholder variant="empty" />);
|
||||
expect(screen.getByText(/jgs/)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('applies role="status" and aria-live="polite" ONLY when variant=loading', () => {
|
||||
const { rerender, container } = render(<Placeholder variant="empty" />);
|
||||
expect(container.querySelector('[role="status"]')).toBeNull();
|
||||
|
||||
rerender(<Placeholder variant="loading" />);
|
||||
const live = container.querySelector('[role="status"]');
|
||||
expect(live).not.toBeNull();
|
||||
expect(live).toHaveAttribute("aria-live", "polite");
|
||||
});
|
||||
|
||||
it("renders children below the message when provided", () => {
|
||||
render(
|
||||
<Placeholder variant="notFound">
|
||||
<button type="button">Go home</button>
|
||||
</Placeholder>,
|
||||
);
|
||||
expect(screen.getByRole("button", { name: "Go home" })).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("merges a custom className onto the outer wrapper", () => {
|
||||
const { container } = render(<Placeholder variant="empty" className="custom-test-class" />);
|
||||
expect(container.firstChild).toHaveClass("custom-test-class");
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run the tests and verify they fail**
|
||||
|
||||
```bash
|
||||
cd web && pnpm test placeholder-component
|
||||
```
|
||||
|
||||
Expected: FAIL — module `@/components/Placeholder` not found.
|
||||
|
||||
- [ ] **Step 3: Implement `index.tsx`**
|
||||
|
||||
Create `web/src/components/Placeholder/index.tsx`:
|
||||
|
||||
```tsx
|
||||
import { useMemo, type ReactNode } from "react";
|
||||
import { cn } from "@/lib/utils";
|
||||
import { pickPiece, type MotionStyle, type PlaceholderVariant } from "./ascii-pool";
|
||||
import { DEFAULT_MESSAGES } from "./messages";
|
||||
import "./Placeholder.css";
|
||||
|
||||
interface PlaceholderProps {
|
||||
variant: PlaceholderVariant;
|
||||
message?: string;
|
||||
children?: ReactNode;
|
||||
className?: string;
|
||||
}
|
||||
|
||||
const MOTION_CLASS: Record<MotionStyle, string> = {
|
||||
bob: "placeholder-motion-bob",
|
||||
flutter: "placeholder-motion-flutter",
|
||||
none: "",
|
||||
};
|
||||
|
||||
const Placeholder = ({ variant, message, children, className }: PlaceholderProps) => {
|
||||
// Stable for the lifetime of this mount; re-rolls only if `variant` changes
|
||||
// (which is rare in practice — most callers pass a constant).
|
||||
const piece = useMemo(() => pickPiece(variant), [variant]);
|
||||
const resolvedMessage = message ?? DEFAULT_MESSAGES[variant];
|
||||
const isLoading = variant === "loading";
|
||||
|
||||
return (
|
||||
<div
|
||||
role={isLoading ? "status" : undefined}
|
||||
aria-live={isLoading ? "polite" : undefined}
|
||||
className={cn("flex flex-col items-center justify-center max-w-md mx-auto px-4 py-8", className)}
|
||||
>
|
||||
<pre
|
||||
aria-hidden="true"
|
||||
className={cn(
|
||||
"font-mono text-xs sm:text-sm leading-tight text-muted-foreground whitespace-pre m-0",
|
||||
MOTION_CLASS[piece.motion],
|
||||
)}
|
||||
>
|
||||
{piece.ascii}
|
||||
</pre>
|
||||
<p className="mt-3 font-mono text-sm text-muted-foreground placeholder-fade-in">
|
||||
{resolvedMessage}
|
||||
</p>
|
||||
<p className="mt-1 font-mono text-[10px] text-muted-foreground/60 placeholder-fade-in">
|
||||
{piece.credit}
|
||||
</p>
|
||||
{children && <div className="mt-4">{children}</div>}
|
||||
</div>
|
||||
);
|
||||
};
|
||||
|
||||
export default Placeholder;
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run the tests and verify they pass**
|
||||
|
||||
```bash
|
||||
cd web && pnpm test placeholder-component
|
||||
```
|
||||
|
||||
Expected: PASS — all ten assertions green.
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add web/src/components/Placeholder/index.tsx web/tests/placeholder-component.test.tsx
|
||||
git commit -m "feat(placeholder): implement <Placeholder> with variant-driven ASCII pool"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 6: Attribution credits
|
||||
|
||||
**Files:**
|
||||
- Create: `web/src/components/Placeholder/CREDITS.md`
|
||||
|
||||
- [ ] **Step 1: Create `CREDITS.md`**
|
||||
|
||||
```markdown
|
||||
# ASCII Art Credits
|
||||
|
||||
The ASCII bird illustrations rendered by `<Placeholder>` are from **Joan Stark's**
|
||||
classic ASCII art collection. Each piece is signed with her `jgs` tag and the
|
||||
month/year it was published.
|
||||
|
||||
- Source archive: https://github.com/oldcompcz/jgs (Joan Stark's ASCII Art Gallery)
|
||||
- Original site (preserved via WebArchive): https://web.archive.org/web/20091028013825/http://www.geocities.com/SoHo/7373/
|
||||
- Wikipedia: https://en.wikipedia.org/wiki/Joan_Stark
|
||||
|
||||
Joan Stark distributed her art freely on Usenet and the early web. We retain
|
||||
the `jgs` signature visible beneath each piece in the UI so attribution travels
|
||||
with the art wherever it is shown.
|
||||
|
||||
If you add new ASCII pieces to `ascii-pool.ts`:
|
||||
|
||||
- Prefer well-attributed art from established collections.
|
||||
- Keep the original artist signature in the `credit` field (e.g. `"jgs · 4/97"`).
|
||||
- If using a different artist, link the source in this file.
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Commit**
|
||||
|
||||
```bash
|
||||
git add web/src/components/Placeholder/CREDITS.md
|
||||
git commit -m "docs(placeholder): credit Joan Stark for ASCII bird art"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 7: Wire into `Inboxes.tsx` and delete `Empty.tsx`
|
||||
|
||||
**Files:**
|
||||
- Modify: `web/src/pages/Inboxes.tsx`
|
||||
- Delete: `web/src/components/Empty.tsx`
|
||||
|
||||
- [ ] **Step 1: Read the current empty-state block in `Inboxes.tsx`**
|
||||
|
||||
Open `web/src/pages/Inboxes.tsx`. The relevant block is at lines 99–105:
|
||||
|
||||
```tsx
|
||||
{notifications.length === 0 ? (
|
||||
<div className="w-full py-16 flex flex-col justify-center items-center">
|
||||
<Empty />
|
||||
<p className="mt-4 text-sm text-muted-foreground">
|
||||
{filter === "unread" ? t("inbox.no-unread") : filter === "archived" ? t("inbox.no-archived") : t("message.no-data")}
|
||||
</p>
|
||||
</div>
|
||||
) : (
|
||||
```
|
||||
|
||||
The outer `<div className="w-full py-16 flex flex-col justify-center items-center">` and the inner `<p>` both become redundant — `<Placeholder>` handles its own centering and message.
|
||||
|
||||
- [ ] **Step 2: Swap the import**
|
||||
|
||||
In `web/src/pages/Inboxes.tsx`, replace the import line:
|
||||
|
||||
```tsx
|
||||
import Empty from "@/components/Empty";
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```tsx
|
||||
import Placeholder from "@/components/Placeholder";
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Replace the empty-state JSX**
|
||||
|
||||
Replace lines 99–105 (the existing empty-state block above) with:
|
||||
|
||||
```tsx
|
||||
{notifications.length === 0 ? (
|
||||
<Placeholder
|
||||
variant="empty"
|
||||
message={
|
||||
filter === "unread"
|
||||
? t("inbox.no-unread")
|
||||
: filter === "archived"
|
||||
? t("inbox.no-archived")
|
||||
: t("message.no-data")
|
||||
}
|
||||
/>
|
||||
) : (
|
||||
```
|
||||
|
||||
(Only the truthy branch of the ternary changes; leave the `: (` start of the falsy branch and everything below it untouched.)
|
||||
|
||||
- [ ] **Step 4: Delete `Empty.tsx`**
|
||||
|
||||
```bash
|
||||
git rm web/src/components/Empty.tsx
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Verify nothing else imports `Empty`**
|
||||
|
||||
```bash
|
||||
cd /Users/steven/Projects/usememos/memos && grep -rn 'from "@/components/Empty"\|from "./Empty"\|from "../Empty"' web/src 2>/dev/null
|
||||
```
|
||||
|
||||
Expected: no output. If anything matches, update that file to use `<Placeholder variant="empty" />` before continuing.
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add web/src/pages/Inboxes.tsx web/src/components/Empty.tsx
|
||||
git commit -m "feat(inboxes): use <Placeholder variant=empty> in place of <Empty>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 8: Verification — lint, types, tests, build, dev preview
|
||||
|
||||
**Files:** none modified (verification only)
|
||||
|
||||
- [ ] **Step 1: Run the lint + typecheck**
|
||||
|
||||
```bash
|
||||
cd web && pnpm lint
|
||||
```
|
||||
|
||||
Expected: exits 0. If Biome flags anything (formatting, sort-order, unused imports), run `pnpm lint:fix` and re-run `pnpm lint`. Commit the fix separately if any changes are required:
|
||||
|
||||
```bash
|
||||
git add -p web/src web/tests
|
||||
git commit -m "chore(placeholder): biome auto-fixes"
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run the full test suite**
|
||||
|
||||
```bash
|
||||
cd web && pnpm test
|
||||
```
|
||||
|
||||
Expected: all suites green, including `placeholder-pool` (8 assertions) and `placeholder-component` (10 assertions). Existing tests must still pass.
|
||||
|
||||
- [ ] **Step 3: Run the production build**
|
||||
|
||||
```bash
|
||||
cd web && pnpm build
|
||||
```
|
||||
|
||||
Expected: exits 0. Confirms TypeScript still compiles end-to-end and the new CSS import is picked up by Vite.
|
||||
|
||||
- [ ] **Step 4: Manual visual check — start the dev server**
|
||||
|
||||
```bash
|
||||
cd web && pnpm dev
|
||||
```
|
||||
|
||||
In a browser, navigate to the running URL (Vite prints it). Sign in with any test account, open the **Inbox** page, and confirm:
|
||||
|
||||
1. When inbox is empty, the crested-parrot ASCII bird is visible.
|
||||
2. It bobs gently every ~3.4 seconds.
|
||||
3. The message text (one of "no unread", "no archived", or "no data") appears below with a soft fade-in.
|
||||
4. The small `jgs · 4/97` credit is visible below the message.
|
||||
5. No console errors or warnings.
|
||||
|
||||
Stop the dev server with `Ctrl+C`.
|
||||
|
||||
- [ ] **Step 5: (Optional) Reduced-motion check**
|
||||
|
||||
Open browser DevTools → command menu → "Emulate CSS prefers-reduced-motion: reduce". Reload the inbox empty state. The bird should be **static** and the message should appear instantly (no fade).
|
||||
|
||||
- [ ] **Step 6: No-op commit point**
|
||||
|
||||
If steps 1–4 all passed and no further changes were needed, there is nothing to commit. Proceed to the next task.
|
||||
|
||||
---
|
||||
|
||||
## Task 9: Document the work in the PR body
|
||||
|
||||
**Files:** none modified
|
||||
|
||||
- [ ] **Step 1: Draft a PR description**
|
||||
|
||||
When opening the PR, use a body like:
|
||||
|
||||
```markdown
|
||||
## Summary
|
||||
|
||||
- Adds a new `<Placeholder variant="empty | loading | noResults | notFound">` component that renders a hand-curated ASCII bird from a pool-shaped data file, with subtle CSS-only motion that respects `prefers-reduced-motion`.
|
||||
- Replaces the single-purpose `Empty.tsx` (used in `Inboxes.tsx`) with `<Placeholder variant="empty">`.
|
||||
- ASCII art is from Joan Stark's (jgs) classic collection — attribution is preserved on every piece and in a co-located `CREDITS.md`.
|
||||
|
||||
## Out of scope (follow-up opportunities)
|
||||
|
||||
- Wire `<Placeholder variant="noResults">` into the memo search results page.
|
||||
- Wire `<Placeholder variant="notFound">` into the router 404 catch-all.
|
||||
- Wire `<Placeholder variant="loading">` into Suspense fallbacks.
|
||||
- Seed additional ASCII pieces per variant — the pool architecture supports it; just append entries to `ASCII_POOL`.
|
||||
|
||||
## Test plan
|
||||
|
||||
- [ ] `pnpm lint` clean
|
||||
- [ ] `pnpm test` green (incl. new `placeholder-pool` and `placeholder-component` suites)
|
||||
- [ ] `pnpm build` succeeds
|
||||
- [ ] Inbox empty state shows the ASCII parrot, bobs, and renders the filter-specific message
|
||||
- [ ] `prefers-reduced-motion: reduce` produces a static bird and an instantly-visible message
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Open the PR** (skip if user prefers to do this themselves)
|
||||
|
||||
This step is left as a manual handoff — do not push or open the PR unless the user has explicitly authorized it.
|
||||
|
||||
---
|
||||
|
||||
## Self-Review Notes
|
||||
|
||||
This plan covers the spec's nine sections as follows:
|
||||
|
||||
| Spec section | Implemented by |
|
||||
|---|---|
|
||||
| Public Component | Task 5 |
|
||||
| ASCII Pool | Tasks 1, 2 |
|
||||
| Default Messages | Task 3 |
|
||||
| Animation | Task 4 (CSS) + Task 5 (component wires classes) |
|
||||
| Accessibility | Task 5 (test assertions + impl) |
|
||||
| File Layout | Tasks 1–6 (all five files created) |
|
||||
| Integration | Task 7 (Inboxes rewire + Empty delete) |
|
||||
| Credits | Task 6 |
|
||||
| Testing | Tasks 1, 3, 5 (pool tests + component tests) |
|
||||
|
||||
No spec section is unimplemented. No "TBD" / "TODO" / vague-handwave language is used in any step. Types, method signatures, and class names referenced across tasks match:
|
||||
|
||||
- `PlaceholderVariant`, `MotionStyle`, `AsciiPiece`, `ASCII_POOL`, `pickPiece` — consistent across Tasks 1, 2, 3, 5
|
||||
- `DEFAULT_MESSAGES` — defined in Task 3, consumed in Task 5
|
||||
- `placeholder-motion-bob`, `placeholder-motion-flutter`, `placeholder-fade-in` — defined in Task 4 CSS, consumed in Task 5 component
|
||||
- `cn` from `@/lib/utils` — matches existing codebase convention (verified in pre-plan exploration)
|
||||
- `Placeholder` is a default export — matches the convention used by `Empty.tsx` and most other `web/src/components/*.tsx` files
|
||||
@@ -0,0 +1,673 @@
|
||||
# Remove react-use 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:** Remove `react-use` from the frontend while preserving current hook behavior and eliminating the transitive `js-cookie@2.2.1` dependency.
|
||||
|
||||
**Architecture:** Replace simple one-off `react-use` helpers with native React hooks inside the consuming components. Add two focused local hooks in `web/src/hooks/` for reused debounce behavior and typed localStorage state. Regenerate the pnpm lockfile from `web/package.json` instead of manually editing lockfile entries.
|
||||
|
||||
**Tech Stack:** React 19, TypeScript 6, Vite 8, Vitest 4, Testing Library, pnpm 11.
|
||||
|
||||
---
|
||||
|
||||
## File Structure
|
||||
|
||||
- Create `web/src/hooks/useDebouncedEffect.ts`
|
||||
- Shared debounce hook for effect-style callbacks.
|
||||
- Create `web/src/hooks/useLocalStorage.ts`
|
||||
- Typed localStorage state hook for persisted UI preferences.
|
||||
- Modify `web/src/hooks/index.ts`
|
||||
- Export the two new hooks for existing `@/hooks` barrel import style.
|
||||
- Create `web/tests/hooks.test.tsx`
|
||||
- Unit tests for the two new local hooks.
|
||||
- Modify `web/src/components/MemoEditor/Toolbar/InsertMenu.tsx`
|
||||
- Replace `react-use` debounce import with local `useDebouncedEffect`.
|
||||
- Modify `web/src/components/MemoEditor/hooks/useLinkMemo.ts`
|
||||
- Replace deep `react-use` debounce import with local `useDebouncedEffect`.
|
||||
- Modify `web/src/components/MemoExplorer/TagsSection.tsx`
|
||||
- Replace deep `react-use` localStorage import with local `useLocalStorage`.
|
||||
- Modify `web/src/components/TagTree.tsx`
|
||||
- Replace `useToggle` with native `useState`.
|
||||
- Modify `web/src/components/MobileHeader.tsx`
|
||||
- Replace `useWindowScroll` with native `useState` and `useEffect`.
|
||||
- Modify `web/src/layouts/RootLayout.tsx`
|
||||
- Replace `usePrevious` with native `useRef` and `useEffect`.
|
||||
- Modify `web/package.json`
|
||||
- Remove the direct `react-use` dependency.
|
||||
- Modify `web/pnpm-lock.yaml`
|
||||
- Regenerate through pnpm after `react-use` is removed.
|
||||
|
||||
---
|
||||
|
||||
### Task 1: Add Failing Hook Tests
|
||||
|
||||
**Files:**
|
||||
- Create: `web/tests/hooks.test.tsx`
|
||||
|
||||
- [ ] **Step 1: Write tests for local hook behavior**
|
||||
|
||||
Create `web/tests/hooks.test.tsx`:
|
||||
|
||||
```tsx
|
||||
import { act, renderHook } from "@testing-library/react";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { useDebouncedEffect, useLocalStorage } from "@/hooks";
|
||||
|
||||
describe("useLocalStorage", () => {
|
||||
beforeEach(() => {
|
||||
window.localStorage.clear();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
window.localStorage.clear();
|
||||
});
|
||||
|
||||
it("uses the default value when storage is empty", () => {
|
||||
const { result } = renderHook(() => useLocalStorage("hook-test-empty", false));
|
||||
|
||||
expect(result.current[0]).toBe(false);
|
||||
});
|
||||
|
||||
it("reads and writes JSON values", () => {
|
||||
window.localStorage.setItem("hook-test-existing", JSON.stringify(true));
|
||||
|
||||
const { result } = renderHook(() => useLocalStorage("hook-test-existing", false));
|
||||
|
||||
expect(result.current[0]).toBe(true);
|
||||
|
||||
act(() => {
|
||||
result.current[1](false);
|
||||
});
|
||||
|
||||
expect(result.current[0]).toBe(false);
|
||||
expect(window.localStorage.getItem("hook-test-existing")).toBe("false");
|
||||
});
|
||||
|
||||
it("supports updater functions", () => {
|
||||
const { result } = renderHook(() => useLocalStorage("hook-test-updater", false));
|
||||
|
||||
act(() => {
|
||||
result.current[1]((current) => !current);
|
||||
});
|
||||
|
||||
expect(result.current[0]).toBe(true);
|
||||
expect(window.localStorage.getItem("hook-test-updater")).toBe("true");
|
||||
});
|
||||
|
||||
it("falls back to the default value for malformed storage", () => {
|
||||
window.localStorage.setItem("hook-test-malformed", "{bad json");
|
||||
|
||||
const { result } = renderHook(() => useLocalStorage("hook-test-malformed", true));
|
||||
|
||||
expect(result.current[0]).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("useDebouncedEffect", () => {
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.runOnlyPendingTimers();
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("runs the latest callback after the delay", () => {
|
||||
const calls: string[] = [];
|
||||
const { rerender } = renderHook(
|
||||
({ value }) => {
|
||||
useDebouncedEffect(
|
||||
() => {
|
||||
calls.push(value);
|
||||
},
|
||||
100,
|
||||
[value],
|
||||
);
|
||||
},
|
||||
{ initialProps: { value: "first" } },
|
||||
);
|
||||
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(99);
|
||||
});
|
||||
expect(calls).toEqual([]);
|
||||
|
||||
rerender({ value: "second" });
|
||||
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(100);
|
||||
});
|
||||
expect(calls).toEqual(["second"]);
|
||||
});
|
||||
|
||||
it("clears the pending timeout on unmount", () => {
|
||||
const calls: string[] = [];
|
||||
const { unmount } = renderHook(() => {
|
||||
useDebouncedEffect(
|
||||
() => {
|
||||
calls.push("called");
|
||||
},
|
||||
100,
|
||||
[],
|
||||
);
|
||||
});
|
||||
|
||||
unmount();
|
||||
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(100);
|
||||
});
|
||||
expect(calls).toEqual([]);
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run tests and verify they fail because hooks do not exist**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cd web
|
||||
pnpm test tests/hooks.test.tsx
|
||||
```
|
||||
|
||||
Expected: FAIL with missing exports for `useDebouncedEffect` and `useLocalStorage` from `@/hooks`.
|
||||
|
||||
- [ ] **Step 3: Commit failing tests**
|
||||
|
||||
```bash
|
||||
git add web/tests/hooks.test.tsx
|
||||
git commit -m "test: cover local frontend hooks"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 2: Implement Local Shared Hooks
|
||||
|
||||
**Files:**
|
||||
- Create: `web/src/hooks/useDebouncedEffect.ts`
|
||||
- Create: `web/src/hooks/useLocalStorage.ts`
|
||||
- Modify: `web/src/hooks/index.ts`
|
||||
- Test: `web/tests/hooks.test.tsx`
|
||||
|
||||
- [ ] **Step 1: Add `useDebouncedEffect`**
|
||||
|
||||
Create `web/src/hooks/useDebouncedEffect.ts`:
|
||||
|
||||
```ts
|
||||
import { type DependencyList, useEffect } from "react";
|
||||
|
||||
export const useDebouncedEffect = (effect: () => void | Promise<void>, delay: number, deps: DependencyList): void => {
|
||||
useEffect(() => {
|
||||
const timeout = window.setTimeout(() => {
|
||||
void effect();
|
||||
}, delay);
|
||||
|
||||
return () => {
|
||||
window.clearTimeout(timeout);
|
||||
};
|
||||
}, [delay, ...deps]);
|
||||
};
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Add `useLocalStorage`**
|
||||
|
||||
Create `web/src/hooks/useLocalStorage.ts`:
|
||||
|
||||
```ts
|
||||
import { useCallback, useEffect, useState } from "react";
|
||||
|
||||
type SetLocalStorageValue<T> = T | ((currentValue: T) => T);
|
||||
|
||||
const readLocalStorageValue = <T>(key: string, defaultValue: T): T => {
|
||||
if (typeof window === "undefined") {
|
||||
return defaultValue;
|
||||
}
|
||||
|
||||
try {
|
||||
const storedValue = window.localStorage.getItem(key);
|
||||
return storedValue === null ? defaultValue : (JSON.parse(storedValue) as T);
|
||||
} catch {
|
||||
return defaultValue;
|
||||
}
|
||||
};
|
||||
|
||||
export const useLocalStorage = <T>(key: string, defaultValue: T): [T, (value: SetLocalStorageValue<T>) => void] => {
|
||||
const [storedValue, setStoredValue] = useState<T>(() => readLocalStorageValue(key, defaultValue));
|
||||
|
||||
useEffect(() => {
|
||||
setStoredValue(readLocalStorageValue(key, defaultValue));
|
||||
}, [key, defaultValue]);
|
||||
|
||||
const setValue = useCallback(
|
||||
(value: SetLocalStorageValue<T>) => {
|
||||
setStoredValue((currentValue) => {
|
||||
const nextValue = typeof value === "function" ? (value as (currentValue: T) => T)(currentValue) : value;
|
||||
|
||||
if (typeof window !== "undefined") {
|
||||
try {
|
||||
window.localStorage.setItem(key, JSON.stringify(nextValue));
|
||||
} catch {
|
||||
// Keep React state updated even if persistence is unavailable.
|
||||
}
|
||||
}
|
||||
|
||||
return nextValue;
|
||||
});
|
||||
},
|
||||
[key],
|
||||
);
|
||||
|
||||
return [storedValue, setValue];
|
||||
};
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Export hooks from the barrel**
|
||||
|
||||
Modify `web/src/hooks/index.ts` to include:
|
||||
|
||||
```ts
|
||||
export * from "./useAsyncEffect";
|
||||
export * from "./useCurrentUser";
|
||||
export * from "./useDateFilterNavigation";
|
||||
export * from "./useDebouncedEffect";
|
||||
export * from "./useFilteredMemoStats";
|
||||
export * from "./useLoading";
|
||||
export * from "./useLocalStorage";
|
||||
export * from "./useMediaQuery";
|
||||
export * from "./useMemoFilters";
|
||||
export * from "./useMemoSorting";
|
||||
export * from "./useNavigateTo";
|
||||
export * from "./useUserLocale";
|
||||
export * from "./useUserTheme";
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run hook tests and verify they pass**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cd web
|
||||
pnpm test tests/hooks.test.tsx
|
||||
```
|
||||
|
||||
Expected: PASS for all `useLocalStorage` and `useDebouncedEffect` tests.
|
||||
|
||||
- [ ] **Step 5: Commit shared hooks**
|
||||
|
||||
```bash
|
||||
git add web/src/hooks/useDebouncedEffect.ts web/src/hooks/useLocalStorage.ts web/src/hooks/index.ts web/tests/hooks.test.tsx
|
||||
git commit -m "feat: add local frontend hooks"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 3: Replace Simple react-use Helpers With React Hooks
|
||||
|
||||
**Files:**
|
||||
- Modify: `web/src/components/TagTree.tsx`
|
||||
- Modify: `web/src/components/MobileHeader.tsx`
|
||||
- Modify: `web/src/layouts/RootLayout.tsx`
|
||||
|
||||
- [ ] **Step 1: Replace `useToggle` in `TagTree`**
|
||||
|
||||
In `web/src/components/TagTree.tsx`, change the imports:
|
||||
|
||||
```ts
|
||||
import { ChevronRightIcon, HashIcon } from "lucide-react";
|
||||
import { useCallback, useEffect, useState } from "react";
|
||||
import { type MemoFilter, useMemoFilterContext } from "@/contexts/MemoFilterContext";
|
||||
```
|
||||
|
||||
Replace the toggle state in `TagItemContainer` with:
|
||||
|
||||
```ts
|
||||
const [showSubTags, setShowSubTags] = useState(false);
|
||||
|
||||
useEffect(() => {
|
||||
setShowSubTags(expandSubTags);
|
||||
}, [expandSubTags]);
|
||||
```
|
||||
|
||||
Replace `handleToggleBtnClick` with:
|
||||
|
||||
```ts
|
||||
const handleToggleBtnClick = useCallback((event: React.MouseEvent) => {
|
||||
event.stopPropagation();
|
||||
setShowSubTags((current) => !current);
|
||||
}, []);
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Replace `useWindowScroll` in `MobileHeader`**
|
||||
|
||||
In `web/src/components/MobileHeader.tsx`, replace the first import with React hooks:
|
||||
|
||||
```ts
|
||||
import { useEffect, useState } from "react";
|
||||
import useMediaQuery from "@/hooks/useMediaQuery";
|
||||
import { cn } from "@/lib/utils";
|
||||
import NavigationDrawer from "./NavigationDrawer";
|
||||
```
|
||||
|
||||
Inside `MobileHeader`, replace `const { y: offsetTop } = useWindowScroll();` with:
|
||||
|
||||
```ts
|
||||
const [offsetTop, setOffsetTop] = useState(() => {
|
||||
if (typeof window === "undefined") return 0;
|
||||
return window.scrollY;
|
||||
});
|
||||
|
||||
useEffect(() => {
|
||||
const handleScroll = () => {
|
||||
setOffsetTop(window.scrollY);
|
||||
};
|
||||
|
||||
window.addEventListener("scroll", handleScroll, { passive: true });
|
||||
handleScroll();
|
||||
|
||||
return () => {
|
||||
window.removeEventListener("scroll", handleScroll);
|
||||
};
|
||||
}, []);
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Replace `usePrevious` in `RootLayout`**
|
||||
|
||||
In `web/src/layouts/RootLayout.tsx`, change the React import and remove the `react-use` import:
|
||||
|
||||
```ts
|
||||
import { useEffect, useRef } from "react";
|
||||
import { Outlet, useLocation, useSearchParams } from "react-router-dom";
|
||||
```
|
||||
|
||||
Replace:
|
||||
|
||||
```ts
|
||||
const prevPathname = usePrevious(pathname);
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```ts
|
||||
const prevPathnameRef = useRef<string | undefined>(undefined);
|
||||
```
|
||||
|
||||
Replace the route filter clearing effect with:
|
||||
|
||||
```ts
|
||||
useEffect(() => {
|
||||
const prevPathname = prevPathnameRef.current;
|
||||
|
||||
// When the route changes and there is no filter in the search params, remove all filters.
|
||||
if (prevPathname !== undefined && prevPathname !== pathname && !searchParams.has("filter")) {
|
||||
removeFilter(() => true);
|
||||
}
|
||||
|
||||
prevPathnameRef.current = pathname;
|
||||
}, [pathname, searchParams, removeFilter]);
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run TypeScript/lint check**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cd web
|
||||
pnpm lint
|
||||
```
|
||||
|
||||
Expected: PASS.
|
||||
|
||||
- [ ] **Step 5: Commit simple helper replacements**
|
||||
|
||||
```bash
|
||||
git add web/src/components/TagTree.tsx web/src/components/MobileHeader.tsx web/src/layouts/RootLayout.tsx
|
||||
git commit -m "refactor: replace simple react-use helpers"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 4: Replace Debounce And LocalStorage Call Sites
|
||||
|
||||
**Files:**
|
||||
- Modify: `web/src/components/MemoEditor/Toolbar/InsertMenu.tsx`
|
||||
- Modify: `web/src/components/MemoEditor/hooks/useLinkMemo.ts`
|
||||
- Modify: `web/src/components/MemoExplorer/TagsSection.tsx`
|
||||
- Test: `web/tests/hooks.test.tsx`
|
||||
|
||||
- [ ] **Step 1: Update `InsertMenu` debounce import and call**
|
||||
|
||||
In `web/src/components/MemoEditor/Toolbar/InsertMenu.tsx`, remove:
|
||||
|
||||
```ts
|
||||
import { useDebounce } from "react-use";
|
||||
```
|
||||
|
||||
Add:
|
||||
|
||||
```ts
|
||||
import { useDebouncedEffect } from "@/hooks";
|
||||
```
|
||||
|
||||
Replace:
|
||||
|
||||
```ts
|
||||
useDebounce(
|
||||
() => {
|
||||
setDebouncedPosition(locationState.position);
|
||||
},
|
||||
1000,
|
||||
[locationState.position],
|
||||
);
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```ts
|
||||
useDebouncedEffect(
|
||||
() => {
|
||||
setDebouncedPosition(locationState.position);
|
||||
},
|
||||
1000,
|
||||
[locationState.position],
|
||||
);
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Update `useLinkMemo` debounce import and call**
|
||||
|
||||
In `web/src/components/MemoEditor/hooks/useLinkMemo.ts`, remove:
|
||||
|
||||
```ts
|
||||
import useDebounce from "react-use/lib/useDebounce";
|
||||
```
|
||||
|
||||
Add:
|
||||
|
||||
```ts
|
||||
import { useDebouncedEffect } from "@/hooks";
|
||||
```
|
||||
|
||||
Replace:
|
||||
|
||||
```ts
|
||||
useDebounce(
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```ts
|
||||
useDebouncedEffect(
|
||||
```
|
||||
|
||||
Keep the existing async callback body, `300` delay, and `[isOpen, searchText]` dependency list unchanged.
|
||||
|
||||
- [ ] **Step 3: Update `TagsSection` localStorage import**
|
||||
|
||||
In `web/src/components/MemoExplorer/TagsSection.tsx`, replace:
|
||||
|
||||
```ts
|
||||
import useLocalStorage from "react-use/lib/useLocalStorage";
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```ts
|
||||
import { useLocalStorage } from "@/hooks";
|
||||
```
|
||||
|
||||
Keep both call sites unchanged:
|
||||
|
||||
```ts
|
||||
const [treeMode, setTreeMode] = useLocalStorage<boolean>("tag-view-as-tree", false);
|
||||
const [treeAutoExpand, setTreeAutoExpand] = useLocalStorage<boolean>("tag-tree-auto-expand", false);
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Verify no source imports from `react-use` remain**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
rg -n 'react-use' web/src web/package.json
|
||||
```
|
||||
|
||||
Expected: only `web/package.json` still reports `react-use` before the dependency removal task.
|
||||
|
||||
- [ ] **Step 5: Run targeted hook tests and lint**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cd web
|
||||
pnpm test tests/hooks.test.tsx
|
||||
pnpm lint
|
||||
```
|
||||
|
||||
Expected: both commands PASS.
|
||||
|
||||
- [ ] **Step 6: Commit call-site replacements**
|
||||
|
||||
```bash
|
||||
git add web/src/components/MemoEditor/Toolbar/InsertMenu.tsx web/src/components/MemoEditor/hooks/useLinkMemo.ts web/src/components/MemoExplorer/TagsSection.tsx web/tests/hooks.test.tsx
|
||||
git commit -m "refactor: use local debounce and storage hooks"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 5: Remove react-use Dependency And Regenerate Lockfile
|
||||
|
||||
**Files:**
|
||||
- Modify: `web/package.json`
|
||||
- Modify: `web/pnpm-lock.yaml`
|
||||
|
||||
- [ ] **Step 1: Remove `react-use` from `web/package.json`**
|
||||
|
||||
In `web/package.json`, remove this dependency line:
|
||||
|
||||
```json
|
||||
"react-use": "^17.6.0",
|
||||
```
|
||||
|
||||
Do not add `js-cookie` as a direct dependency.
|
||||
|
||||
- [ ] **Step 2: Regenerate the lockfile**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cd web
|
||||
pnpm install --lockfile-only
|
||||
```
|
||||
|
||||
Expected: command succeeds and updates `web/pnpm-lock.yaml`.
|
||||
|
||||
- [ ] **Step 3: Verify dependency graph removal**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cd web
|
||||
pnpm why react-use js-cookie
|
||||
```
|
||||
|
||||
Expected: output does not list installed versions for `react-use` or `js-cookie`.
|
||||
|
||||
- [ ] **Step 4: Verify repository text references**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
rg -n '"react-use"|react-use/lib|react-use@17\.6\.0|^\s+react-use:|js-cookie' web/package.json web/pnpm-lock.yaml web/src
|
||||
```
|
||||
|
||||
Expected: no matches.
|
||||
|
||||
- [ ] **Step 5: Commit dependency removal**
|
||||
|
||||
```bash
|
||||
git add web/package.json web/pnpm-lock.yaml
|
||||
git commit -m "chore: remove react-use dependency"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 6: Final Verification
|
||||
|
||||
**Files:**
|
||||
- Verify: all files changed by Tasks 1-5
|
||||
|
||||
- [ ] **Step 1: Run frontend tests for the new hook coverage**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cd web
|
||||
pnpm test tests/hooks.test.tsx
|
||||
```
|
||||
|
||||
Expected: PASS.
|
||||
|
||||
- [ ] **Step 2: Run frontend lint**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cd web
|
||||
pnpm lint
|
||||
```
|
||||
|
||||
Expected: PASS.
|
||||
|
||||
- [ ] **Step 3: Run frontend build**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cd web
|
||||
pnpm build
|
||||
```
|
||||
|
||||
Expected: PASS.
|
||||
|
||||
- [ ] **Step 4: Confirm no removed dependency remains**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cd web
|
||||
pnpm why react-use js-cookie
|
||||
rg -n '"react-use"|react-use/lib|react-use@17\.6\.0|^\s+react-use:|js-cookie' src package.json pnpm-lock.yaml
|
||||
```
|
||||
|
||||
Expected: no `react-use` or `js-cookie` dependency remains. The `rg` command returns no matches.
|
||||
|
||||
- [ ] **Step 5: Inspect final diff**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
git diff --stat HEAD~5..HEAD
|
||||
git status --short
|
||||
```
|
||||
|
||||
Expected: diff contains only hook tests, local hooks, six call-site rewrites, and dependency files. Working tree is clean after all task commits.
|
||||
File diff suppressed because it is too large
Load Diff
File diff suppressed because it is too large
Load Diff
@@ -0,0 +1,887 @@
|
||||
# CEL Filter Surface Expansion — 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:** Let users write three more CEL constructs in the `filter` field — scalar `startsWith()`/`endsWith()` (case-insensitive), `matches(regex)`, and `all()` over tags (non-empty) — each compiled to SQL across SQLite/MySQL/Postgres.
|
||||
|
||||
**Architecture:** The `internal/filter` engine parses CEL with `cel-go`, walks the AST into a dialect-agnostic IR (`ir.go`), and renders dialect SQL (`render.go`). cel-go never evaluates — every feature must become a SQL `WHERE` fragment. We add IR nodes + parser recognition + per-dialect rendering, and register a Go-backed `REGEXP` function for SQLite (which has no built-in one).
|
||||
|
||||
**Tech Stack:** Go, `github.com/google/cel-go v0.28.0`, `modernc.org/sqlite` (pure-Go), `github.com/stretchr/testify/require`.
|
||||
|
||||
**Spec:** `docs/superpowers/specs/2026-06-15-cel-filter-surface-expansion-design.md`
|
||||
|
||||
---
|
||||
|
||||
## File structure
|
||||
|
||||
| File | Change | Responsibility |
|
||||
|------|--------|----------------|
|
||||
| `internal/filter/ir.go` | Modify | Replace `ContainsCondition` with `TextMatchCondition`; add `RegexCondition`; add `ComprehensionAll` kind |
|
||||
| `internal/filter/parser.go` | Modify | Recognize top-level `contains`/`startsWith`/`endsWith`/`matches`; accept `all()` comprehension |
|
||||
| `internal/filter/render.go` | Modify | Render text-match (LIKE), regex, and `all()` per-element subqueries; shared `foldedLike`/`likePattern`/`escapeLikeLiteral` helpers |
|
||||
| `internal/filter/schema.go` | Modify | Add `cel.ValidateRegexLiterals()` validator; enable text matching on attachment `mime_type` |
|
||||
| `internal/filter/engine_test.go` | Modify | Compile-level accept/reject unit tests |
|
||||
| `store/db/sqlite/functions.go` | Modify | Register a Go-backed `regexp(pattern, value)` scalar function with a compiled-pattern cache |
|
||||
| `store/db/sqlite/sqlite.go` | Modify | Call `ensureRegexpRegistered()` in `NewDB` |
|
||||
| `store/test/memo_filter_test.go` | Modify | Behavioral tests for the new memo filters |
|
||||
| `store/test/attachment_filter_test.go` | Modify | Behavioral tests for `filename`/`mime_type` |
|
||||
| `internal/filter/README.md` | Modify | Document new syntax + regex cross-dialect caveat |
|
||||
|
||||
**Key design choices locked in:**
|
||||
- The existing `Field.SupportsContains` flag is **reused** as the gate for *all* text-matching ops (`contains`/`startsWith`/`endsWith`/`matches`) — no rename, lower risk. We just enable it on `mime_type`.
|
||||
- New scalar `startsWith`/`endsWith`/`contains` are **case-insensitive** (reuse the existing `memos_unicode_lower` / `ILIKE` machinery). `matches()` and `==` are case-sensitive.
|
||||
- `all()` over a memo with zero tags does **not** match (non-empty guard).
|
||||
- LIKE patterns escape `%` `_` `\`; only SQLite needs an explicit `ESCAPE '\'` clause (Postgres/MySQL default the escape char to backslash, and patterns are passed as bound parameters so no SQL-literal backslash hazard).
|
||||
|
||||
**Suggested task order** lands the two cheap features (text-match refactor, scalar prefix/suffix, regex) before the heavy `all()` work, giving a natural stop point. `all()` (Task 5) is the largest piece and could be deferred to a follow-up if needed.
|
||||
|
||||
---
|
||||
|
||||
### Task 1: Refactor `ContainsCondition` → `TextMatchCondition` (+ LIKE escaping)
|
||||
|
||||
Foundation refactor. No new user-facing behavior except that LIKE metacharacters in `contains()` values are now treated literally. Existing tests must stay green.
|
||||
|
||||
**Files:**
|
||||
- Modify: `internal/filter/ir.go` (replace `ContainsCondition`)
|
||||
- Modify: `internal/filter/parser.go` (`buildContainsCondition` → shared builder)
|
||||
- Modify: `internal/filter/render.go` (`renderContainsCondition` → `renderTextMatch` + helpers)
|
||||
|
||||
- [ ] **Step 1: Add a failing escaping test** in `internal/filter/engine_test.go`
|
||||
|
||||
```go
|
||||
func TestCompileContainsEscapesLikeWildcards(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
engine, err := NewEngine(NewSchema())
|
||||
require.NoError(t, err)
|
||||
|
||||
stmt, err := engine.CompileToStatement(context.Background(), `content.contains("50%_off")`, RenderOptions{Dialect: DialectSQLite})
|
||||
require.NoError(t, err)
|
||||
// The % and _ in the value must be escaped so they are matched literally,
|
||||
// and SQLite needs an explicit ESCAPE clause.
|
||||
require.Contains(t, stmt.SQL, `ESCAPE '\'`)
|
||||
require.Equal(t, []any{`%50\%\_off%`}, stmt.Args)
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run it to verify it fails**
|
||||
|
||||
Run: `go test ./internal/filter/ -run TestCompileContainsEscapesLikeWildcards -v`
|
||||
Expected: FAIL (current renderer emits `%50%_off%` with no `ESCAPE`).
|
||||
|
||||
- [ ] **Step 3: Replace `ContainsCondition` in `internal/filter/ir.go`**
|
||||
|
||||
Delete the `ContainsCondition` struct + its `isCondition()` (lines ~76-82) and add:
|
||||
|
||||
```go
|
||||
// TextMatchMode enumerates LIKE-based string match modes.
|
||||
type TextMatchMode string
|
||||
|
||||
const (
|
||||
TextMatchContains TextMatchMode = "contains"
|
||||
TextMatchPrefix TextMatchMode = "prefix"
|
||||
TextMatchSuffix TextMatchMode = "suffix"
|
||||
)
|
||||
|
||||
// TextMatchCondition models a case-insensitive LIKE match on a scalar string field
|
||||
// (content.contains/startsWith/endsWith).
|
||||
type TextMatchCondition struct {
|
||||
Field string
|
||||
Mode TextMatchMode
|
||||
Value string
|
||||
}
|
||||
|
||||
func (*TextMatchCondition) isCondition() {}
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Update the parser in `internal/filter/parser.go`**
|
||||
|
||||
In `buildCallCondition`, replace the `case "contains":` line with:
|
||||
|
||||
```go
|
||||
case "contains":
|
||||
return buildTextMatchCondition(call, schema, TextMatchContains)
|
||||
```
|
||||
|
||||
Delete `buildContainsCondition` (lines ~196-227) and add:
|
||||
|
||||
```go
|
||||
func buildTextMatchCondition(call *exprv1.Expr_Call, schema Schema, mode TextMatchMode) (Condition, error) {
|
||||
if call.Target == nil {
|
||||
return nil, errors.New("text match requires a target")
|
||||
}
|
||||
targetName, err := getIdentName(call.Target)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
field, ok := schema.Field(targetName)
|
||||
if !ok {
|
||||
return nil, errors.Errorf("unknown identifier %q", targetName)
|
||||
}
|
||||
if !field.SupportsContains {
|
||||
return nil, errors.Errorf("identifier %q does not support text matching", targetName)
|
||||
}
|
||||
if len(call.Args) != 1 {
|
||||
return nil, errors.New("text match expects exactly one argument")
|
||||
}
|
||||
value, err := getConstValue(call.Args[0])
|
||||
if err != nil {
|
||||
return nil, errors.Wrap(err, "text match only supports literal arguments")
|
||||
}
|
||||
str, ok := value.(string)
|
||||
if !ok {
|
||||
return nil, errors.New("text match argument must be a string")
|
||||
}
|
||||
return &TextMatchCondition{Field: targetName, Mode: mode, Value: str}, nil
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Update the renderer in `internal/filter/render.go`**
|
||||
|
||||
In `renderCondition`, replace `case *ContainsCondition:` / `return r.renderContainsCondition(c)` with:
|
||||
|
||||
```go
|
||||
case *TextMatchCondition:
|
||||
return r.renderTextMatch(c)
|
||||
```
|
||||
|
||||
Delete `renderContainsCondition` (lines ~449-469) and add:
|
||||
|
||||
```go
|
||||
func (r *renderer) renderTextMatch(cond *TextMatchCondition) (renderResult, error) {
|
||||
field, ok := r.schema.Field(cond.Field)
|
||||
if !ok {
|
||||
return renderResult{}, errors.Errorf("unknown field %q", cond.Field)
|
||||
}
|
||||
column := field.columnExpr(r.dialect)
|
||||
pattern := likePattern(cond.Mode, cond.Value)
|
||||
return renderResult{sql: r.foldedLike(column, pattern)}, nil
|
||||
}
|
||||
|
||||
// foldedLike renders a case-insensitive LIKE comparison of colExpr against a
|
||||
// (already metacharacter-escaped) pattern, using each dialect's case-folding.
|
||||
func (r *renderer) foldedLike(colExpr, pattern string) string {
|
||||
switch r.dialect {
|
||||
case DialectSQLite:
|
||||
// memos_unicode_lower gives Unicode-aware folding; ESCAPE '\' is required
|
||||
// because SQLite has no default LIKE escape character.
|
||||
return fmt.Sprintf(`memos_unicode_lower(%s) LIKE memos_unicode_lower(%s) ESCAPE '\'`, colExpr, r.addArg(pattern))
|
||||
case DialectPostgres:
|
||||
// ILIKE is case-insensitive; backslash is the default escape character.
|
||||
return fmt.Sprintf("%s ILIKE %s", colExpr, r.addArg(pattern))
|
||||
default: // MySQL: default collation is case-insensitive; backslash is the default escape.
|
||||
return fmt.Sprintf("%s LIKE %s", colExpr, r.addArg(pattern))
|
||||
}
|
||||
}
|
||||
|
||||
// likePattern escapes LIKE metacharacters in value and wraps it for the mode.
|
||||
func likePattern(mode TextMatchMode, value string) string {
|
||||
escaped := escapeLikeLiteral(value)
|
||||
switch mode {
|
||||
case TextMatchPrefix:
|
||||
return escaped + "%"
|
||||
case TextMatchSuffix:
|
||||
return "%" + escaped
|
||||
default:
|
||||
return "%" + escaped + "%"
|
||||
}
|
||||
}
|
||||
|
||||
// escapeLikeLiteral escapes the LIKE metacharacters \, %, and _ so user input
|
||||
// is matched literally. Backslash is the escape character on all three dialects.
|
||||
func escapeLikeLiteral(s string) string {
|
||||
return strings.NewReplacer(`\`, `\\`, `%`, `\%`, `_`, `\_`).Replace(s)
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 6: Run the new test + existing suites to verify green**
|
||||
|
||||
Run: `go test ./internal/filter/ -v`
|
||||
Expected: PASS (including `TestCompileContainsEscapesLikeWildcards`).
|
||||
|
||||
Run: `go test ./store/test/ -run TestMemoFilterContent -v`
|
||||
Expected: PASS (existing `contains` behavioral tests, including special-characters/unicode, still pass).
|
||||
|
||||
- [ ] **Step 7: Commit**
|
||||
|
||||
```bash
|
||||
git add internal/filter/ir.go internal/filter/parser.go internal/filter/render.go internal/filter/engine_test.go
|
||||
git commit -m "refactor(filter): unify string matching into TextMatchCondition with LIKE escaping
|
||||
|
||||
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 2: Scalar `startsWith()` / `endsWith()`
|
||||
|
||||
Wire the new prefix/suffix modes through the parser and enable text matching on attachment `mime_type`. Rendering already exists from Task 1.
|
||||
|
||||
**Files:**
|
||||
- Modify: `internal/filter/parser.go` (add `startsWith`/`endsWith` cases)
|
||||
- Modify: `internal/filter/schema.go` (enable `SupportsContains` on `mime_type`)
|
||||
- Modify: `store/test/memo_filter_test.go`, `store/test/attachment_filter_test.go` (behavioral tests)
|
||||
- Modify: `internal/filter/engine_test.go` (reject on unsupported field)
|
||||
|
||||
- [ ] **Step 1: Add failing behavioral tests** in `store/test/memo_filter_test.go`
|
||||
|
||||
```go
|
||||
func TestMemoFilterContentStartsWith(t *testing.T) {
|
||||
t.Parallel()
|
||||
tc := NewMemoFilterTestContext(t)
|
||||
defer tc.Close()
|
||||
|
||||
tc.CreateMemo(NewMemoBuilder("memo-todo", tc.User.ID).Content("TODO: buy milk"))
|
||||
tc.CreateMemo(NewMemoBuilder("memo-done", tc.User.ID).Content("Done with milk"))
|
||||
|
||||
// Prefix match, case-insensitive (consistent with contains()).
|
||||
memos := tc.ListWithFilter(`content.startsWith("todo")`)
|
||||
require.Len(t, memos, 1)
|
||||
require.Equal(t, "memo-todo", memos[0].UID)
|
||||
|
||||
memos = tc.ListWithFilter(`content.startsWith("nope")`)
|
||||
require.Len(t, memos, 0)
|
||||
}
|
||||
|
||||
func TestMemoFilterContentEndsWith(t *testing.T) {
|
||||
t.Parallel()
|
||||
tc := NewMemoFilterTestContext(t)
|
||||
defer tc.Close()
|
||||
|
||||
tc.CreateMemo(NewMemoBuilder("memo-md", tc.User.ID).Content("notes.md"))
|
||||
tc.CreateMemo(NewMemoBuilder("memo-txt", tc.User.ID).Content("notes.txt"))
|
||||
|
||||
memos := tc.ListWithFilter(`content.endsWith(".md")`)
|
||||
require.Len(t, memos, 1)
|
||||
require.Equal(t, "memo-md", memos[0].UID)
|
||||
}
|
||||
```
|
||||
|
||||
And in `store/test/attachment_filter_test.go`:
|
||||
|
||||
```go
|
||||
func TestAttachmentFilterFilenameStartsWith(t *testing.T) {
|
||||
t.Parallel()
|
||||
tc := NewAttachmentFilterTestContextWithUser(t)
|
||||
defer tc.Close()
|
||||
|
||||
tc.CreateAttachment(NewAttachmentBuilder(tc.CreatorID).Filename("invoice-2026.pdf").MimeType("application/pdf"))
|
||||
tc.CreateAttachment(NewAttachmentBuilder(tc.CreatorID).Filename("photo.png").MimeType("image/png"))
|
||||
|
||||
got := tc.ListWithFilter(`filename.startsWith("invoice")`)
|
||||
require.Len(t, got, 1)
|
||||
require.Equal(t, "invoice-2026.pdf", got[0].Filename)
|
||||
|
||||
// mime_type prefix matching (newly enabled).
|
||||
got = tc.ListWithFilter(`mime_type.startsWith("image/")`)
|
||||
require.Len(t, got, 1)
|
||||
require.Equal(t, "photo.png", got[0].Filename)
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run to verify they fail**
|
||||
|
||||
Run: `go test ./store/test/ -run 'TestMemoFilterContentStartsWith|TestMemoFilterContentEndsWith|TestAttachmentFilterFilenameStartsWith' -v`
|
||||
Expected: FAIL — `startsWith` hits `buildCallCondition`'s default branch ("unsupported call expression"), and `mime_type` is not yet text-matchable.
|
||||
|
||||
- [ ] **Step 3: Add parser cases** in `internal/filter/parser.go` `buildCallCondition`
|
||||
|
||||
Immediately after the `case "contains":` line, add:
|
||||
|
||||
```go
|
||||
case "startsWith":
|
||||
return buildTextMatchCondition(call, schema, TextMatchPrefix)
|
||||
case "endsWith":
|
||||
return buildTextMatchCondition(call, schema, TextMatchSuffix)
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Enable text matching on `mime_type`** in `internal/filter/schema.go`
|
||||
|
||||
In `NewAttachmentSchema`, add `SupportsContains: true` to the `mime_type` field entry:
|
||||
|
||||
```go
|
||||
"mime_type": {
|
||||
Name: "mime_type",
|
||||
Kind: FieldKindScalar,
|
||||
Type: FieldTypeString,
|
||||
Column: Column{Table: "attachment", Name: "type"},
|
||||
SupportsContains: true,
|
||||
Expressions: map[DialectName]string{},
|
||||
},
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Add a compile-reject unit test** in `internal/filter/engine_test.go`
|
||||
|
||||
```go
|
||||
func TestCompileRejectsStartsWithOnUnsupportedField(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
engine, err := NewEngine(NewSchema())
|
||||
require.NoError(t, err)
|
||||
|
||||
_, err = engine.Compile(context.Background(), `visibility.startsWith("P")`)
|
||||
require.Error(t, err)
|
||||
require.Contains(t, err.Error(), "does not support text matching")
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 6: Run tests to verify green**
|
||||
|
||||
Run: `go test ./internal/filter/ ./store/test/ -run 'StartsWith|EndsWith|TextMatch' -v`
|
||||
Expected: PASS.
|
||||
|
||||
- [ ] **Step 7: Commit**
|
||||
|
||||
```bash
|
||||
git add internal/filter/parser.go internal/filter/schema.go internal/filter/engine_test.go store/test/memo_filter_test.go store/test/attachment_filter_test.go
|
||||
git commit -m "feat(filter): support startsWith()/endsWith() on scalar string fields
|
||||
|
||||
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 3: Register a SQLite `REGEXP` function
|
||||
|
||||
`modernc.org/sqlite` has no built-in `REGEXP`. SQLite desugars `X REGEXP Y` to `regexp(Y, X)`, so register a 2-arg `regexp(pattern, value)` scalar function backed by Go's `regexp`, mirroring `ensureUnicodeLowerRegistered`.
|
||||
|
||||
**Files:**
|
||||
- Modify: `store/db/sqlite/functions.go`
|
||||
- Modify: `store/db/sqlite/sqlite.go`
|
||||
- Test: `store/db/sqlite/functions_test.go` (create)
|
||||
|
||||
- [ ] **Step 1: Write a failing test** — create `store/db/sqlite/functions_test.go`
|
||||
|
||||
```go
|
||||
package sqlite
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func TestRegexpFunctionMatches(t *testing.T) {
|
||||
require.NoError(t, ensureRegexpRegistered())
|
||||
|
||||
re, err := compileRegexp(`^v\d+$`)
|
||||
require.NoError(t, err)
|
||||
require.True(t, re.MatchString("v12"))
|
||||
require.False(t, re.MatchString("version"))
|
||||
|
||||
// Caching returns the same compiled instance.
|
||||
re2, err := compileRegexp(`^v\d+$`)
|
||||
require.NoError(t, err)
|
||||
require.Same(t, re, re2)
|
||||
|
||||
_, err = compileRegexp(`(`)
|
||||
require.Error(t, err)
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run to verify it fails**
|
||||
|
||||
Run: `go test ./store/db/sqlite/ -run TestRegexpFunctionMatches -v`
|
||||
Expected: FAIL — `ensureRegexpRegistered`/`compileRegexp` undefined.
|
||||
|
||||
- [ ] **Step 3: Implement in `store/db/sqlite/functions.go`**
|
||||
|
||||
Add `"errors"` and `"regexp"` to the imports, then append:
|
||||
|
||||
```go
|
||||
var (
|
||||
registerRegexpOnce sync.Once
|
||||
registerRegexpErr error
|
||||
// regexpCache memoizes compiled patterns; keys are pattern strings.
|
||||
regexpCache sync.Map
|
||||
)
|
||||
|
||||
// ensureRegexpRegistered registers a Go-backed `regexp(pattern, value)` scalar
|
||||
// function so SQLite's `value REGEXP pattern` operator works (modernc.org/sqlite
|
||||
// has no built-in implementation). Patterns use Go's RE2 syntax. Registered once
|
||||
// globally; safe to call multiple times.
|
||||
func ensureRegexpRegistered() error {
|
||||
registerRegexpOnce.Do(func() {
|
||||
registerRegexpErr = msqlite.RegisterScalarFunction("regexp", 2, func(_ *msqlite.FunctionContext, args []driver.Value) (driver.Value, error) {
|
||||
if len(args) != 2 || args[0] == nil || args[1] == nil {
|
||||
return int64(0), nil
|
||||
}
|
||||
pattern, ok := args[0].(string)
|
||||
if !ok {
|
||||
return nil, errors.New("regexp pattern must be a string")
|
||||
}
|
||||
var value string
|
||||
switch v := args[1].(type) {
|
||||
case string:
|
||||
value = v
|
||||
case []byte:
|
||||
value = string(v)
|
||||
default:
|
||||
return int64(0), nil
|
||||
}
|
||||
re, err := compileRegexp(pattern)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if re.MatchString(value) {
|
||||
return int64(1), nil
|
||||
}
|
||||
return int64(0), nil
|
||||
})
|
||||
})
|
||||
return registerRegexpErr
|
||||
}
|
||||
|
||||
// compileRegexp compiles and caches a RE2 pattern.
|
||||
func compileRegexp(pattern string) (*regexp.Regexp, error) {
|
||||
if cached, ok := regexpCache.Load(pattern); ok {
|
||||
return cached.(*regexp.Regexp), nil
|
||||
}
|
||||
re, err := regexp.Compile(pattern)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
regexpCache.Store(pattern, re)
|
||||
return re, nil
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Wire into `NewDB`** in `store/db/sqlite/sqlite.go`
|
||||
|
||||
Right after the `ensureUnicodeLowerRegistered()` block, add:
|
||||
|
||||
```go
|
||||
if err := ensureRegexpRegistered(); err != nil {
|
||||
return nil, errors.Wrap(err, "failed to register sqlite regexp function")
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Run to verify green**
|
||||
|
||||
Run: `go test ./store/db/sqlite/ -run TestRegexpFunctionMatches -v`
|
||||
Expected: PASS.
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add store/db/sqlite/functions.go store/db/sqlite/sqlite.go store/db/sqlite/functions_test.go
|
||||
git commit -m "feat(sqlite): register Go-backed REGEXP function
|
||||
|
||||
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 4: `matches(regex)` on string fields
|
||||
|
||||
Add the IR node, parser recognition, per-dialect rendering, and the compile-time regex validator.
|
||||
|
||||
**Files:**
|
||||
- Modify: `internal/filter/ir.go` (add `RegexCondition`)
|
||||
- Modify: `internal/filter/parser.go` (`matches` case + builder)
|
||||
- Modify: `internal/filter/render.go` (`renderRegex`)
|
||||
- Modify: `internal/filter/schema.go` (add `cel.ValidateRegexLiterals()` to both schemas)
|
||||
- Modify: `internal/filter/engine_test.go`, `store/test/memo_filter_test.go`
|
||||
|
||||
- [ ] **Step 1: Add failing tests** — compile-level in `internal/filter/engine_test.go`:
|
||||
|
||||
```go
|
||||
func TestCompileRejectsMalformedRegex(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
engine, err := NewEngine(NewSchema())
|
||||
require.NoError(t, err)
|
||||
|
||||
_, err = engine.Compile(context.Background(), `content.matches("(")`)
|
||||
require.Error(t, err)
|
||||
}
|
||||
|
||||
func TestCompileMatchesRendersRegexOperator(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
engine, err := NewEngine(NewSchema())
|
||||
require.NoError(t, err)
|
||||
|
||||
stmt, err := engine.CompileToStatement(context.Background(), `content.matches("v[0-9]+")`, RenderOptions{Dialect: DialectPostgres})
|
||||
require.NoError(t, err)
|
||||
require.Contains(t, stmt.SQL, "~")
|
||||
require.Equal(t, []any{"v[0-9]+"}, stmt.Args)
|
||||
}
|
||||
```
|
||||
|
||||
And behavioral in `store/test/memo_filter_test.go` (runs against SQLite by default, exercising the registered `REGEXP` function):
|
||||
|
||||
```go
|
||||
func TestMemoFilterContentMatches(t *testing.T) {
|
||||
t.Parallel()
|
||||
tc := NewMemoFilterTestContext(t)
|
||||
defer tc.Close()
|
||||
|
||||
tc.CreateMemo(NewMemoBuilder("memo-v1", tc.User.ID).Content("release v12 shipped"))
|
||||
tc.CreateMemo(NewMemoBuilder("memo-plain", tc.User.ID).Content("no version here"))
|
||||
|
||||
memos := tc.ListWithFilter(`content.matches("v[0-9]+")`)
|
||||
require.Len(t, memos, 1)
|
||||
require.Equal(t, "memo-v1", memos[0].UID)
|
||||
|
||||
memos = tc.ListWithFilter(`content.matches("^xyz")`)
|
||||
require.Len(t, memos, 0)
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run to verify they fail**
|
||||
|
||||
Run: `go test ./internal/filter/ -run 'Malformed|MatchesRenders' -v && go test ./store/test/ -run TestMemoFilterContentMatches -v`
|
||||
Expected: FAIL — `matches` is unhandled and no regex validator is configured.
|
||||
|
||||
- [ ] **Step 3: Add the IR node** in `internal/filter/ir.go`
|
||||
|
||||
```go
|
||||
// RegexCondition models field.matches("pattern") on a string field.
|
||||
type RegexCondition struct {
|
||||
Field string
|
||||
Pattern string
|
||||
}
|
||||
|
||||
func (*RegexCondition) isCondition() {}
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Add parser support** in `internal/filter/parser.go`
|
||||
|
||||
In `buildCallCondition`, after the `case "endsWith":` block, add:
|
||||
|
||||
```go
|
||||
case "matches":
|
||||
return buildMatchesCondition(call, schema)
|
||||
```
|
||||
|
||||
Then add the builder:
|
||||
|
||||
```go
|
||||
func buildMatchesCondition(call *exprv1.Expr_Call, schema Schema) (Condition, error) {
|
||||
if call.Target == nil {
|
||||
return nil, errors.New("matches requires a target")
|
||||
}
|
||||
targetName, err := getIdentName(call.Target)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
field, ok := schema.Field(targetName)
|
||||
if !ok {
|
||||
return nil, errors.Errorf("unknown identifier %q", targetName)
|
||||
}
|
||||
if !field.SupportsContains {
|
||||
return nil, errors.Errorf("identifier %q does not support matches()", targetName)
|
||||
}
|
||||
if len(call.Args) != 1 {
|
||||
return nil, errors.New("matches expects exactly one argument")
|
||||
}
|
||||
value, err := getConstValue(call.Args[0])
|
||||
if err != nil {
|
||||
return nil, errors.Wrap(err, "matches only supports literal arguments")
|
||||
}
|
||||
pattern, ok := value.(string)
|
||||
if !ok {
|
||||
return nil, errors.New("matches argument must be a string")
|
||||
}
|
||||
return &RegexCondition{Field: targetName, Pattern: pattern}, nil
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Add the renderer** in `internal/filter/render.go`
|
||||
|
||||
In `renderCondition`, after the `case *TextMatchCondition:` arm, add:
|
||||
|
||||
```go
|
||||
case *RegexCondition:
|
||||
return r.renderRegex(c)
|
||||
```
|
||||
|
||||
Then add:
|
||||
|
||||
```go
|
||||
func (r *renderer) renderRegex(cond *RegexCondition) (renderResult, error) {
|
||||
field, ok := r.schema.Field(cond.Field)
|
||||
if !ok {
|
||||
return renderResult{}, errors.Errorf("unknown field %q", cond.Field)
|
||||
}
|
||||
column := field.columnExpr(r.dialect)
|
||||
switch r.dialect {
|
||||
case DialectPostgres:
|
||||
// POSIX regex match operator.
|
||||
return renderResult{sql: fmt.Sprintf("%s ~ %s", column, r.addArg(cond.Pattern))}, nil
|
||||
case DialectMySQL, DialectSQLite:
|
||||
// MySQL has a native REGEXP operator; SQLite uses the registered regexp() function.
|
||||
return renderResult{sql: fmt.Sprintf("%s REGEXP %s", column, r.addArg(cond.Pattern))}, nil
|
||||
default:
|
||||
return renderResult{}, errors.Errorf("unsupported dialect %s", r.dialect)
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 6: Add the regex validator** in `internal/filter/schema.go`
|
||||
|
||||
Add the `cel` import line already present. In **both** `NewSchema` and `NewAttachmentSchema`, append the validator to the `envOptions` slice (e.g. after `nowFunction`):
|
||||
|
||||
```go
|
||||
cel.ASTValidators(cel.ValidateRegexLiterals()),
|
||||
```
|
||||
|
||||
- [ ] **Step 7: Run tests to verify green**
|
||||
|
||||
Run: `go test ./internal/filter/ -run 'Malformed|MatchesRenders' -v && go test ./store/test/ -run TestMemoFilterContentMatches -v`
|
||||
Expected: PASS.
|
||||
|
||||
- [ ] **Step 8: Commit**
|
||||
|
||||
```bash
|
||||
git add internal/filter/ir.go internal/filter/parser.go internal/filter/render.go internal/filter/schema.go internal/filter/engine_test.go store/test/memo_filter_test.go
|
||||
git commit -m "feat(filter): support matches() regex on string fields
|
||||
|
||||
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 5: `all()` comprehension on tags (non-empty)
|
||||
|
||||
The heaviest task. `exists()` matches against the *serialized* JSON array and cannot express "every element matches", so `all()` needs real per-element iteration via `json_each` / `jsonb_array_elements_text` / `JSON_TABLE`, plus a non-empty guard.
|
||||
|
||||
**Files:**
|
||||
- Modify: `internal/filter/ir.go` (add `ComprehensionAll`)
|
||||
- Modify: `internal/filter/parser.go` (accept `all()` in `detectComprehensionKind`)
|
||||
- Modify: `internal/filter/render.go` (`renderTagAll` + element predicate SQL; branch in `renderListComprehension`)
|
||||
- Modify: `store/test/memo_filter_test.go`
|
||||
|
||||
- [ ] **Step 1: Add failing behavioral tests** in `store/test/memo_filter_test.go`
|
||||
|
||||
```go
|
||||
func TestMemoFilterTagsAll(t *testing.T) {
|
||||
t.Parallel()
|
||||
tc := NewMemoFilterTestContext(t)
|
||||
defer tc.Close()
|
||||
|
||||
tc.CreateMemo(NewMemoBuilder("memo-all-work", tc.User.ID).Content("all work").Tags("work/a", "work/b"))
|
||||
tc.CreateMemo(NewMemoBuilder("memo-mixed", tc.User.ID).Content("mixed").Tags("work/a", "home"))
|
||||
tc.CreateMemo(NewMemoBuilder("memo-untagged", tc.User.ID).Content("untagged"))
|
||||
|
||||
// Every tag starts with "work/": only the all-work memo qualifies.
|
||||
memos := tc.ListWithFilter(`tags.all(t, t.startsWith("work/"))`)
|
||||
require.Len(t, memos, 1)
|
||||
require.Equal(t, "memo-all-work", memos[0].UID)
|
||||
|
||||
// Untagged memos must NOT match (non-empty guard, decision B).
|
||||
require.NotContains(t, uids(memos), "memo-untagged")
|
||||
}
|
||||
|
||||
func TestMemoFilterTagsAllEquals(t *testing.T) {
|
||||
t.Parallel()
|
||||
tc := NewMemoFilterTestContext(t)
|
||||
defer tc.Close()
|
||||
|
||||
tc.CreateMemo(NewMemoBuilder("memo-only-x", tc.User.ID).Content("only x").Tags("x", "x"))
|
||||
tc.CreateMemo(NewMemoBuilder("memo-x-and-y", tc.User.ID).Content("x and y").Tags("x", "y"))
|
||||
|
||||
memos := tc.ListWithFilter(`tags.all(t, t == "x")`)
|
||||
require.Len(t, memos, 1)
|
||||
require.Equal(t, "memo-only-x", memos[0].UID)
|
||||
}
|
||||
```
|
||||
|
||||
Add this helper near the top of `store/test/memo_filter_test.go` (after the imports) if not already present:
|
||||
|
||||
```go
|
||||
func uids(memos []*store.Memo) []string {
|
||||
out := make([]string, 0, len(memos))
|
||||
for _, m := range memos {
|
||||
out = append(out, m.UID)
|
||||
}
|
||||
return out
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run to verify they fail**
|
||||
|
||||
Run: `go test ./store/test/ -run 'TestMemoFilterTagsAll' -v`
|
||||
Expected: FAIL — `detectComprehensionKind` returns "all() comprehension is not supported".
|
||||
|
||||
- [ ] **Step 3: Add the IR kind** in `internal/filter/ir.go`
|
||||
|
||||
In the `ComprehensionKind` const block, add `ComprehensionAll`:
|
||||
|
||||
```go
|
||||
const (
|
||||
ComprehensionExists ComprehensionKind = "exists"
|
||||
ComprehensionAll ComprehensionKind = "all"
|
||||
)
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Accept `all()` in the parser** in `internal/filter/parser.go`
|
||||
|
||||
In `detectComprehensionKind`, replace the `all()` rejection block:
|
||||
|
||||
```go
|
||||
// all() starts with true and uses AND (&&) - not supported
|
||||
if accuInit.GetBoolValue() {
|
||||
if step := comp.LoopStep.GetCallExpr(); step != nil && step.Function == "_&&_" {
|
||||
return "", errors.New("all() comprehension is not supported; use exists() instead")
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```go
|
||||
// all() starts with true and uses AND (&&) in the loop step.
|
||||
if accuInit.GetBoolValue() {
|
||||
if step := comp.LoopStep.GetCallExpr(); step != nil && step.Function == "_&&_" {
|
||||
return ComprehensionAll, nil
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Branch and render in `internal/filter/render.go`**
|
||||
|
||||
At the top of `renderListComprehension`, right after the `field.Kind != FieldKindJSONList` guard, add:
|
||||
|
||||
```go
|
||||
if cond.Kind == ComprehensionAll {
|
||||
return r.renderTagAll(field, cond.Predicate)
|
||||
}
|
||||
```
|
||||
|
||||
Then add the new render path + element-predicate helper:
|
||||
|
||||
```go
|
||||
// renderTagAll renders tags.all(t, <pred>): the array is non-empty AND no element
|
||||
// fails the predicate. Element predicates use plain CEL semantics (case-insensitive
|
||||
// for startsWith/endsWith/contains, case-sensitive for ==), evaluated per element.
|
||||
func (r *renderer) renderTagAll(field Field, pred PredicateExpr) (renderResult, error) {
|
||||
arrayExpr := jsonArrayExpr(r.dialect, field)
|
||||
elemCond, err := r.elementPredicateSQL(pred)
|
||||
if err != nil {
|
||||
return renderResult{}, err
|
||||
}
|
||||
switch r.dialect {
|
||||
case DialectSQLite:
|
||||
nonEmpty := fmt.Sprintf("%s IS NOT NULL AND %s != '[]'", arrayExpr, arrayExpr)
|
||||
sub := fmt.Sprintf("NOT EXISTS (SELECT 1 FROM json_each(%s) WHERE NOT (%s))", arrayExpr, elemCond)
|
||||
return renderResult{sql: fmt.Sprintf("(%s AND %s)", nonEmpty, sub)}, nil
|
||||
case DialectMySQL:
|
||||
nonEmpty := fmt.Sprintf("%s IS NOT NULL AND JSON_LENGTH(%s) > 0", arrayExpr, arrayExpr)
|
||||
sub := fmt.Sprintf("NOT EXISTS (SELECT 1 FROM JSON_TABLE(%s, '$[*]' COLUMNS (value VARCHAR(512) PATH '$')) AS elem WHERE NOT (%s))", arrayExpr, elemCond)
|
||||
return renderResult{sql: fmt.Sprintf("(%s AND %s)", nonEmpty, sub)}, nil
|
||||
case DialectPostgres:
|
||||
nonEmpty := fmt.Sprintf("%s IS NOT NULL AND jsonb_array_length(%s) > 0", arrayExpr, arrayExpr)
|
||||
sub := fmt.Sprintf("NOT EXISTS (SELECT 1 FROM jsonb_array_elements_text(%s) AS elem(value) WHERE NOT (%s))", arrayExpr, elemCond)
|
||||
return renderResult{sql: fmt.Sprintf("(%s AND %s)", nonEmpty, sub)}, nil
|
||||
default:
|
||||
return renderResult{}, errors.Errorf("unsupported dialect %s", r.dialect)
|
||||
}
|
||||
}
|
||||
|
||||
// elementPredicateSQL builds the per-element SQL condition for an all() predicate.
|
||||
// The iterated element is exposed as the unqualified column `value` on all dialects
|
||||
// (json_each.value / JSON_TABLE column / elem(value)).
|
||||
func (r *renderer) elementPredicateSQL(pred PredicateExpr) (string, error) {
|
||||
switch p := pred.(type) {
|
||||
case *EqualsPredicate:
|
||||
return fmt.Sprintf("value = %s", r.addArg(p.Value)), nil
|
||||
case *StartsWithPredicate:
|
||||
return r.foldedLike("value", likePattern(TextMatchPrefix, p.Prefix)), nil
|
||||
case *EndsWithPredicate:
|
||||
return r.foldedLike("value", likePattern(TextMatchSuffix, p.Suffix)), nil
|
||||
case *ContainsPredicate:
|
||||
return r.foldedLike("value", likePattern(TextMatchContains, p.Substring)), nil
|
||||
default:
|
||||
return "", errors.Errorf("unsupported predicate %T in all()", pred)
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
> Note: `foldedLike`, `likePattern`, and `escapeLikeLiteral` were added in Task 1; reuse them as-is.
|
||||
|
||||
- [ ] **Step 6: Run tests to verify green**
|
||||
|
||||
Run: `go test ./store/test/ -run 'TestMemoFilterTagsAll' -v`
|
||||
Expected: PASS.
|
||||
|
||||
Run: `go test ./store/test/ -run 'TestMemoFilterTagsExists' -v`
|
||||
Expected: PASS (exists() rendering untouched).
|
||||
|
||||
- [ ] **Step 7: Commit**
|
||||
|
||||
```bash
|
||||
git add internal/filter/ir.go internal/filter/parser.go internal/filter/render.go store/test/memo_filter_test.go
|
||||
git commit -m "feat(filter): support all() comprehension over tags
|
||||
|
||||
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 6: Docs + full verification
|
||||
|
||||
**Files:**
|
||||
- Modify: `internal/filter/README.md`
|
||||
|
||||
- [ ] **Step 1: Document the new syntax** — append to the "SQL Generation Notes" section of `internal/filter/README.md`:
|
||||
|
||||
```markdown
|
||||
- **String Matching** — `content.contains(x)`, `content.startsWith(x)`, and
|
||||
`content.endsWith(x)` render as case-insensitive `LIKE`/`ILIKE` with LIKE
|
||||
metacharacters (`%`, `_`, `\`) escaped. Available on scalar string fields whose
|
||||
schema sets `SupportsContains` (memo `content`; attachment `filename`,
|
||||
`mime_type`).
|
||||
- **Regex** — `field.matches("pattern")` renders to `~` (Postgres) or `REGEXP`
|
||||
(MySQL/SQLite). SQLite uses a Go-backed `regexp` function registered in
|
||||
`store/db/sqlite/functions.go`. Patterns are validated at compile time against
|
||||
Go's RE2 via `cel.ValidateRegexLiterals()`. **Caveat:** regex *syntax* differs
|
||||
per engine (Go RE2 on SQLite, POSIX ERE on Postgres, ICU on MySQL 8.0+), so
|
||||
engine-specific patterns may not be portable.
|
||||
- **Tag `all()`** — `tags.all(t, <pred>)` matches only non-empty tag sets where
|
||||
every element satisfies the predicate, via per-element iteration
|
||||
(`json_each` / `jsonb_array_elements_text` / `JSON_TABLE`).
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run the full engine + store suite (SQLite)**
|
||||
|
||||
Run: `go test ./internal/filter/... ./store/...`
|
||||
Expected: PASS.
|
||||
|
||||
- [ ] **Step 3: Vet and lint**
|
||||
|
||||
Run: `go vet ./internal/filter/... ./store/db/sqlite/...`
|
||||
Expected: no output.
|
||||
|
||||
Run: `golangci-lint run internal/filter/... store/db/sqlite/...` (if available; skip if the binary is absent).
|
||||
Expected: no findings.
|
||||
|
||||
- [ ] **Step 4: Cross-dialect verification (if Docker/CI DSNs available)**
|
||||
|
||||
Run MySQL and Postgres suites to confirm the `all()` subqueries and regex operators render correctly:
|
||||
|
||||
```bash
|
||||
DRIVER=mysql go test ./store/test/ -run 'TagsAll|Matches|StartsWith|EndsWith'
|
||||
DRIVER=postgres go test ./store/test/ -run 'TagsAll|Matches|StartsWith|EndsWith'
|
||||
```
|
||||
|
||||
Expected: PASS. (These require the project's standard test DB setup; if unavailable locally, rely on CI which runs all three drivers.)
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add internal/filter/README.md
|
||||
git commit -m "docs(filter): document string matching, regex, and tag all() support
|
||||
|
||||
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Self-review notes
|
||||
|
||||
- **Spec coverage:** ① scalar `startsWith`/`endsWith` → Task 2; ② `all()` non-empty → Task 5; ④ `matches()` + SQLite REGEXP fn + `ValidateRegexLiterals` → Tasks 3-4; the LIKE-escaping fix → Task 1; docs/caveat → Task 6. `lowerAscii`/`upperAscii` correctly omitted (dropped in spec). Hardening/native-AST migration correctly deferred to the follow-up spec.
|
||||
- **Type consistency:** `TextMatchCondition`/`TextMatchMode`/`likePattern`/`foldedLike`/`escapeLikeLiteral` (Task 1) are reused by Tasks 2 and 5; `RegexCondition`/`renderRegex` (Task 4) and `ensureRegexpRegistered`/`compileRegexp` (Task 3) names match across their call sites; `ComprehensionAll` (Task 5) matches its parser and render references.
|
||||
- **Element reference:** the unqualified `value` column is produced by `json_each` (SQLite), the `JSON_TABLE(... COLUMNS (value ...))` (MySQL), and `elem(value)` (Postgres), so `elementPredicateSQL` is dialect-agnostic.
|
||||
```
|
||||
@@ -0,0 +1,124 @@
|
||||
# ActivityCalendar: Honor `timeBasis` (Create vs Update)
|
||||
|
||||
## Problem
|
||||
|
||||
The ActivityCalendar in `web/src/components/ActivityCalendar/` always aggregates by memo creation time, regardless of how the surrounding memo list is sorted.
|
||||
|
||||
The application already supports a global "time basis" toggle (`web/src/contexts/ViewContext.tsx:3`):
|
||||
|
||||
```ts
|
||||
export type MemoTimeBasis = "create_time" | "update_time";
|
||||
```
|
||||
|
||||
The toggle is persisted in `localStorage` and drives memo list ordering across the app. When a user switches the list to `update_time`, the heatmap below it continues to show creation counts — the two views literally disagree about what "today" means.
|
||||
|
||||
This is the user-visible bug we are fixing.
|
||||
|
||||
## Non-goals
|
||||
|
||||
The following were considered and explicitly excluded:
|
||||
|
||||
- **Tracking every individual edit event.** This would require resurrecting the `activity` table that was deliberately dropped in migration `0.27/03__drop_activity.sql`. The cost (write-path instrumentation, storage growth, privacy review) is not justified by this UI bug.
|
||||
- **Tracking archive / restore / delete events.** Housekeeping actions, not contributions; would also leak private behavior on public Profile/Explore pages.
|
||||
- **Adding comments or reactions to the heatmap count.** A reasonable separate feature, but a different decision (event-type expansion). Out of scope for this spec — one ticket, one problem.
|
||||
- **Renaming `ActivityCalendar` to `ContributionCalendar`.** Out of scope.
|
||||
|
||||
## Design
|
||||
|
||||
### Semantics
|
||||
|
||||
The heatmap aggregates one timestamp per memo:
|
||||
|
||||
- When `timeBasis === "create_time"`: use `memo.created_ts` (current behavior).
|
||||
- When `timeBasis === "update_time"`: use `memo.updated_ts`.
|
||||
|
||||
Each memo contributes exactly one cell of color, on the day of its chosen timestamp. This matches the list view's semantics exactly: in `update_time` mode, a memo edited on 5/1 and again on 5/2 appears once at 5/2 in the list, and the heatmap will show +1 on 5/2 and nothing on 5/1. The "lossiness" is identical to the lossiness already accepted by the list view — so by definition, the two are consistent.
|
||||
|
||||
### Backend
|
||||
|
||||
`UserStats` (proto/api/v1/user_service.proto) gains one field:
|
||||
|
||||
```proto
|
||||
// The latest update timestamps of the user's memos.
|
||||
repeated google.protobuf.Timestamp memo_updated_timestamps = 8;
|
||||
```
|
||||
|
||||
The implementation mirrors `memo_created_timestamps` in `server/router/api/v1/user_service_stats.go`: in the same loop that appends `memo.CreatedTs` (line 115), also append `memo.UpdatedTs` to a parallel slice. The same `FindMemo` filters apply automatically: `RowStatus: NORMAL` (archived excluded), `ExcludeComments: true`, and the viewer-based visibility filter. Both `GetUserStats` and `ListAllUserStats` paths must be updated symmetrically.
|
||||
|
||||
No DB migration. No new tables. No new write paths.
|
||||
|
||||
### Frontend
|
||||
|
||||
`web/src/hooks/useFilteredMemoStats.ts` reads `useView().timeBasis` and switches its data source:
|
||||
|
||||
- `create_time` → `userStats.memoCreatedTimestamps` (today's behavior, untouched)
|
||||
- `update_time` → `userStats.memoUpdatedTimestamps`
|
||||
|
||||
The `explore` context branch (which derives stats from the in-memory memo list rather than `userStats`) applies the same switch using `memo.createTime` vs `memo.updateTime` from the cached memos.
|
||||
|
||||
The `MonthCalendar` / `YearCalendar` components themselves require no changes — they receive an opaque `Record<date, count>` and render it. The change is confined to the data-source layer.
|
||||
|
||||
### Tooltip / labeling
|
||||
|
||||
A small but necessary clarification for the user: the cell tooltip should reflect which basis is active, e.g.
|
||||
|
||||
- `create_time` mode: "3 memos on May 2"
|
||||
- `update_time` mode: "3 memos updated on May 2"
|
||||
|
||||
This belongs in `ActivityCalendar/utils.ts:getTooltipText`, which already takes a `t` translator. Add a `timeBasis` argument and pick the right i18n key.
|
||||
|
||||
## Components
|
||||
|
||||
| Unit | Responsibility | Depends on |
|
||||
|---|---|---|
|
||||
| `UserStats` proto | Carry both timestamp arrays | — |
|
||||
| `GetUserStats` server impl | Populate both arrays from `memo` table | store |
|
||||
| `useFilteredMemoStats` | Pick the correct array based on `timeBasis`; aggregate by day | `useView`, `useUserStats`, `useMemos` |
|
||||
| `getTooltipText` | Render basis-aware tooltip | i18n |
|
||||
| `MonthCalendar` / `YearCalendar` | Unchanged — render `Record<date, count>` | — |
|
||||
|
||||
## Data flow
|
||||
|
||||
```
|
||||
ViewContext.timeBasis ──┐
|
||||
▼
|
||||
useFilteredMemoStats ── pick array ── countBy(day) ── Record<date,count> ── MonthCalendar
|
||||
▲
|
||||
userStats ───────────┤ (memo_created_timestamps OR memo_updated_timestamps)
|
||||
│
|
||||
memos cache ─────────┘ (createTime OR updateTime — explore context only)
|
||||
```
|
||||
|
||||
## Error handling
|
||||
|
||||
No new failure modes.
|
||||
|
||||
`protobuf-es` generates `repeated` fields as non-optional `T[]`, so an older server that doesn't populate the new field deserializes it as `[]` (never `undefined`). Naïvely treating empty as "no data" would be wrong, because a user with zero memos also gets `[]`. Detection uses **length divergence**: since `memo.updated_ts` is initialized to `created_ts` at row creation, the two arrays are the same length whenever there are any memos. So:
|
||||
|
||||
- `created.length === 0 && updated.length === 0` — user has no memos, render empty.
|
||||
- `created.length > 0 && updated.length === created.length` — new server, normal path.
|
||||
- `created.length > 0 && updated.length === 0` — old server, fall back to `memoCreatedTimestamps` regardless of `timeBasis`, with a one-line `console.warn`.
|
||||
|
||||
## Testing
|
||||
|
||||
- Unit test `useFilteredMemoStats`: given a fixed `userStats`, switching `timeBasis` returns aggregations matching the expected source array.
|
||||
- Unit test the new `getTooltipText` branch.
|
||||
- Manual verification: in dev, toggle the global time basis and confirm:
|
||||
- Heatmap recomputes
|
||||
- A memo edited yesterday but created last week shows up "yesterday" in update mode and "last week" in create mode
|
||||
- Tooltip text reflects the basis
|
||||
|
||||
## Migration / compatibility
|
||||
|
||||
- Proto field is additive (tag 8 is unused; tag 2 is `reserved`).
|
||||
- Old clients ignore the new field.
|
||||
- New clients tolerate old servers via the fallback above.
|
||||
- No DB migration.
|
||||
- No data backfill — `updated_ts` already exists on every memo row.
|
||||
|
||||
## Out-of-scope follow-ups (not part of this work)
|
||||
|
||||
These came up during brainstorming and are tracked here only so they aren't lost:
|
||||
|
||||
1. Adding comment / reaction event types to the heatmap count.
|
||||
2. A "Memo History / Versions" feature (per-edit snapshots, diffs, optional commit messages). If pursued, the heatmap would become a downstream consumer of that history, and the field added here may be revisited.
|
||||
@@ -0,0 +1,165 @@
|
||||
# Create memo on selected calendar date — design
|
||||
|
||||
**Date:** 2026-05-02
|
||||
**Scope:** Frontend-only.
|
||||
|
||||
## Problem
|
||||
|
||||
Clicking a date in the activity calendar filters the memo list to that date but does nothing for the inline editor. To create a memo dated for the selected day, the user must (1) create with today's timestamp, then (2) open the timestamp popover on the saved memo and edit `createTime`. This is a two-step retro-fill that defeats the calendar selection. Empty dates are also not clickable today, so the user cannot start the first memo for an empty day from the calendar at all.
|
||||
|
||||
## Goal
|
||||
|
||||
When a user picks a calendar date and immediately writes in the home editor, the resulting memo is created on that date — single-step.
|
||||
|
||||
## Non-goals
|
||||
|
||||
- Backend changes. The API already accepts custom `createTime` and `updateTime`.
|
||||
- Changes to the comment/reply editor or the edit-mode editor.
|
||||
- Changes outside the home page's editor render site (Explore/Archived/Profile pages have no editor).
|
||||
- Reworking the timestamp popover UI itself.
|
||||
- Empty-state copy changes on non-Home pages when an empty date is selected.
|
||||
|
||||
## User-visible behavior
|
||||
|
||||
1. **Empty calendar dates are clickable.** Clicking a date with zero memos sets the `displayTime` filter the same way a populated date does. Tooltip and selection ring still work.
|
||||
2. **When the home editor renders with an active `displayTime` filter:**
|
||||
- The `TimestampPopover` (already used in edit mode) appears in create mode, pre-populated with the selected date.
|
||||
- The draft's `createTime` is set to **selected local date + current local hh:mm:ss** (e.g., picking May 1 at 14:32 → `2025-05-01 14:32`).
|
||||
- The draft's `updateTime` is set to the same value, to avoid the saved memo immediately reading "updated today" relative to a back-dated `createTime`.
|
||||
- The user can adjust either field via the popover before saving.
|
||||
3. **When no `displayTime` filter is active**, the editor is identical to today: no popover in create mode, no override, server stamps with "now".
|
||||
4. **Live derivation.** If the filter changes while a draft is in progress, the editor's prefilled timestamps re-sync to the new date. The popover stays visible so the change is observable. (Manual popover edits before the next filter change are overwritten — chosen tradeoff.)
|
||||
5. **Future dates are allowed** (e.g., May 15 when today is May 2). Backend already accepts future timestamps.
|
||||
6. **Other contexts** (Explore/Archived/Profile) gain empty-date clickability for navigation consistency, but have no editor and so no prefill behavior.
|
||||
|
||||
## Architecture
|
||||
|
||||
Four touch points, all in `web/src/`:
|
||||
|
||||
| File | Change |
|
||||
|------|--------|
|
||||
| `components/ActivityCalendar/CalendarCell.tsx` | Drop `day.count > 0` gate so empty in-month cells are clickable. |
|
||||
| `components/MemoEditor/index.tsx` | Accept `defaultCreateTime?: Date` prop; render `TimestampPopover` in create mode when set; sync state on prop change. |
|
||||
| `components/MemoEditor/utils/deriveDefaultCreateTime.ts` (new) | Pure helper: `(filters, now?) => Date \| undefined` derived from any `displayTime` filter. |
|
||||
| `components/PagedMemoList/PagedMemoList.tsx` | At the home-editor render site (line 155), read `MemoFilterContext`, compute `defaultCreateTime`, pass as prop. |
|
||||
|
||||
### Data flow
|
||||
|
||||
```
|
||||
CalendarCell click
|
||||
→ useDateFilterNavigation
|
||||
→ URL ?filter=displayTime:YYYY-MM-DD
|
||||
→ MemoFilterContext re-renders
|
||||
→ PagedMemoList recomputes defaultCreateTime via deriveDefaultCreateTimeFromFilters(filters)
|
||||
→ <MemoEditor defaultCreateTime={...}> re-renders
|
||||
→ editor reducer syncs state.timestamps (create + update) and renders TimestampPopover
|
||||
→ save → memoService.ts:111 sends createTime/updateTime to API
|
||||
```
|
||||
|
||||
## Component contracts
|
||||
|
||||
### `MemoEditor` — new prop
|
||||
|
||||
```ts
|
||||
interface MemoEditorProps {
|
||||
// ...existing props
|
||||
/**
|
||||
* When set in create mode (no `memo` prop), seeds the draft's
|
||||
* createTime/updateTime and reveals the TimestampPopover so the
|
||||
* user can adjust. Tracked live: changes after mount re-sync state.
|
||||
* Ignored in edit mode (when `memo` is set).
|
||||
*/
|
||||
defaultCreateTime?: Date;
|
||||
}
|
||||
```
|
||||
|
||||
Internal behavior:
|
||||
- On `INIT_MEMO` for create mode, if `defaultCreateTime` is set, payload `timestamps` is `{ createTime: defaultCreateTime, updateTime: defaultCreateTime }`.
|
||||
- A `useEffect` keyed on `[defaultCreateTime?.getTime(), memo]` dispatches `SET_TIMESTAMPS` whenever the prop changes in create mode.
|
||||
- Popover render condition becomes `memoName || (!memo && state.timestamps.createTime)`.
|
||||
|
||||
### `deriveDefaultCreateTimeFromFilters` — pure helper
|
||||
|
||||
```ts
|
||||
// web/src/components/MemoEditor/utils/deriveDefaultCreateTime.ts
|
||||
export function deriveDefaultCreateTimeFromFilters(
|
||||
filters: MemoFilter[],
|
||||
now: Date = new Date(),
|
||||
): Date | undefined {
|
||||
const dateFilter = filters.find((f) => f.factor === "displayTime");
|
||||
if (!dateFilter) return undefined;
|
||||
const [y, m, d] = dateFilter.value.split("-").map(Number);
|
||||
if (!y || !m || !d) return undefined;
|
||||
return new Date(y, m - 1, d, now.getHours(), now.getMinutes(), now.getSeconds());
|
||||
}
|
||||
```
|
||||
|
||||
Notes:
|
||||
- Defensive parse — returns `undefined` for malformed values rather than throwing.
|
||||
- `now` is injectable for deterministic tests.
|
||||
- Multiple `displayTime` filters are not produced by current UI; `find` ignores extras safely.
|
||||
|
||||
### `PagedMemoList.tsx` — call-site change
|
||||
|
||||
```tsx
|
||||
const { filters } = useMemoFilterContext();
|
||||
const defaultCreateTime = useMemo(
|
||||
() => deriveDefaultCreateTimeFromFilters(filters),
|
||||
[filters],
|
||||
);
|
||||
// ...
|
||||
{showMemoEditor ? (
|
||||
<MemoEditor
|
||||
className="mb-2"
|
||||
cacheKey="home-memo-editor"
|
||||
placeholder={t("editor.any-thoughts")}
|
||||
defaultCreateTime={defaultCreateTime}
|
||||
/>
|
||||
) : null}
|
||||
```
|
||||
|
||||
`useMemo` keyed on `filters` keeps the reference stable when the filter doesn't change, avoiding unnecessary editor re-syncs. `now` is captured once per filter change — matches "the local time when you picked the date".
|
||||
|
||||
### `CalendarCell.tsx` — empty-cell clickability
|
||||
|
||||
- `handleClick`: drop the `day.count > 0` check; just call `onClick(day.date)` if `onClick` is provided.
|
||||
- `isInteractive`: `Boolean(onClick)`.
|
||||
- `tabIndex` / `aria-disabled` / hover-cursor classes follow the new `isInteractive`.
|
||||
- `shouldShowTooltip`: drop the `day.count > 0` gate; tooltip text already conveys the count.
|
||||
- Out-of-month cells (existing early return) stay unclickable.
|
||||
- The `selected` ring already works on count=0 cells. Visual contrast on the lowest-intensity background may need a small ring-weight bump in light theme; eyeball during implementation.
|
||||
|
||||
## Edge cases
|
||||
|
||||
- **No filter / filter cleared:** `defaultCreateTime` becomes `undefined`; editor falls back to current behavior.
|
||||
- **User edits draft, then re-picks date:** live-derived; editor's `createTime` updates, popover reflects new value.
|
||||
- **User manually edits via popover, then changes filter:** prop sync overwrites manual edit. Acceptable per design choice; popover keeps the change observable.
|
||||
- **Draft cache (`cacheKey="home-memo-editor"`):** caches `content`, not `timestamps`. Reload restores text but `createTime` is freshly derived from current filter — consistent.
|
||||
- **Future dates:** allowed. No clamp.
|
||||
- **DST / timezone:** date arithmetic uses local time (`new Date(y, m-1, d, h, mi, s)`), matching `useDateFilterNavigation`'s local-date convention. Server receives an absolute `Timestamp`.
|
||||
- **Comment editor (`MemoCommentSection`):** doesn't pass `defaultCreateTime` → no behavior change.
|
||||
- **Edit mode (`memo` prop set):** prop is ignored; existing edit-mode popover is unchanged.
|
||||
- **Empty-date click on Explore/Archived/Profile:** filters to empty date → "no memos" empty state. Acceptable.
|
||||
|
||||
## Testing
|
||||
|
||||
- **Unit (Vitest)** for `deriveDefaultCreateTimeFromFilters`:
|
||||
- no `displayTime` filter → `undefined`
|
||||
- valid `displayTime:2025-05-01` + injected `now=14:32:10` → `2025-05-01 14:32:10` local
|
||||
- malformed value (`"not-a-date"`, `"2025-13-40"`) → `undefined`
|
||||
- extra non-`displayTime` filters present → still works
|
||||
- **Component (React Testing Library) for `CalendarCell`:** count=0 in-month cell is clickable, has correct `tabIndex`/`aria-disabled`, fires `onClick` with date.
|
||||
- **Component for `MemoEditor`:** with `defaultCreateTime` prop, popover renders in create mode and `state.timestamps.createTime` matches; without prop, no popover; changing the prop re-syncs state.
|
||||
- **Manual smoke (per CLAUDE.md UI-changes rule):** `pnpm dev`, click a non-today date (with and without existing memos), type a memo, save, confirm it appears under that date. Clear the filter chip; confirm a new memo posts to today.
|
||||
|
||||
## Risks
|
||||
|
||||
- The `useEffect` re-sync overwriting an in-progress popover edit is a *chosen* behavior. If users later complain, a "manual override sticky" flag is the natural follow-up. Not pre-built.
|
||||
- Selection-ring contrast on the lowest-intensity background may need a small visual tweak; flagged for implementation.
|
||||
|
||||
## Out of scope (explicit)
|
||||
|
||||
- Sticky manual-override semantics for the popover.
|
||||
- New empty-state copy on Explore/Archived/Profile when filtering to a date with zero memos.
|
||||
- Any backend/API change.
|
||||
- Any change to the comment editor, edit mode, or non-Home editor sites.
|
||||
@@ -0,0 +1,511 @@
|
||||
# STT and Audio-LLM Split — Design Spec
|
||||
|
||||
**Date:** 2026-05-02
|
||||
**Status:** Draft, pending user review
|
||||
|
||||
## 1. Goal
|
||||
|
||||
Refactor `internal/ai/` to split **speech-to-text (STT)** and **audio-multimodal-LLM (Audio-LLM)** into two separate Go interfaces, aligning with mainstream OSS conventions (Vercel AI SDK, LiteLLM, the Go AI ecosystem). Update the public API handler to dispatch to the right interface based on provider type. Make two small comment improvements to `proto/store/instance_setting.proto` and the generated bindings; **no proto field changes**.
|
||||
|
||||
## 2. Non-Goals
|
||||
|
||||
The following are intentionally **out of scope** for this design:
|
||||
|
||||
- **`enabled` boolean field on `TranscriptionConfig`** (improvement #1 from the brainstorming) — keep using `provider_id == ""` as the disabled signal.
|
||||
- **Direction C (audio → structured note pipeline)** — auto-summarization / tag extraction. Independent future feature.
|
||||
- **Multi-provider STT with default-model selector** (Dify-style) — Memos has a single transcription config; that stays.
|
||||
- **Per-model credential overrides**, **load balancing**, **capability YAML schemas** — Dify-style enterprise complexity.
|
||||
- **Streaming transcription, retry policy, OpenAI Translations endpoint** — YAGNI.
|
||||
- **`gpt-4o-audio-preview` user-facing support** — the `audiollm/openai` package will be implementable after this refactor, but UI support is a follow-up.
|
||||
- **TTS** (text-to-speech) — different concern, not affected.
|
||||
|
||||
## 3. Background
|
||||
|
||||
The current `internal/ai/` has one `Transcriber` interface with two implementations: `openAITranscriber` (calls `/audio/transcriptions`, a real STT endpoint) and `geminiTranscriber` (calls `generateContent`, a multimodal-LLM endpoint dressed up to act like STT). This conflation has caused real symptoms:
|
||||
|
||||
- `TranscribeResponse.Language` and `Duration` are silently empty for Gemini (Gemini multimodal doesn't return them).
|
||||
- The `Prompt` field has different semantics across providers — Whisper treats it as a soft hint that may be ignored; Gemini treats it as a literal instruction.
|
||||
- Gemini's multimodal failure modes (safety filter, token-truncation, refusals) are flattened to a single "did not include text" error.
|
||||
- Gemini-specific code (WebM transcoding, `maxGeminiInlineAudioSize`, `genai` SDK) lives in the same package as the OpenAI Whisper integration.
|
||||
|
||||
The brainstorming session (this conversation, 2026-05-02) ran two rounds of OSS research to validate the corrective direction.
|
||||
|
||||
## 4. Research Findings Summary
|
||||
|
||||
Detailed findings in conversation history; abridged here for design accountability.
|
||||
|
||||
### 4.1 SDK-Layer Research
|
||||
|
||||
| Source | Key Decision |
|
||||
|---|---|
|
||||
| **Vercel AI SDK** (`vercel/ai`) | `TranscriptionModelV3` is implemented **only** by providers with a dedicated STT endpoint (OpenAI Whisper/gpt-4o-transcribe, Deepgram, ElevenLabs, AssemblyAI). **Google provider deliberately does not implement it** — Gemini audio rides through `generateText` with `FilePart`. Two completely separate code paths. No "source" discriminator. Provider id is `vendor.modality` (`openai.transcription`); model is a free string. |
|
||||
| **LiteLLM** (`BerriAI/litellm`) | `litellm.transcription()` only routes to providers with `/audio/transcriptions`-style endpoints. **Gemini is absent** from the transcription router (`litellm/llms/gemini/` has no `audio_transcription/` subdirectory). Multimodal audio rides through `litellm.completion()` with `{"type":"input_audio"}` content parts. Response is `text + usage`, no provider discriminator. |
|
||||
| **Go AI SDKs** (`cloudwego/eino`, `tmc/langchaingo`, `sashabaranov/go-openai`) | One package per provider; provider identity = import path; **no provider enum**. `Model` is opaque string. OpenAI-compatible endpoints handled via `BaseURL` config field, never via separate package. go-openai's `audio.go` is structurally separate from `chat.go`. |
|
||||
|
||||
**Convergent finding:** All three ecosystems split STT and multimodal-audio into separate interfaces. None expose a "this came from a multimodal LLM" discriminator. None encode wire-format into the provider type enum.
|
||||
|
||||
### 4.2 Application-Layer Research
|
||||
|
||||
| Source | STT-Storage Design |
|
||||
|---|---|
|
||||
| **Open WebUI** | STT is a **flat singleton config block** (`audio.stt.*` namespace), completely separate from chat providers (`openai.*` namespace). `STT_ENGINE` enum dispatches; per-engine credentials side-by-side in one config. |
|
||||
| **LobeChat** | STT is a **separate global user setting** (`UserTTSConfig`). But credentials silently piggyback on the `openai` chat provider's `keyVaults` — author has marked the helper `@deprecated`. |
|
||||
| **Dify** | `ProviderEntity.supported_model_types` declares capabilities; STT is the `SPEECH2TEXT` enum value. STT info lives in a **separate "system model" config row** (`tenant_default_models(model_type='speech2text', provider_name, model_name)`) that **references** an existing provider. |
|
||||
|
||||
**Convergent finding:** Zero apps add STT-specific fields onto the AI provider entity. All three keep providers capability-agnostic and put STT config in a separate place.
|
||||
|
||||
### 4.3 Proto Schema Assessment
|
||||
|
||||
The current `proto/store/instance_setting.proto` `InstanceAISetting` + `TranscriptionConfig` is **already aligned with the mainstream pattern**:
|
||||
|
||||
- ✅ `AIProviderConfig` carries no STT-specific field (capability-agnostic)
|
||||
- ✅ `TranscriptionConfig` is a separate pointer (`provider_id` references a provider)
|
||||
- ✅ `AIProviderType` is vendor-level (`OPENAI`, `GEMINI`) — no wire-format suffix
|
||||
- ✅ `model` is a free string
|
||||
- ✅ Comments already document Whisper vs Gemini prompt semantics (though could be clearer)
|
||||
|
||||
The proto schema requires **no field changes**. Only two comment improvements (§7 below).
|
||||
|
||||
## 5. Current State (Files Touched)
|
||||
|
||||
```
|
||||
internal/ai/
|
||||
ai.go # ProviderType, ProviderConfig, errors
|
||||
client.go # NewTranscriber factory, transcriberOptions, normalizeEndpoint, requireAPIKey
|
||||
transcription.go # Transcriber interface, TranscribeRequest, TranscribeResponse
|
||||
openai.go # openAITranscriber → /audio/transcriptions
|
||||
openai_test.go
|
||||
gemini.go # geminiTranscriber → generateContent (multimodal)
|
||||
gemini_test.go
|
||||
models.go # DefaultOpenAITranscriptionModel, DefaultGeminiTranscriptionModel
|
||||
resolver.go # FindProvider
|
||||
errors.go # ErrProviderNotFound, ErrCapabilityUnsupported
|
||||
audio/
|
||||
webm.go # IsWebMContentType, WebMOpusToWAV (used by Gemini path)
|
||||
webm_test.go
|
||||
|
||||
server/router/api/v1/
|
||||
ai_service.go # Transcribe handler (lines 42–123)
|
||||
|
||||
proto/store/
|
||||
instance_setting.proto # InstanceAISetting, AIProviderConfig, AIProviderType, TranscriptionConfig
|
||||
|
||||
web/src/components/Settings/
|
||||
AISection.tsx # Provider list UI, TranscriptionForm
|
||||
```
|
||||
|
||||
The handler at `server/router/api/v1/ai_service.go:42` is the **single integration point** between proto config and the `internal/ai/` SDK. It already discards Language/Duration (returns `{Text}` only), so the response narrowing is already in place.
|
||||
|
||||
## 6. Target Design
|
||||
|
||||
### 6.1 Package Structure
|
||||
|
||||
```
|
||||
internal/ai/
|
||||
ai.go # ProviderType (unchanged: OPENAI, GEMINI), ProviderConfig (unchanged)
|
||||
resolver.go # FindProvider (unchanged)
|
||||
errors.go # add ErrSTTNotSupported, ErrAudioLLMNotSupported
|
||||
audio/
|
||||
webm.go # unchanged — moves with audiollm/gemini consumer
|
||||
webm_test.go
|
||||
|
||||
stt/
|
||||
stt.go # Transcriber interface, Request, Response, Segment
|
||||
factory.go # NewTranscriber(cfg ai.ProviderConfig, opts...) (Transcriber, error)
|
||||
options.go # TranscriberOption, WithHTTPClient
|
||||
openai/
|
||||
openai.go # openAITranscriber → POST /audio/transcriptions
|
||||
openai_test.go
|
||||
|
||||
audiollm/
|
||||
audiollm.go # Model interface, Request, Response, FinishReason
|
||||
factory.go # NewModel(cfg ai.ProviderConfig, opts...) (Model, error)
|
||||
options.go # ModelOption, WithHTTPClient
|
||||
gemini/
|
||||
gemini.go # geminiModel → POST :generateContent (multimodal audio)
|
||||
gemini_test.go
|
||||
# openai/ — NOT created in this refactor; reserved for future gpt-4o-audio support
|
||||
```
|
||||
|
||||
**Rationale (Go-ecosystem convention, per §4.1):** one package per provider; provider identity is import path; capability is implied by which umbrella package (`stt` vs `audiollm`) you import from. The runtime dispatch (factory) is the only place that translates `ProviderConfig.Type` enum → concrete implementation.
|
||||
|
||||
### 6.2 Interfaces
|
||||
|
||||
#### `internal/ai/stt/stt.go`
|
||||
|
||||
```go
|
||||
package stt
|
||||
|
||||
import (
|
||||
"context"
|
||||
"io"
|
||||
)
|
||||
|
||||
// Transcriber transcribes audio into text using a provider's dedicated STT endpoint
|
||||
// (e.g. OpenAI /audio/transcriptions). Implementations are deterministic STT —
|
||||
// they are NOT for multimodal LLMs that happen to accept audio input. For
|
||||
// multimodal audio understanding, see internal/ai/audiollm.
|
||||
type Transcriber interface {
|
||||
Transcribe(ctx context.Context, req Request) (*Response, error)
|
||||
}
|
||||
|
||||
type Request struct {
|
||||
Audio io.Reader
|
||||
Size int64
|
||||
Filename string
|
||||
ContentType string // IANA media type, e.g. "audio/wav"
|
||||
Model string // provider-specific model id (e.g. "whisper-1", "gpt-4o-transcribe")
|
||||
Prompt string // soft spelling/vocabulary hint (Whisper "prompt" parameter)
|
||||
Language string // ISO 639-1, optional
|
||||
}
|
||||
|
||||
type Response struct {
|
||||
Text string
|
||||
Language string // empty if provider did not return it (best-effort)
|
||||
Segments []Segment // empty unless provider returned timestamps
|
||||
}
|
||||
|
||||
type Segment struct {
|
||||
Text string
|
||||
Start float64
|
||||
End float64
|
||||
Speaker string // empty unless using a diarization-capable model (e.g. gpt-4o-transcribe-diarize)
|
||||
}
|
||||
```
|
||||
|
||||
#### `internal/ai/audiollm/audiollm.go`
|
||||
|
||||
```go
|
||||
package audiollm
|
||||
|
||||
import (
|
||||
"context"
|
||||
"io"
|
||||
)
|
||||
|
||||
// Model invokes a multimodal LLM with audio input. Implementations call
|
||||
// chat-completions or generate-content style APIs that happen to accept audio.
|
||||
// They are NOT deterministic STT — outputs may be refused, truncated, or
|
||||
// rephrased per the LLM's behavior. For pure transcription, prefer
|
||||
// internal/ai/stt where available.
|
||||
type Model interface {
|
||||
GenerateFromAudio(ctx context.Context, req Request) (*Response, error)
|
||||
}
|
||||
|
||||
type Request struct {
|
||||
Audio io.Reader
|
||||
Size int64
|
||||
ContentType string
|
||||
Model string
|
||||
Instructions string // literal instruction the model is expected to follow
|
||||
Temperature *float32 // optional; nil leaves provider default
|
||||
}
|
||||
|
||||
type Response struct {
|
||||
Text string
|
||||
FinishReason FinishReason
|
||||
}
|
||||
|
||||
type FinishReason string
|
||||
|
||||
const (
|
||||
FinishStop FinishReason = "stop" // model finished normally
|
||||
FinishLength FinishReason = "length" // truncated by max-tokens
|
||||
FinishSafety FinishReason = "safety" // safety filter blocked output
|
||||
FinishOther FinishReason = "other" // anything else (incl. unknown)
|
||||
)
|
||||
```
|
||||
|
||||
#### Factory dispatch
|
||||
|
||||
```go
|
||||
// internal/ai/stt/factory.go
|
||||
package stt
|
||||
|
||||
import (
|
||||
"github.com/pkg/errors"
|
||||
"github.com/usememos/memos/internal/ai"
|
||||
"github.com/usememos/memos/internal/ai/stt/openai"
|
||||
)
|
||||
|
||||
func NewTranscriber(cfg ai.ProviderConfig, opts ...TranscriberOption) (Transcriber, error) {
|
||||
switch cfg.Type {
|
||||
case ai.ProviderOpenAI:
|
||||
return openai.New(cfg, applyOptions(opts...))
|
||||
case ai.ProviderGemini:
|
||||
return nil, errors.Wrapf(ai.ErrSTTNotSupported,
|
||||
"Gemini does not provide a dedicated STT endpoint; use audiollm.NewModel instead")
|
||||
default:
|
||||
return nil, errors.Wrapf(ai.ErrCapabilityUnsupported, "provider type %q", cfg.Type)
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
```go
|
||||
// internal/ai/audiollm/factory.go
|
||||
package audiollm
|
||||
|
||||
import (
|
||||
"github.com/pkg/errors"
|
||||
"github.com/usememos/memos/internal/ai"
|
||||
"github.com/usememos/memos/internal/ai/audiollm/gemini"
|
||||
)
|
||||
|
||||
func NewModel(cfg ai.ProviderConfig, opts ...ModelOption) (Model, error) {
|
||||
switch cfg.Type {
|
||||
case ai.ProviderGemini:
|
||||
return gemini.New(cfg, applyOptions(opts...))
|
||||
case ai.ProviderOpenAI:
|
||||
// NOTE: gpt-4o-audio-preview support belongs here but is out of scope;
|
||||
// see §2 (Non-Goals).
|
||||
return nil, errors.Wrapf(ai.ErrAudioLLMNotSupported,
|
||||
"OpenAI multimodal audio (gpt-4o-audio) is not yet implemented in this codebase")
|
||||
default:
|
||||
return nil, errors.Wrapf(ai.ErrCapabilityUnsupported, "provider type %q", cfg.Type)
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### 6.3 Backend Handler Dispatch
|
||||
|
||||
The handler at `server/router/api/v1/ai_service.go:Transcribe` dispatches based on `provider.Type`:
|
||||
|
||||
```go
|
||||
func (s *APIV1Service) Transcribe(ctx context.Context, request *v1pb.TranscribeRequest) (*v1pb.TranscribeResponse, error) {
|
||||
// ... existing config loading, provider resolution, audio reading ...
|
||||
|
||||
switch provider.Type {
|
||||
case ai.ProviderOpenAI:
|
||||
text, err := s.transcribeViaSTT(ctx, provider, transcriptionCfg, audio, contentType)
|
||||
if err != nil {
|
||||
return nil, status.Errorf(codes.Internal, "failed to transcribe: %v", err)
|
||||
}
|
||||
return &v1pb.TranscribeResponse{Text: text}, nil
|
||||
|
||||
case ai.ProviderGemini:
|
||||
text, err := s.transcribeViaAudioLLM(ctx, provider, transcriptionCfg, audio, contentType)
|
||||
if err != nil {
|
||||
return nil, status.Errorf(codes.Internal, "failed to transcribe: %v", err)
|
||||
}
|
||||
return &v1pb.TranscribeResponse{Text: text}, nil
|
||||
|
||||
default:
|
||||
return nil, status.Errorf(codes.FailedPrecondition,
|
||||
"provider type %q is not supported for transcription", provider.Type)
|
||||
}
|
||||
}
|
||||
|
||||
func (s *APIV1Service) transcribeViaSTT(ctx context.Context, provider ai.ProviderConfig,
|
||||
cfg *storepb.TranscriptionConfig,
|
||||
audio io.Reader, contentType string) (string, error) {
|
||||
t, err := stt.NewTranscriber(provider)
|
||||
if err != nil { return "", err }
|
||||
resp, err := t.Transcribe(ctx, stt.Request{
|
||||
Audio: audio,
|
||||
Filename: "audio",
|
||||
ContentType: contentType,
|
||||
Model: resolveModel(provider, cfg.Model),
|
||||
Prompt: cfg.Prompt, // Whisper: soft hint, may be ignored
|
||||
Language: cfg.Language,
|
||||
})
|
||||
if err != nil { return "", err }
|
||||
return resp.Text, nil
|
||||
}
|
||||
|
||||
func (s *APIV1Service) transcribeViaAudioLLM(ctx context.Context, provider ai.ProviderConfig,
|
||||
cfg *storepb.TranscriptionConfig,
|
||||
audio io.Reader, contentType string) (string, error) {
|
||||
m, err := audiollm.NewModel(provider)
|
||||
if err != nil { return "", err }
|
||||
resp, err := m.GenerateFromAudio(ctx, audiollm.Request{
|
||||
Audio: audio,
|
||||
ContentType: contentType,
|
||||
Model: resolveModel(provider, cfg.Model),
|
||||
Instructions: buildTranscriptionInstructions(cfg.Prompt, cfg.Language),
|
||||
})
|
||||
if err != nil { return "", err }
|
||||
if resp.FinishReason != audiollm.FinishStop {
|
||||
return "", errors.Errorf("transcription incomplete (finish reason: %s)", resp.FinishReason)
|
||||
}
|
||||
return resp.Text, nil
|
||||
}
|
||||
```
|
||||
|
||||
`buildTranscriptionInstructions` lives next to the handler and centralizes the literal instruction sent to multimodal LLMs:
|
||||
|
||||
```go
|
||||
func buildTranscriptionInstructions(prompt, language string) string {
|
||||
parts := []string{
|
||||
"Transcribe the audio accurately. Return only the transcript text. " +
|
||||
"Do not summarize, explain, or add content that is not spoken.",
|
||||
}
|
||||
if language != "" {
|
||||
parts = append(parts, fmt.Sprintf("The input language is %s.", language))
|
||||
}
|
||||
if prompt != "" {
|
||||
parts = append(parts, "Context and spelling hints:\n"+prompt)
|
||||
}
|
||||
return strings.Join(parts, "\n\n")
|
||||
}
|
||||
```
|
||||
|
||||
`resolveModel` returns `cfg.Model` if non-empty, else the per-provider default from `ai/models.go` (unchanged from today).
|
||||
|
||||
### 6.4 Implementation Notes Per Package
|
||||
|
||||
#### `internal/ai/stt/openai/openai.go`
|
||||
|
||||
- Identical wire behavior to current `internal/ai/openai.go::openAITranscriber.Transcribe`.
|
||||
- Uses `github.com/openai/openai-go/v3` SDK (already a dep).
|
||||
- Defaults `endpoint` to `https://api.openai.com/v1`. Trims trailing slash. Validates URL.
|
||||
- Honors `cfg.Endpoint` to support OpenAI-compatible providers (Groq Whisper, faster-whisper self-hosted, Azure Whisper deployments). The user simply adds another `AIProviderConfig` row with `Type=OPENAI` and a different `Endpoint`.
|
||||
- Supports any model the underlying endpoint accepts: `whisper-1`, `gpt-4o-transcribe`, `gpt-4o-mini-transcribe`, `gpt-4o-transcribe-diarize`, etc. The model string is opaque.
|
||||
- Returns `Response.Language` and `Response.Segments` populated when the API returns them; otherwise empty.
|
||||
|
||||
#### `internal/ai/audiollm/gemini/gemini.go`
|
||||
|
||||
- Identical wire behavior to current `internal/ai/gemini.go::geminiTranscriber.Transcribe`, EXCEPT:
|
||||
- Reads `Instructions` from the caller (not hardcoded inside the package).
|
||||
- Maps `genai.FinishReason` to `audiollm.FinishReason` (`STOP→FinishStop`, `MAX_TOKENS→FinishLength`, `SAFETY→FinishSafety`, anything else → `FinishOther`).
|
||||
- Returns `Response{Text, FinishReason}` instead of swallowing the finish reason into a generic error.
|
||||
- Continues to use `internal/ai/audio.WebMOpusToWAV` for WebM transcoding (Gemini doesn't accept WebM).
|
||||
- Continues to enforce `maxGeminiInlineAudioSize` (14 MiB) — File API support is out of scope.
|
||||
- Uses `google.golang.org/genai` SDK (already a dep).
|
||||
|
||||
#### `internal/ai/errors.go`
|
||||
|
||||
Add:
|
||||
```go
|
||||
var ErrSTTNotSupported = errors.New("provider does not support speech-to-text capability")
|
||||
var ErrAudioLLMNotSupported = errors.New("provider does not support multimodal audio capability")
|
||||
```
|
||||
Keep existing `ErrProviderNotFound` and `ErrCapabilityUnsupported`.
|
||||
|
||||
### 6.5 Proto Schema Changes
|
||||
|
||||
**Two comment-only updates. No field additions, no field renames, no breaking changes.**
|
||||
|
||||
#### Improvement #2 — `TranscriptionConfig.model` comment
|
||||
|
||||
Replace lines 179–181 of `proto/store/instance_setting.proto`:
|
||||
|
||||
```proto
|
||||
// model is the provider-specific model identifier.
|
||||
// Empty string falls back to the engine default
|
||||
// (whisper-1 for OPENAI providers, gemini-2.5-flash for GEMINI providers).
|
||||
string model = 2;
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```proto
|
||||
// model is the provider-specific model identifier.
|
||||
// Empty string falls back to the engine default.
|
||||
// OPENAI examples:
|
||||
// - whisper-1 (legacy, lower cost)
|
||||
// - gpt-4o-transcribe, gpt-4o-mini-transcribe (higher quality)
|
||||
// - gpt-4o-transcribe-diarize (includes speaker labels)
|
||||
// GEMINI examples:
|
||||
// - gemini-2.5-flash (default, multimodal call)
|
||||
// - gemini-2.5-pro
|
||||
string model = 2;
|
||||
```
|
||||
|
||||
**Rationale:** OpenAI's `/audio/transcriptions` endpoint now supports the `gpt-4o-transcribe` family in addition to `whisper-1`. The current comment is misleading — it implies Whisper is the only OpenAI option.
|
||||
|
||||
#### Improvement #3 — `TranscriptionConfig.prompt` comment
|
||||
|
||||
Replace lines 188–191:
|
||||
|
||||
```proto
|
||||
// prompt is a default spelling/vocabulary hint passed to the provider.
|
||||
// Used as the OpenAI Whisper "prompt" parameter and folded into the Gemini
|
||||
// generation prompt as a "Context and spelling hints" block.
|
||||
string prompt = 4;
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```proto
|
||||
// prompt is a default spelling/vocabulary hint passed to the provider.
|
||||
// Used as the OpenAI Whisper "prompt" parameter (a soft hint that the model
|
||||
// may ignore) and folded into the Gemini generation prompt as a "Context and
|
||||
// spelling hints" block (which the LLM will treat more literally).
|
||||
string prompt = 4;
|
||||
```
|
||||
|
||||
**Rationale:** Same field, two semantically different behaviors. Surfacing this in the schema documentation (which propagates to generated Go and TypeScript via JSDoc) makes the cross-provider variability explicit for any caller reading the bindings cold.
|
||||
|
||||
After editing the proto, regenerate via `cd proto && buf format -w && buf generate`. The two regenerated files are:
|
||||
- `proto/gen/store/instance_setting.pb.go`
|
||||
- `web/src/types/proto/store/instance_setting_pb.ts`
|
||||
|
||||
#### Why NOT add an `enabled` field (improvement #1)
|
||||
|
||||
Out of scope per §2. Doing it would add a new field that the frontend, backend, and migration logic all need to handle, for the sole benefit of letting users "disable but keep the config." The current `provider_id == ""` semantics work; the cost of the change exceeds the benefit at this moment.
|
||||
|
||||
### 6.6 Frontend Impact
|
||||
|
||||
Minimal. `web/src/components/Settings/AISection.tsx` already:
|
||||
|
||||
- Switches the model placeholder per provider (`placeholderForProvider` at line 371, using `setting.ai.transcription-model-placeholder-gemini` / `-openai`).
|
||||
- Disables the form when `providerId == ""`.
|
||||
- Validates that the referenced provider exists.
|
||||
|
||||
Recommended adjustments (in scope):
|
||||
|
||||
1. **Update i18n model placeholder strings** in `web/src/locales/en.json` to reflect the new model examples:
|
||||
- `setting.ai.transcription-model-placeholder-openai`: include `gpt-4o-transcribe` family alongside `whisper-1`.
|
||||
- `setting.ai.transcription-model-placeholder-gemini`: confirm `gemini-2.5-flash` is the listed example.
|
||||
2. **Update the prompt help text** (`setting.ai.transcription-prompt-help`) to note the cross-provider semantic difference, mirroring the new proto comment in user-facing language.
|
||||
|
||||
No structural component changes. No new fields. No state-shape changes.
|
||||
|
||||
### 6.7 What Stays Identical
|
||||
|
||||
- Database storage (`InstanceSetting` rows, `AISetting` blob) — proto field tags unchanged.
|
||||
- API surface (`TranscribeRequest`, `TranscribeResponse` messages) — unchanged.
|
||||
- gRPC/Connect endpoint paths — unchanged.
|
||||
- Frontend state shape (`LocalTranscription`) — unchanged.
|
||||
- All existing tests semantically unchanged (will be ported to new package paths).
|
||||
|
||||
## 7. Migration Path
|
||||
|
||||
The refactor is internal to the Go server. End-to-end behavior is preserved. Migration is staged so each stage is independently buildable, testable, and revertable.
|
||||
|
||||
| Stage | What | Compiles? | Tests pass? |
|
||||
|---|---|---|---|
|
||||
| A | Add `internal/ai/stt/` and `internal/ai/audiollm/` with new interfaces and (empty) factories. Add new errors. | ✅ | ✅ (no callers yet) |
|
||||
| B | Implement `internal/ai/stt/openai/` — port behavior from current `openai.go::openAITranscriber`. Port tests to `stt/openai/openai_test.go`. | ✅ | ✅ |
|
||||
| C | Implement `internal/ai/audiollm/gemini/` — port behavior from current `gemini.go::geminiTranscriber`, but: lift instructions out into the caller, return `FinishReason` instead of swallowing it. Port tests. | ✅ | ✅ |
|
||||
| D | Refactor `server/router/api/v1/ai_service.go::Transcribe` to dispatch via the new factories. Add `transcribeViaSTT` and `transcribeViaAudioLLM`. Add `buildTranscriptionInstructions`. | ✅ | ✅ |
|
||||
| E | Delete old files: `internal/ai/transcription.go`, `client.go`, `openai.go`, `openai_test.go`, `gemini.go`, `gemini_test.go`. | ✅ | ✅ |
|
||||
| F | Update proto comments (#2 and #3), run `buf format -w && buf generate`. | ✅ | ✅ |
|
||||
| G | Update `web/src/locales/en.json` strings for model placeholders and prompt help. | ✅ | ✅ |
|
||||
|
||||
Each stage is one commit. Reverting any single stage leaves the system in a working state.
|
||||
|
||||
## 8. Anti-Patterns Avoided (and Why)
|
||||
|
||||
| Anti-pattern | Where it would have come from | Why we're avoiding it |
|
||||
|---|---|---|
|
||||
| `ProviderType` enum with wire-format suffix (`OPENAI_TRANSCRIPTIONS`, `OPENAI_CHAT_AUDIO`) | Earlier brainstorming draft | Vercel/LiteLLM/Go ecosystem all use vendor-level identity; capability is implied by which interface you call. |
|
||||
| `Response.Source` enum (`NativeSTT`, `MultimodalLLM`) | Earlier brainstorming draft | None of the three SDKs surveyed has this. It re-introduces the "pretend STT" smell at a different layer. |
|
||||
| Adapter wrapping `audiollm.Model` as `stt.Transcriber` | Earlier brainstorming draft | Adapter would re-create the conflation we're trying to remove. Application-layer dispatch is honest. |
|
||||
| Adding `transcription_*` fields to `AIProviderConfig` | Naive instinct | Three of three OSS apps surveyed (Open WebUI, LobeChat, Dify) do **not** do this. Pollutes the provider entity; repeats with every new capability. |
|
||||
| Silently reusing chat provider credentials for STT (LobeChat's deprecated pattern) | LobeChat-style shortcut | LobeChat's own author marked the helper `@deprecated`. Memos's existing `provider_id` reference is more flexible (user can configure a different OpenAI-compatible endpoint, e.g. Groq, just for STT). |
|
||||
| Per-model credential overrides, capability YAML, load balancing | Dify | Enterprise complexity that doesn't fit Memos's scope. |
|
||||
| Auto-fallback from STT failure to multimodal-LLM transcription | Plausible "smart" idea | LiteLLM doesn't do this; failure modes and cost differ enough that fallback would surprise users. Explicit dispatch by provider type is what LiteLLM ships. |
|
||||
|
||||
## 9. Open Decisions
|
||||
|
||||
All resolved during brainstorming. None remain open. For the record:
|
||||
|
||||
1. **Direction A (split STT/Audio-LLM into separate interfaces) over Direction B (capability-flag system) over Direction C (audio-to-structured-note pipeline).** Resolved: A. Rationale: most honest abstraction, matches mainstream SDKs, leaves the door open to C as a future addition without rework.
|
||||
2. **Provider type naming: vendor-level (`openai`/`gemini`) over wire-format-encoded.** Resolved: vendor-level. Rationale: matches Vercel/LiteLLM/Go convention; new OpenAI transcription model snapshots require zero schema or code changes.
|
||||
3. **`TranscriptionConfig.Duration` field decision.** Not present in current proto; not added. Audio duration belongs to resource metadata (computed from the file at upload time), not to the transcription response.
|
||||
4. **Multimodal failure-mode surface.** Resolved: expose `FinishReason` from `audiollm.Model` to the application layer; the Transcribe handler converts non-`Stop` reasons into informative errors.
|
||||
|
||||
## 10. Implementation Plan Pointer
|
||||
|
||||
Once this spec is approved, the implementation plan will be created at `docs/superpowers/plans/2026-05-02-stt-audiollm-split.md` covering Stages A–G from §7 above as discrete, bite-sized tasks with TDD steps and per-stage commits.
|
||||
@@ -0,0 +1,155 @@
|
||||
# Transcription (STT) settings — design
|
||||
|
||||
**Date:** 2026-05-02
|
||||
**Scope:** Backend + frontend. Schema-additive (no migration required).
|
||||
|
||||
## Problem
|
||||
|
||||
Memos has one AI feature today: audio transcription (speech-to-text). The current design has three concrete problems:
|
||||
|
||||
1. **Model is hard-coded per provider type.** `internal/ai/models.go` pins OpenAI to `gpt-4o-transcribe` and Gemini to `gemini-2.5-flash`. Users who want `whisper-1` (cheaper, often more accurate for non-English) or third-party Whisper-compatible endpoints (Groq's `whisper-large-v3-turbo`, self-hosted whisper.cpp / Speaches via OpenAI-compatible URL) cannot configure them at all.
|
||||
2. **No explicit transcription configuration.** `InstanceAISetting.providers` is a generic credentials list. The frontend (`MemoEditor/index.tsx:65`) implicitly picks "the first provider with an API key whose type is in TRANSCRIPTION_PROVIDER_TYPES." Users cannot:
|
||||
- Choose which provider runs transcription when they have multiple.
|
||||
- Set a default language (Whisper API supports it but it is never sent).
|
||||
- Set a `prompt` hint to bias spelling of proper nouns / jargon (a documented Whisper feature, surfaced by every other STT product).
|
||||
3. **Gemini fails for browser-recorded audio.** `internal/ai/gemini.go:23` does not list `audio/webm` in `geminiSupportedContentTypes`, but `MediaRecorder` in browsers defaults to `audio/webm`. So selecting a Gemini provider for in-editor recording produces a content-type error every time.
|
||||
|
||||
## Goal
|
||||
|
||||
Let the operator configure transcription explicitly: which provider, which model, default language, and a spelling-hint prompt. Make the OpenAI provider work as a universal "OpenAI-compatible" engine so Groq / self-hosted Whisper / Speaches are reachable through endpoint override.
|
||||
|
||||
## Non-goals
|
||||
|
||||
- Adding STT engines beyond OpenAI and Gemini (Azure, Deepgram, AWS Transcribe — out of scope; the schema admits them later via `AIProviderType` enum).
|
||||
- Other AI features (summarization, embeddings, tag suggestion). The schema is shaped so they fit later, but none are designed here.
|
||||
- Per-call provider override at recording time. Research across all surveyed products (OpenWebUI, LibreChat, Whisper Memos, Superwhisper, etc.) confirms STT engine is a global preference, not an action-time choice. We follow the same pattern.
|
||||
- Server-side audio transcoding (e.g., webm → wav for Gemini). See "Gemini webm" below for the chosen mitigation.
|
||||
- Multi-user or per-user override of admin defaults. Memos' STT setting is instance-scoped, like every other instance setting.
|
||||
|
||||
## Naming
|
||||
|
||||
Field and message names follow cross-platform STT conventions, not Memos-internal shorthand:
|
||||
|
||||
| Concept | Chosen name | Rationale |
|
||||
|---|---|---|
|
||||
| Config message | `TranscriptionConfig` | AssemblyAI uses this exact identifier; matches OpenAI's `CreateTranscription*` verb family and Memos' existing `Transcribe` RPC. The `STT` acronym is not used as a type name in any major STT API. |
|
||||
| Provider reference | `provider_id` (string) | Plain protobuf convention for a string-ID reference (`field_id`, `user_id` style). `engine` was rejected as an OpenWebUI-only term; typed message refs are not needed since providers are addressed by string ID. |
|
||||
| Model | `model` | Unanimous across OpenAI, Google v2, Deepgram, OpenWebUI, LibreChat. Not `model_id`. |
|
||||
| Default language | `language` | Bare `language` is the modern convention (OpenAI, Whisper family, Deepgram, Wyoming). `language_code` is the older Google/AWS form; we accept ISO 639-1 short codes the same way OpenAI does. |
|
||||
| Spelling hint | `prompt` | OpenAI's public API field name and AssemblyAI's. Whisper's internal name is `initial_prompt`, but `prompt` is what users of `audio.transcriptions.create` recognize. |
|
||||
|
||||
A note on the message name collision: `proto/api/v1/ai_service.proto` already declares a `TranscriptionConfig` for **per-call** prompt/language overrides. The new store-level `TranscriptionConfig` lives in package `memos.store`, so the two compile cleanly. Memos already uses parallel `api.v1.X` / `store.X` message pairs (e.g. `User`, `Memo`); this matches that pattern.
|
||||
|
||||
## Architecture
|
||||
|
||||
### Schema (additive)
|
||||
|
||||
`proto/store/instance_setting.proto`:
|
||||
|
||||
```proto
|
||||
message InstanceAISetting {
|
||||
repeated AIProviderConfig providers = 1; // unchanged — credential pool
|
||||
TranscriptionConfig transcription = 2; // NEW — feature config
|
||||
}
|
||||
|
||||
message TranscriptionConfig {
|
||||
// References an entry in providers[].id. Empty string = transcription disabled.
|
||||
string provider_id = 1;
|
||||
// Free text. Empty string = engine default (whisper-1 for OPENAI, gemini-2.5-flash for GEMINI).
|
||||
string model = 2;
|
||||
// ISO 639-1 short code. Empty string = auto-detect.
|
||||
string language = 3;
|
||||
// Up to ~200 tokens. Used as the OpenAI Whisper `prompt` parameter and as
|
||||
// a "Context and spelling hints:" block in the Gemini prompt.
|
||||
string prompt = 4;
|
||||
}
|
||||
```
|
||||
|
||||
`proto/api/v1/ai_service.proto`:
|
||||
|
||||
- `TranscribeRequest.provider_id` becomes optional. When omitted, the server resolves the provider from `InstanceAISetting.transcription.provider_id`.
|
||||
- `TranscribeRequest.config` (per-call `TranscriptionConfig` with `prompt` / `language`) is kept for advanced overrides but its fields, when empty, fall back to the persisted defaults from `InstanceAISetting.transcription`.
|
||||
|
||||
### Backend changes
|
||||
|
||||
1. **`internal/ai/models.go`** — `DefaultTranscriptionModel` already exists; reuse it as the fallback when `TranscriptionConfig.model` is empty. No new code, just used from a new call site.
|
||||
2. **`server/router/api/v1/ai_service.go`**:
|
||||
- Read `InstanceAISetting.transcription` at the start of `Transcribe`.
|
||||
- Resolve `provider_id` from request → fall back to `transcription.provider_id`. If both empty, return `FailedPrecondition` with a clear "transcription not configured" message.
|
||||
- Resolve `model` similarly: request override → `transcription.model` → engine default via `DefaultTranscriptionModel`.
|
||||
- Merge `language` and `prompt`: per-call overrides win; otherwise fall through to persisted defaults.
|
||||
3. **`internal/ai/gemini.go`** — out of scope to fix the webm content-type list here. See mitigation below.
|
||||
|
||||
### Frontend changes
|
||||
|
||||
`web/src/components/Settings/AISection.tsx` is restructured into two settings groups inside the existing `SettingSection`:
|
||||
|
||||
1. **AI Integrations** (renamed from "Providers" — current behavior): list of credential entries (id, title, type, endpoint, api key). No functional changes; the rename communicates that this section is just credentials.
|
||||
2. **Transcription** (new): three-segment form
|
||||
- **Provider** — Select dropdown listing entries from group 1 by `title`. First option is "None — transcription disabled". Disabled with a hint "Add an AI integration first ↑" when group 1 is empty.
|
||||
- **Model** — text input. Placeholder updates dynamically based on the selected provider's type (`whisper-1` for OPENAI, `gemini-2.5-flash` for GEMINI). Help text below: "Free text. Use the provider's model identifier — e.g., whisper-1, gpt-4o-transcribe, whisper-large-v3-turbo."
|
||||
- **Default language** — text input, ISO 639-1 placeholder, empty = auto.
|
||||
- **Prompt hints** — textarea, ~200 token soft limit, help text "Improves spelling of proper nouns and jargon. Whisper limit is ~224 tokens."
|
||||
|
||||
`web/src/components/MemoEditor/index.tsx:65` changes:
|
||||
|
||||
- Replace the "first provider with apiKey in TRANSCRIPTION_PROVIDER_TYPES" lookup with this enable rule: transcribe button shows iff `aiSetting.transcription.providerId` is non-empty AND the referenced provider exists in `aiSetting.providers` AND that provider has `apiKeySet === true`.
|
||||
- The editor no longer needs to know the provider object itself for the call — see service change below.
|
||||
|
||||
`web/src/components/MemoEditor/services/transcriptionService.ts` is simplified: it stops accepting a `provider` argument and simply omits `provider_id` from the request. The server resolves the provider, model, language, and prompt from `InstanceAISetting.transcription`. (No override path is exposed at the editor layer; advanced callers can still pass `provider_id` directly via the proto if needed in the future.)
|
||||
|
||||
### How "OpenAI-compatible" backends work
|
||||
|
||||
To use Groq, Speaches, or self-hosted whisper.cpp:
|
||||
|
||||
1. In **AI Integrations**, add a provider with type `OPENAI`, set `endpoint` to e.g. `https://api.groq.com/openai/v1` or `http://speaches:8000/v1`, set the API key, give it a recognizable title ("Groq", "Self-hosted Whisper").
|
||||
2. In **Transcription**, select that provider and set `model` to the backend's model identifier (`whisper-large-v3-turbo`, `Systran/faster-distil-whisper-large-v3`, etc.).
|
||||
|
||||
This is the universal escape hatch confirmed across OpenWebUI, LibreChat, and Whisper Obsidian plugin: don't enumerate every backend — let the OpenAI engine be a transport, not a brand.
|
||||
|
||||
## Gemini webm mitigation
|
||||
|
||||
The Gemini `audio/webm` failure is a real user-blocking bug but separate from the settings redesign. Three options were considered:
|
||||
|
||||
- **(a) Server-side transcode** with ffmpeg. Adds a heavy runtime dep; rejected as YAGNI.
|
||||
- **(b) Switch MediaRecorder format** when STT engine is Gemini. Browser support for `audio/mp4` and `audio/wav` in `MediaRecorder` is patchy across Firefox / Safari / Chrome; rejected as fragile.
|
||||
- **(c) Inline hint + accept the limitation.** Selected. The Transcription section shows a small warning under the model field when the chosen provider type is `GEMINI`: "Gemini does not accept browser-recorded `audio/webm`. For in-editor recording, use an OpenAI-compatible provider."
|
||||
|
||||
Server-side transcoding can be revisited later as a self-contained change if Gemini demand grows.
|
||||
|
||||
## Validation
|
||||
|
||||
Server validation (`server/router/api/v1/ai_service.go`):
|
||||
|
||||
- `transcription.provider_id`, when set, must reference an existing entry in `providers[]`. On `UpdateInstanceSetting` for the AI key, reject with `InvalidArgument` if it doesn't.
|
||||
- `transcription.model` length cap: 256 chars (covers `Systran/faster-distil-whisper-large-v3`-style names with margin).
|
||||
- `transcription.language` length cap: 32 chars (existing constant `maxTranscriptionLanguageLength`).
|
||||
- `transcription.prompt` length cap: 4096 chars (existing constant `maxTranscriptionPromptLength`).
|
||||
|
||||
Frontend validation in `AISection.tsx`:
|
||||
|
||||
- "Save" disabled if `transcription.providerId` is set but the referenced provider was just deleted from the integrations list (in the same unsaved edit).
|
||||
- Inline warning shown (but Save still allowed) if the referenced provider exists but has `apiKeySet === false` — surfacing the broken state so the operator can fix it without blocking unrelated edits to other settings.
|
||||
|
||||
## Backwards compatibility
|
||||
|
||||
The schema change is purely additive. Existing instances with `providers` configured but no `transcription` field default to `provider_id = ""`, which means transcription is disabled until the operator visits the new Transcription section and selects a provider.
|
||||
|
||||
This is a small UX regression for instances that were relying on the implicit "first provider wins" behavior — they now must make a one-click selection. Acceptable trade-off because:
|
||||
|
||||
- It makes the choice explicit (the implicit pick was the source of confusion when users had multiple providers).
|
||||
- A one-time migration that auto-fills `transcription.provider_id` with the first STT-capable provider is feasible but adds complexity for a one-line user action. Skip the migration; document the change in the release notes.
|
||||
|
||||
## Testing
|
||||
|
||||
- `internal/ai/transcription_test.go` (existing) covers the transcribe RPC. Add cases for: empty `provider_id` falls back to setting; empty `model` falls back to `DefaultTranscriptionModel`; per-call overrides win over settings.
|
||||
- `server/router/api/v1/test/ai_service_test.go` (existing) covers the API service. Add cases for the validation rules above (unknown provider_id, oversized model/language/prompt).
|
||||
- Frontend: manual verification via the dev server (`pnpm dev` in `web/`) — load Settings, add a provider, configure transcription, verify the home editor's record button enables/disables based on `provider_id`. No new component tests required (existing AISection has none).
|
||||
|
||||
## Out of scope, explicitly
|
||||
|
||||
- Multiple transcription configurations / per-tag or per-user routing.
|
||||
- Per-call provider override exposed in the editor UI.
|
||||
- Test-transcription button in settings (worth doing later; deferred to keep this scope tight).
|
||||
- Glossary / vocabulary list as a separate field — folded into `prompt` for now (Joplin/Superwhisper split this; we can add later if users ask).
|
||||
- TTS settings. Memos has none today and none planned.
|
||||
@@ -0,0 +1,113 @@
|
||||
# First Screen Lazy Heavy Dependencies Design
|
||||
|
||||
## Context
|
||||
|
||||
The auth and signup pages currently fetch JavaScript and CSS assets for features that are not used on the first screen, including Mermaid, KaTeX, Leaflet, and React Leaflet. The current routing already uses lazy route components, so the remaining problem is eager imports from shared app entry points and feature modules.
|
||||
|
||||
The goal is to reduce first screen load time, especially for `/auth` and `/auth/signup`, without changing memo rendering, map behavior, or authenticated workflows.
|
||||
|
||||
## Goals
|
||||
|
||||
- Prevent Mermaid and Leaflet vendor chunks from loading on auth/signup before they are needed.
|
||||
- Prevent Leaflet and KaTeX CSS from loading globally at app startup.
|
||||
- Preserve current behavior when users view Mermaid diagrams, math content, memo location previews, profile maps, or location pickers.
|
||||
- Keep fallbacks small and consistent with existing async rendering patterns.
|
||||
- Verify the production build and confirm auth/signup network requests no longer include Mermaid or Leaflet chunks during initial render.
|
||||
|
||||
## Non-Goals
|
||||
|
||||
- Rewriting the markdown rendering pipeline.
|
||||
- Removing support for Mermaid, KaTeX, Leaflet, or React Leaflet.
|
||||
- Optimizing every authenticated route in this change.
|
||||
- Changing server behavior, route guards, or authentication flow.
|
||||
|
||||
## Recommended Approach
|
||||
|
||||
Use feature-level lazy loading for heavy optional features. Keep the app shell and auth routes free of diagram, math styling, and map dependencies. Load those dependencies from the feature boundary where the user actually needs them.
|
||||
|
||||
This approach has the best balance of impact and risk because it removes known eager imports while preserving existing route structure and feature internals.
|
||||
|
||||
## Architecture
|
||||
|
||||
### App Entry
|
||||
|
||||
`web/src/main.tsx` should no longer import:
|
||||
|
||||
- `leaflet/dist/leaflet.css`
|
||||
- `katex/dist/katex.min.css`
|
||||
|
||||
These styles are feature-specific and should be loaded from the map and markdown rendering paths.
|
||||
|
||||
### Mermaid
|
||||
|
||||
`web/src/components/MemoContent/MermaidBlock.tsx` should replace the static `import mermaid from "mermaid"` with an async `import("mermaid")` inside the render effect.
|
||||
|
||||
The component should keep its current behavior:
|
||||
|
||||
- initialize Mermaid with the resolved app theme;
|
||||
- render when code content or theme changes;
|
||||
- show the existing error fallback when rendering fails.
|
||||
|
||||
The Mermaid chunk should only be requested when a memo actually renders a Mermaid code block.
|
||||
|
||||
### KaTeX
|
||||
|
||||
KaTeX CSS should load from the memo markdown rendering path instead of the app entry. Since `rehype-katex` is only useful when memo markdown is rendered, loading the stylesheet near `MemoMarkdownRenderer` keeps auth/signup free of KaTeX CSS while preserving math output styling.
|
||||
|
||||
This change does not need content-level math detection. Loading KaTeX CSS with memo markdown is simpler and still removes it from the first auth/signup screen.
|
||||
|
||||
### Leaflet Maps
|
||||
|
||||
Leaflet-dependent UI should be moved behind lazy component boundaries:
|
||||
|
||||
- `UserMemoMap` should be lazy-loaded by the user profile route or by a small wrapper component.
|
||||
- `LocationPicker` should be lazy-loaded where location UI is opened or displayed.
|
||||
|
||||
The underlying map implementations can continue using Leaflet, React Leaflet, marker clustering, and their current helpers. The key boundary is that parent components must not statically import the map implementation if that parent can be pulled into non-map first-screen chunks.
|
||||
|
||||
Leaflet CSS and marker cluster CSS should load inside the lazy map implementation path, not from `main.tsx`.
|
||||
|
||||
### Type Imports
|
||||
|
||||
Any imports from `leaflet` that are used only as TypeScript types should use `import type`. Runtime construction such as `new LatLng(...)` should be avoided in parent components that are meant to stay Leaflet-free; pass plain latitude/longitude data into lazy map wrappers and construct Leaflet objects inside the lazy implementation.
|
||||
|
||||
## Data Flow
|
||||
|
||||
Auth/signup initial render:
|
||||
|
||||
1. App entry initializes theme, locale, providers, auth, and instance data.
|
||||
2. Router loads only the auth/signup route component and shared app shell dependencies.
|
||||
3. Mermaid, Leaflet, React Leaflet, marker cluster, and feature CSS are not requested.
|
||||
|
||||
Memo markdown render:
|
||||
|
||||
1. Memo content renders with the existing markdown renderer.
|
||||
2. KaTeX CSS loads with the markdown rendering path.
|
||||
3. If a code block language is `mermaid`, `MermaidBlock` dynamically imports Mermaid and renders the diagram.
|
||||
|
||||
Map render:
|
||||
|
||||
1. A map feature mounts through a lazy boundary.
|
||||
2. The lazy implementation imports Leaflet, React Leaflet, and required map CSS.
|
||||
3. Existing map interactions and display behavior continue inside the loaded implementation.
|
||||
|
||||
## Error Handling
|
||||
|
||||
- Mermaid import or render failures should use the existing Mermaid error UI with the original code content visible.
|
||||
- Lazy map boundaries should use minimal fallbacks sized like the eventual map container to avoid layout shift.
|
||||
- Chunk load failures should continue to use the existing router chunk reload behavior where applicable.
|
||||
|
||||
## Testing
|
||||
|
||||
- Run `pnpm build` in `web`.
|
||||
- Run `pnpm lint` in `web`.
|
||||
- Inspect production build output to ensure Mermaid and Leaflet remain split chunks.
|
||||
- Use a production preview or equivalent browser check for `/auth/signup` and confirm initial network requests do not include Mermaid or Leaflet JavaScript chunks.
|
||||
- Smoke test memo content with Mermaid and math.
|
||||
- Smoke test location picker, location popover, and user profile map.
|
||||
|
||||
## Risks
|
||||
|
||||
- Loading CSS from lazy paths can cause a small style delay the first time a map or math content appears. Use map-sized fallbacks and keep CSS imports in the feature implementation to minimize visible shifts.
|
||||
- Moving Leaflet runtime types out of parent components may require small prop shape changes.
|
||||
- Dynamic Mermaid import needs effect cancellation to avoid setting state after unmount.
|
||||
@@ -0,0 +1,282 @@
|
||||
# Placeholder Component Design
|
||||
|
||||
## Context
|
||||
|
||||
The web frontend currently renders empty states with a single `Empty.tsx` component that displays a lucide `BirdIcon`. It is used in exactly one place (`web/src/pages/Inboxes.tsx`) and offers no support for other state varieties — loading, no-search-results, or 404 pages — each of which is currently handled inconsistently or not at all.
|
||||
|
||||
The goal is to replace `Empty.tsx` with a single reusable `<Placeholder>` component that renders a hand-curated ASCII bird illustration plus a short muted message, supports four distinct variants, and ships with the architectural seam needed to grow a randomized pool of ASCII pieces per variant in future work.
|
||||
|
||||
The visual register is "cozy and minimal": muted colors, monospace throughout, gentle motion that respects `prefers-reduced-motion`. The ASCII art itself is drawn from Joan Stark's (jgs) classic ASCII bird collection (https://github.com/oldcompcz/jgs), preserving the `jgs` signature and date as a visible credit beneath each piece.
|
||||
|
||||
## Goals
|
||||
|
||||
- Provide a single `<Placeholder variant="…">` component covering empty, loading, no-results, and 404 states.
|
||||
- Render real, recognizable ASCII bird art (not freehand sketches), preserving Joan Stark's attribution.
|
||||
- Apply subtle CSS-only animation (bob for perched birds, flutter for in-flight birds, fade-in on the message).
|
||||
- Respect `prefers-reduced-motion` — no animation when the user opts out.
|
||||
- Provide a pool-shaped data file so future PRs can drop in additional ASCII pieces per variant without component changes.
|
||||
- Replace the existing `Empty.tsx` in `Inboxes.tsx` as the proof of integration.
|
||||
|
||||
## Non-Goals
|
||||
|
||||
- Adding additional ASCII pieces beyond the four initial picks (one per variant). The pool architecture supports growth, but seeding more pieces is a follow-up.
|
||||
- Wiring the component into search results, the 404 route, or Suspense fallbacks. Each is a candidate; each is a separate PR.
|
||||
- Translating the default messages. The file structure leaves a seam for `i18next`, but the initial PR ships plain strings.
|
||||
- Adding a JS animation library (Framer Motion, react-spring, etc.). Three keyframe animations do not justify a new dependency.
|
||||
- Visual regression testing infrastructure.
|
||||
|
||||
## Recommended Approach
|
||||
|
||||
Build a single `<Placeholder>` component with a `variant` prop that selects a randomized ASCII piece from a co-located pool data file. Animation is CSS-only, with motion presets keyed off each piece's `motion` field. Accessibility uses `aria-hidden` on the decorative ASCII and a semantic `<p>` for the message.
|
||||
|
||||
This approach has the best balance of present-day simplicity and future extensibility: the initial pool contains one piece per variant, so the component is deterministic today, but the picker function and pool shape impose no constraints on how many pieces are added later.
|
||||
|
||||
## Architecture
|
||||
|
||||
### Public Component
|
||||
|
||||
**Path:** `web/src/components/Placeholder/index.tsx`
|
||||
|
||||
```tsx
|
||||
import { useMemo } from "react";
|
||||
import clsx from "clsx";
|
||||
import { pickPiece, MotionStyle, PlaceholderVariant } from "./ascii-pool";
|
||||
import { DEFAULT_MESSAGES } from "./messages";
|
||||
import "./Placeholder.css";
|
||||
|
||||
interface PlaceholderProps {
|
||||
variant: PlaceholderVariant;
|
||||
message?: string;
|
||||
children?: React.ReactNode;
|
||||
className?: string;
|
||||
}
|
||||
|
||||
const MOTION_CLASS: Record<MotionStyle, string> = {
|
||||
bob: "placeholder-motion-bob",
|
||||
flutter: "placeholder-motion-flutter",
|
||||
none: "",
|
||||
};
|
||||
|
||||
export function Placeholder({ variant, message, children, className }: PlaceholderProps) {
|
||||
const piece = useMemo(() => pickPiece(variant), [variant]);
|
||||
const resolvedMessage = message ?? DEFAULT_MESSAGES[variant];
|
||||
const isLoading = variant === "loading";
|
||||
|
||||
return (
|
||||
<div
|
||||
role={isLoading ? "status" : undefined}
|
||||
aria-live={isLoading ? "polite" : undefined}
|
||||
className={clsx("flex flex-col items-center justify-center max-w-md mx-auto px-4 py-8", className)}
|
||||
>
|
||||
<pre
|
||||
aria-hidden="true"
|
||||
className={clsx(
|
||||
"font-mono text-xs sm:text-sm leading-tight text-muted-foreground whitespace-pre",
|
||||
MOTION_CLASS[piece.motion],
|
||||
)}
|
||||
>
|
||||
{piece.ascii}
|
||||
</pre>
|
||||
<p className="mt-3 font-mono text-sm text-muted-foreground placeholder-fade-in">
|
||||
{resolvedMessage}
|
||||
</p>
|
||||
<p className="mt-1 font-mono text-[10px] text-muted-foreground/60">{piece.credit}</p>
|
||||
{children && <div className="mt-4">{children}</div>}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
```
|
||||
|
||||
The `useMemo` keyed on `variant` ensures the piece is stable for the mount but re-rolls if the variant prop changes (rare in practice). Re-mounting re-rolls naturally.
|
||||
|
||||
### ASCII Pool
|
||||
|
||||
**Path:** `web/src/components/Placeholder/ascii-pool.ts`
|
||||
|
||||
```ts
|
||||
export type PlaceholderVariant = "empty" | "loading" | "noResults" | "notFound";
|
||||
export type MotionStyle = "bob" | "flutter" | "none";
|
||||
|
||||
export interface AsciiPiece {
|
||||
id: string;
|
||||
variant: PlaceholderVariant;
|
||||
ascii: string;
|
||||
credit: string;
|
||||
motion: MotionStyle;
|
||||
}
|
||||
|
||||
export const ASCII_POOL: AsciiPiece[] = [
|
||||
{
|
||||
id: "jgs-crested-parrot",
|
||||
variant: "empty",
|
||||
credit: "jgs · 4/97",
|
||||
motion: "bob",
|
||||
ascii: ` .---.
|
||||
/ 6_6
|
||||
\\_ (__\\
|
||||
// \\\\
|
||||
(( ))
|
||||
=====""===""=====
|
||||
|||
|
||||
|`,
|
||||
},
|
||||
{
|
||||
id: "jgs-hummingbird-sm",
|
||||
variant: "loading",
|
||||
credit: "jgs · 7/98",
|
||||
motion: "flutter",
|
||||
ascii: ` , _
|
||||
{ \\/\`o;====-
|
||||
.----'-/\`-/
|
||||
\`'-..-| /
|
||||
/\\/\\
|
||||
\`--\``,
|
||||
},
|
||||
{
|
||||
id: "jgs-wide-eyed-owl",
|
||||
variant: "noResults",
|
||||
credit: "jgs · 2/01",
|
||||
motion: "bob",
|
||||
ascii: ` __ __
|
||||
\\ \`-'"'-\` /
|
||||
/ \\_ _/ \\
|
||||
| d\\_/b |
|
||||
.'\\ V /'.
|
||||
/ '-...-' \\
|
||||
| / \\ |
|
||||
\\/\\ /\\/
|
||||
==(||)---(||)==`,
|
||||
},
|
||||
{
|
||||
id: "jgs-bird-flown-away",
|
||||
variant: "notFound",
|
||||
credit: "jgs · 7/96",
|
||||
motion: "flutter",
|
||||
ascii: ` ___
|
||||
_,-' ______
|
||||
.' .-' ____7
|
||||
/ / ___7
|
||||
_| / ___7
|
||||
>(')\\ | ___7
|
||||
\\\\/ \\_______
|
||||
' _======>
|
||||
\`'----\\\\\``,
|
||||
},
|
||||
];
|
||||
|
||||
export function pickPiece(variant: PlaceholderVariant): AsciiPiece {
|
||||
const matches = ASCII_POOL.filter(p => p.variant === variant);
|
||||
return matches[Math.floor(Math.random() * matches.length)];
|
||||
}
|
||||
```
|
||||
|
||||
> Each `ascii` value is a template literal; backslashes and backticks are escaped per JS rules. The art shown here is verbatim from the brainstorm sign-off; implementation must preserve every space.
|
||||
|
||||
The `ascii` field of each entry holds the verbatim ASCII string. The four initial pieces are the ones validated during brainstorming:
|
||||
|
||||
- **empty** — Joan Stark's "early-bird" parrot with crest, perched on a branch
|
||||
- **loading** — Joan Stark's compact hummingbird (mid-flight, fluttering)
|
||||
- **noResults** — Joan Stark's wide-eyed two-feathered owl
|
||||
- **notFound** — Joan Stark's flying-away bird with motion trails
|
||||
|
||||
Each piece's `ascii` is committed as a template literal preserving exact whitespace; the file is the source of truth and is small enough (~5–10 lines of art per piece) to keep inline rather than splitting into per-piece files.
|
||||
|
||||
### Default Messages
|
||||
|
||||
**Path:** `web/src/components/Placeholder/messages.ts`
|
||||
|
||||
```ts
|
||||
export const DEFAULT_MESSAGES: Record<PlaceholderVariant, string> = {
|
||||
empty: "No memos yet",
|
||||
loading: "Loading…",
|
||||
noResults: "Nothing matches that search",
|
||||
notFound: "This page flew the coop",
|
||||
};
|
||||
```
|
||||
|
||||
Plain strings for the initial PR. A future i18n pass swaps these for `t("placeholder.empty")` etc. without touching the component.
|
||||
|
||||
### Animation
|
||||
|
||||
CSS keyframes live in a small co-located stylesheet (`Placeholder.css`) imported by `index.tsx`, scoped via a `.placeholder-…` class prefix to avoid leakage. The codebase uses Tailwind v4 plus plain CSS imports elsewhere; this matches the existing pattern.
|
||||
|
||||
```css
|
||||
@media (prefers-reduced-motion: no-preference) {
|
||||
.placeholder-motion-bob { animation: placeholder-bob 3.4s ease-in-out infinite; }
|
||||
.placeholder-motion-flutter { animation: placeholder-flutter 0.7s ease-in-out infinite; }
|
||||
.placeholder-fade-in { animation: placeholder-fade 1s ease-out 0.3s both; opacity: 0; }
|
||||
}
|
||||
|
||||
@keyframes placeholder-bob {
|
||||
0%, 100% { transform: translateY(0); }
|
||||
50% { transform: translateY(-4px); }
|
||||
}
|
||||
|
||||
@keyframes placeholder-flutter {
|
||||
0%, 100% { transform: translate(0, 0); }
|
||||
50% { transform: translate(2px, -1px); }
|
||||
}
|
||||
|
||||
@keyframes placeholder-fade {
|
||||
to { opacity: 1; }
|
||||
}
|
||||
```
|
||||
|
||||
The reduced-motion guard wraps all three rules. When the user prefers reduced motion, the bird is static and the message appears without fading.
|
||||
|
||||
### Accessibility
|
||||
|
||||
- ASCII `<pre>` carries `aria-hidden="true"`; screen readers do not announce it character-by-character.
|
||||
- The message is a semantic `<p>`. It is the accessible name of the placeholder for assistive tech.
|
||||
- For `variant="loading"` only, the wrapper has `role="status"` and `aria-live="polite"` so the loading message is announced when the placeholder appears.
|
||||
- The credit line (`jgs · 4/97`) is visible-but-small below the message. It is not aria-hidden because it is intentionally part of the visual presentation.
|
||||
- The component itself is not focusable. Anything passed as `children` (e.g. a "Go home" button on 404) participates normally in tab order.
|
||||
- Color contrast relies on the existing `text-muted-foreground` token, which already meets WCAG AA in the project's theme.
|
||||
|
||||
### File Layout
|
||||
|
||||
```
|
||||
web/src/components/Placeholder/
|
||||
index.tsx # the <Placeholder> component (public export)
|
||||
Placeholder.css # keyframes + .placeholder-motion-* classes
|
||||
ascii-pool.ts # AsciiPiece type, ASCII_POOL array, pickPiece()
|
||||
messages.ts # DEFAULT_MESSAGES map (i18n-ready seam)
|
||||
CREDITS.md # Joan Stark attribution + link to oldcompcz/jgs
|
||||
```
|
||||
|
||||
### Integration
|
||||
|
||||
- **Delete:** `web/src/components/Empty.tsx`.
|
||||
- **Modify:** `web/src/pages/Inboxes.tsx` — replace the `<Empty />` import and usage with `<Placeholder variant="empty" />`.
|
||||
|
||||
Other potential call sites (search results page, router 404 catch-all, Suspense fallbacks) are explicitly out of scope for this PR. They are noted in the PR description as follow-up opportunities.
|
||||
|
||||
### Credits
|
||||
|
||||
**Path:** `web/src/components/Placeholder/CREDITS.md`
|
||||
|
||||
A short Markdown file pointing to https://github.com/oldcompcz/jgs and acknowledging Joan Stark's work. This survives even if the per-piece `credit` field is ever cleaned up by a refactor.
|
||||
|
||||
## Testing
|
||||
|
||||
A small Vitest suite covers:
|
||||
|
||||
- Each variant renders without throwing.
|
||||
- The chosen `<pre>` content contains text from the matching pool entry.
|
||||
- `aria-hidden="true"` on the `<pre>`.
|
||||
- `role="status"` only present when `variant="loading"`.
|
||||
- A custom `message` prop overrides the default.
|
||||
- The credit text is present in the DOM.
|
||||
|
||||
No visual regression testing is added.
|
||||
|
||||
## Open Questions
|
||||
|
||||
None. All design decisions were validated during the brainstorming session — animation register (cozy), bird selections (jgs collection), naming (`Placeholder`, no "bird" in the name), text treatment (plain muted monospace), and integration scope (replace `Empty.tsx`, do not wire other sites yet).
|
||||
|
||||
## References
|
||||
|
||||
- Joan Stark's ASCII Art Gallery (jgs collection): https://github.com/oldcompcz/jgs
|
||||
- Existing component being replaced: `web/src/components/Empty.tsx`
|
||||
- Existing call site: `web/src/pages/Inboxes.tsx`
|
||||
- Visual brainstorm artifacts: `.superpowers/brainstorm/1991-1778593581/content/` (mascot-approach, animation-vibe, bird-shapes-v2, jgs-bird-set, text-treatment-c)
|
||||
@@ -0,0 +1,82 @@
|
||||
# Remove react-use Design
|
||||
|
||||
## Context
|
||||
|
||||
The frontend has a direct dependency on `react-use@17.6.0`. Current source usage is limited to six imports:
|
||||
|
||||
- `usePrevious` in `web/src/layouts/RootLayout.tsx`
|
||||
- `useLocalStorage` in `web/src/components/MemoExplorer/TagsSection.tsx`
|
||||
- `useWindowScroll` in `web/src/components/MobileHeader.tsx`
|
||||
- `useToggle` in `web/src/components/TagTree.tsx`
|
||||
- `useDebounce` in `web/src/components/MemoEditor/Toolbar/InsertMenu.tsx`
|
||||
- `useDebounce` in `web/src/components/MemoEditor/hooks/useLinkMemo.ts`
|
||||
|
||||
`react-use` pulls in `js-cookie@2.2.1` transitively. A direct `js-cookie@3.0.7` dependency does not remove that vulnerable transitive copy, so the better remediation is to remove `react-use` rather than adding another copy of `js-cookie`.
|
||||
|
||||
## Goals
|
||||
|
||||
- Remove `react-use` from `web/package.json` and `web/pnpm-lock.yaml`.
|
||||
- Remove all source imports from `react-use` and `react-use/lib/*`.
|
||||
- Prefer built-in React hooks directly where the replacement is simple.
|
||||
- Add local hooks only where reuse or browser-side-effect cleanup makes the code clearer and safer.
|
||||
- Confirm `react-use` and `js-cookie` are no longer present in the frontend dependency graph.
|
||||
|
||||
## Non-Goals
|
||||
|
||||
- Do not introduce another general-purpose React hooks library.
|
||||
- Do not refactor unrelated frontend state management.
|
||||
- Do not change user-visible behavior of tag preferences, tag tree expansion, scroll shadow behavior, route filter clearing, memo link search, or reverse geocoding debounce.
|
||||
|
||||
## Approach
|
||||
|
||||
Use native React hooks for one-off, simple behavior:
|
||||
|
||||
- Replace `useToggle` in `TagTree` with `useState(false)` plus local callbacks.
|
||||
- Replace `usePrevious` in `RootLayout` with local `useRef` and `useEffect` while preserving the existing previous-path comparison.
|
||||
- Replace `useWindowScroll` in `MobileHeader` with local `useState` and `useEffect` for the scroll listener.
|
||||
|
||||
Create focused local hooks where repeated behavior or cleanup consistency matters:
|
||||
|
||||
- Add `useDebouncedEffect` under `web/src/hooks/` for the two debounce call sites. It schedules a callback after the configured delay and clears the timeout when dependencies change or the component unmounts.
|
||||
- Add a typed `useLocalStorage` under `web/src/hooks/` for tag display settings. It reads the stored value on initialization, falls back to the provided default, writes updates to `localStorage`, and tolerates unavailable or malformed storage by using the default.
|
||||
|
||||
Update imports to use `@/hooks/...` for local hooks. Keep hook APIs small and shaped around current usage rather than cloning the full `react-use` API.
|
||||
|
||||
## Data Flow And Behavior
|
||||
|
||||
Tag view settings continue to persist in `localStorage` under the same keys:
|
||||
|
||||
- `tag-view-as-tree`
|
||||
- `tag-tree-auto-expand`
|
||||
|
||||
Debounced effects continue to defer:
|
||||
|
||||
- reverse-geocoding position updates in the memo editor insert menu by 1000 ms
|
||||
- memo link search requests by 300 ms
|
||||
|
||||
The route filter clearing logic continues to run only when navigation changes route and the URL has no `filter` parameter.
|
||||
|
||||
The mobile header continues to show its shadow when the window has scrolled below the top.
|
||||
|
||||
## Error Handling
|
||||
|
||||
The local storage hook catches read and write failures. On read failure or malformed JSON, it returns the default value. On write failure, it keeps React state updated so the current UI interaction still works, while avoiding a thrown render or event-handler error.
|
||||
|
||||
Debounced effects do not swallow errors inside the user callback. Existing call sites already handle their own asynchronous errors where needed.
|
||||
|
||||
## Testing And Verification
|
||||
|
||||
Run frontend validation after implementation:
|
||||
|
||||
- `pnpm install --lockfile-only` from `web/` to regenerate `pnpm-lock.yaml`
|
||||
- `pnpm why react-use js-cookie` from `web/` and confirm neither dependency remains
|
||||
- `pnpm lint` from `web/`
|
||||
- `pnpm build` from `web/`
|
||||
|
||||
Manual checks should cover:
|
||||
|
||||
- toggling tag tree mode and auto-expand persists across reload
|
||||
- opening tag tree nodes still works
|
||||
- mobile header shadow appears after scrolling
|
||||
- memo link search still debounces and updates results
|
||||
- location reverse geocoding still waits for position changes before lookup
|
||||
@@ -0,0 +1,241 @@
|
||||
# OpenAPI-Driven MCP Support Design
|
||||
|
||||
## Context
|
||||
|
||||
Memos previously had an MCP server, but it was removed in commit `2a4638b3` and should not be used as the baseline for this work. GitHub issue #6022 reported that some old MCP tools returned a bare JSON array in `result.structuredContent`, which strict MCP clients reject because `structuredContent` must be an object.
|
||||
|
||||
The new MCP support should start from zero and use the generated OpenAPI document at `proto/gen/openapi.yaml` as the source of truth. The OpenAPI file already describes the public REST gateway operations generated from protobuf definitions, including operation IDs, descriptions, parameters, request schemas, and response schemas.
|
||||
|
||||
## Goals
|
||||
|
||||
- Add MCP support back through a new implementation that is mechanically tied to `proto/gen/openapi.yaml`.
|
||||
- Expose a standard MCP Streamable HTTP endpoint at the `/mcp` path.
|
||||
- Register tools only; do not add MCP resources or prompts in the first version.
|
||||
- Expose a curated memo-focused toolset derived from OpenAPI operations, not every API endpoint.
|
||||
- Execute MCP tool calls through the existing API contract instead of duplicating store or service logic.
|
||||
- Return object-shaped `structuredContent` for every tool result.
|
||||
- Keep authentication and authorization behavior aligned with the existing API.
|
||||
|
||||
## Non-Goals
|
||||
|
||||
- Reviving or adapting the removed MCP package design.
|
||||
- Adding custom MCP route aliases, readonly endpoints, or toolset filtering headers.
|
||||
- Exposing every operation in `proto/gen/openapi.yaml` as an MCP tool.
|
||||
- Adding MCP tools for admin settings, users, identity providers, webhooks, personal access tokens, authentication, share-link management, AI transcription, or bulk deletion.
|
||||
- Adding `list_tags` or `search_memos` unless matching proto/API operations are added and OpenAPI is regenerated.
|
||||
- Hand-editing generated OpenAPI or generated protobuf outputs.
|
||||
|
||||
## Recommended Approach
|
||||
|
||||
Build a new `server/router/mcp` package that parses `proto/gen/openapi.yaml` at startup, selects a curated allowlist of operation IDs, and converts those selected OpenAPI operations into MCP tools. Tool calls map MCP arguments into the selected operation's path parameters, query parameters, and JSON body, then execute the corresponding `/api/v1/...` HTTP request through the existing Echo/gRPC-Gateway route in-process.
|
||||
|
||||
This approach keeps OpenAPI as the authoritative contract while avoiding the usability and safety problems of exposing a large API-mirrored tool surface.
|
||||
|
||||
## Architecture
|
||||
|
||||
### MCP Service
|
||||
|
||||
Create a new MCP service package under `server/router/mcp`. `server.NewServer` creates the existing `APIV1Service`, registers the normal file, RSS, and API routes, then registers a new MCP service against the same Echo server.
|
||||
|
||||
The service exposes one MCP endpoint path:
|
||||
|
||||
```text
|
||||
/mcp
|
||||
```
|
||||
|
||||
The implementation must support standard Streamable HTTP client messages on `POST /mcp`. It may also support `GET /mcp` and `DELETE /mcp` if the chosen MCP transport implementation requires those methods for standards-compliant streaming or session cleanup.
|
||||
|
||||
The first version advertises only the MCP tools capability. It does not advertise prompts or resources.
|
||||
|
||||
### OpenAPI Operation Registry
|
||||
|
||||
At startup, the MCP service loads `proto/gen/openapi.yaml` and builds an operation registry keyed by `operationId`. Each parsed operation stores:
|
||||
|
||||
- operation ID
|
||||
- HTTP method
|
||||
- OpenAPI route template
|
||||
- description
|
||||
- path parameters
|
||||
- query parameters
|
||||
- JSON request body schema
|
||||
- HTTP 200 JSON response schema
|
||||
|
||||
The parser should fail fast during service construction if any curated operation ID is missing or cannot be converted into a valid MCP tool schema.
|
||||
|
||||
### Tool Names
|
||||
|
||||
Tool names are derived from OpenAPI `operationId` values and normalized for MCP clients. The exact naming convention should be deterministic and tested. A practical convention is lower snake case without the `Service` suffix in the subject:
|
||||
|
||||
```text
|
||||
MemoService_ListMemos -> memo_list_memos
|
||||
AttachmentService_GetAttachment -> attachment_get_attachment
|
||||
```
|
||||
|
||||
The OpenAPI `operationId` remains stored in tool metadata so tests and future diagnostics can prove which OpenAPI operation produced each MCP tool.
|
||||
|
||||
### Tool Schemas
|
||||
|
||||
Each MCP tool input schema is an object built from:
|
||||
|
||||
- OpenAPI path parameters
|
||||
- OpenAPI query parameters
|
||||
- JSON request body fields, when present
|
||||
|
||||
Required path parameters stay required. Required request bodies stay required. Optional query parameters stay optional. The schema should preserve OpenAPI descriptions and primitive types where possible.
|
||||
|
||||
Each MCP tool output schema is the OpenAPI HTTP 200 JSON response schema. For empty 200 responses with no JSON schema, the MCP output schema is:
|
||||
|
||||
```json
|
||||
{ "type": "object", "properties": { "ok": { "type": "boolean" } } }
|
||||
```
|
||||
|
||||
## Tool Scope
|
||||
|
||||
The first version exposes these curated OpenAPI operations:
|
||||
|
||||
- `MemoService_ListMemos`
|
||||
- `MemoService_CreateMemo`
|
||||
- `MemoService_GetMemo`
|
||||
- `MemoService_UpdateMemo`
|
||||
- `MemoService_DeleteMemo`
|
||||
- `MemoService_ListMemoComments`
|
||||
- `MemoService_CreateMemoComment`
|
||||
- `MemoService_ListMemoAttachments`
|
||||
- `MemoService_SetMemoAttachments`
|
||||
- `MemoService_ListMemoReactions`
|
||||
- `MemoService_UpsertMemoReaction`
|
||||
- `MemoService_DeleteMemoReaction`
|
||||
- `MemoService_ListMemoRelations`
|
||||
- `MemoService_SetMemoRelations`
|
||||
- `AttachmentService_ListAttachments`
|
||||
- `AttachmentService_GetAttachment`
|
||||
- `AttachmentService_DeleteAttachment`
|
||||
|
||||
Excluded in the first version:
|
||||
|
||||
- auth sign-in, sign-out, and refresh
|
||||
- user management
|
||||
- personal access token management
|
||||
- identity provider management
|
||||
- webhooks
|
||||
- instance settings
|
||||
- share-link management
|
||||
- AI transcription
|
||||
- bulk delete operations
|
||||
- any operation not present in generated OpenAPI
|
||||
|
||||
## Data Flow
|
||||
|
||||
### Startup
|
||||
|
||||
1. `server.NewServer` creates the existing `APIV1Service`.
|
||||
2. The new `MCPService` loads `proto/gen/openapi.yaml`.
|
||||
3. The OpenAPI parser builds the operation registry.
|
||||
4. The curated operation allowlist selects supported operations.
|
||||
5. Each selected operation is registered as an MCP tool with input schema, output schema, description, annotations, and operation metadata.
|
||||
|
||||
### Tool Call
|
||||
|
||||
1. The client sends a standard MCP `tools/call` request to `/mcp`.
|
||||
2. The MCP server validates the tool name and arguments against the OpenAPI-derived input schema.
|
||||
3. The adapter substitutes path parameters into the OpenAPI route template.
|
||||
4. The adapter encodes query parameters into the query string.
|
||||
5. The adapter marshals request body arguments as JSON when the operation has a request body.
|
||||
6. The adapter forwards the caller's `Authorization` header and executes the matching `/api/v1/...` request through the existing Echo handler in-process.
|
||||
7. The API response JSON is decoded into `map[string]any`.
|
||||
8. The MCP result returns a compact JSON text fallback plus object-shaped `structuredContent`.
|
||||
|
||||
## Result Shape
|
||||
|
||||
Every MCP result must use object-shaped `structuredContent`.
|
||||
|
||||
Rules:
|
||||
|
||||
- If the API response is a JSON object, return it unchanged.
|
||||
- If the API response is empty, return `{ "ok": true }`.
|
||||
- If an unexpected raw JSON array appears, wrap it as `{ "result": [...] }`.
|
||||
- If an unexpected scalar appears, wrap it as `{ "result": value }`.
|
||||
|
||||
This directly addresses issue #6022 by preventing collection tools from returning bare arrays.
|
||||
|
||||
## Authentication And Origin Safety
|
||||
|
||||
The MCP endpoint accepts the same bearer credentials as the API:
|
||||
|
||||
```text
|
||||
Authorization: Bearer <PAT-or-access-token>
|
||||
```
|
||||
|
||||
The MCP adapter forwards the bearer header to the in-process API request. Public API operations can work without authentication when the API allows them. Mutating operations require authentication because the API already enforces that behavior.
|
||||
|
||||
For browser-origin safety, `/mcp` rejects cross-origin browser requests unless the `Origin` header is same-origin or matches the configured instance URL. Requests without an `Origin` header are allowed because desktop MCP clients commonly omit it.
|
||||
|
||||
## Errors
|
||||
|
||||
The MCP server should convert failures into MCP tool errors:
|
||||
|
||||
- Invalid MCP arguments: concise validation message.
|
||||
- API `401` or `403`: preserve the API message where available.
|
||||
- API `404`: report that the resource was not found.
|
||||
- Other API errors: include the HTTP status code and decoded API message.
|
||||
- Internal OpenAPI or adapter errors: log server-side details and return a concise tool error.
|
||||
|
||||
Adapter errors should not bypass MCP result formatting unless the underlying MCP framework requires protocol-level errors for invalid protocol messages.
|
||||
|
||||
## Tool Annotations
|
||||
|
||||
Tool annotations are derived from HTTP methods:
|
||||
|
||||
- `GET`: read-only, non-destructive, idempotent.
|
||||
- `POST`: mutating unless the operation is explicitly known to be read-only.
|
||||
- `PATCH`: mutating, non-idempotent by default.
|
||||
- `DELETE`: destructive and idempotent by default.
|
||||
|
||||
These annotations are hints for clients and do not replace API authorization.
|
||||
|
||||
## Testing
|
||||
|
||||
### OpenAPI Parsing Tests
|
||||
|
||||
Tests should verify:
|
||||
|
||||
- every curated operation ID exists in `proto/gen/openapi.yaml`
|
||||
- every selected tool has an object input schema
|
||||
- every selected tool has an object output schema
|
||||
- selected operations do not include admin, auth, webhook, identity provider, personal access token, instance setting, share-link, AI transcription, or bulk-delete operations
|
||||
- tool names are deterministic and unique
|
||||
|
||||
### MCP Protocol Tests
|
||||
|
||||
Using an Echo test server and JSON-RPC requests, tests should verify:
|
||||
|
||||
- `initialize` succeeds
|
||||
- `tools/list` returns only curated OpenAPI-derived tools
|
||||
- tool definitions include input and output schemas
|
||||
- no prompts or resources capabilities are advertised
|
||||
- collection tool calls return object-shaped `structuredContent`, never a bare array
|
||||
|
||||
### Adapter Tests
|
||||
|
||||
Representative adapter tests should cover:
|
||||
|
||||
- `GET /api/v1/memos?pageSize=...`
|
||||
- `POST /api/v1/memos`
|
||||
- `GET /api/v1/memos/{memo}`
|
||||
- `PATCH /api/v1/memos/{memo}`
|
||||
- `DELETE /api/v1/memos/{memo}`
|
||||
- `GET /api/v1/memos/{memo}/comments`
|
||||
- `POST /api/v1/memos/{memo}/reactions`
|
||||
|
||||
Before finishing implementation, run:
|
||||
|
||||
```bash
|
||||
go test ./server/router/mcp/... ./server/router/api/v1/... ./server/...
|
||||
```
|
||||
|
||||
## Implementation Notes
|
||||
|
||||
- Do not hand-edit `proto/gen/openapi.yaml`.
|
||||
- If a needed MCP tool is not represented by OpenAPI, add or adjust the proto/API surface first, run `cd proto && buf generate`, then derive the MCP tool from the regenerated OpenAPI.
|
||||
- Prefer existing API auth and gateway behavior over duplicating authorization logic in the MCP adapter.
|
||||
- Keep the first version intentionally small. Additional OpenAPI-derived tools can be added by extending the curated allowlist and tests.
|
||||
@@ -0,0 +1,168 @@
|
||||
# Markdown WYSIWYG Editor — Design
|
||||
|
||||
**Date:** 2026-06-11
|
||||
**Status:** Approved pending user review
|
||||
|
||||
## Goal
|
||||
|
||||
Replace the plain `<textarea>` memo editor with a modern WYSIWYG editor that renders
|
||||
markdown syntax live as the user types (Linear / Claude.ai composer style) while keeping
|
||||
markdown — not HTML — as the only storage and API format. A toggle lets users drop back
|
||||
to the raw textarea editor at any time.
|
||||
|
||||
## Background
|
||||
|
||||
The current editor (`web/src/components/MemoEditor/Editor/index.tsx`) is a textarea with
|
||||
hand-rolled features layered on top: tag suggestions (`#`), slash commands (`/`),
|
||||
Ctrl+B/I markdown shortcuts, list auto-continuation, URL-paste-to-link, IME composition
|
||||
plumbing, auto-grow height, and a toolbar that inserts raw markdown strings through an
|
||||
imperative, character-offset-based `EditorRefActions` API.
|
||||
|
||||
Live markdown editing is among the most-requested memos features
|
||||
(usememos/memos#2216, #3495, #766, #4259).
|
||||
|
||||
### Prior art (researched 2026-06)
|
||||
|
||||
- **Linear** builds its editor directly on **ProseMirror** (+ y-prosemirror for collab).
|
||||
- **ChatGPT's composer** is raw **ProseMirror**.
|
||||
- **Claude.ai's composer** is **Tiptap** (headless framework on ProseMirror).
|
||||
|
||||
All three references are ProseMirror-family WYSIWYG editors with markdown input rules.
|
||||
|
||||
### Library options considered
|
||||
|
||||
| Option | Verdict |
|
||||
|---|---|
|
||||
| **Tiptap v3 + `@tiptap/markdown`** | **Chosen.** Claude.ai's stack. Best ecosystem; StarterKit covers the core set with input rules; Suggestion utility replaces hand-rolled tag/slash popups; official bidirectional markdown (early release — gated by a spike). First-class React bindings; best-in-class IME handling via ProseMirror. ~100 KB gzipped. |
|
||||
| Milkdown | Markdown-first ProseMirror + remark; strongest fidelity by design, but thin UX building blocks, small community, low bus factor. |
|
||||
| Raw ProseMirror + prosemirror-markdown | Linear's literal approach; maximum control, most engineering effort for the least product difference. |
|
||||
| Lexical | Non-ProseMirror engine; markdown is a conversion target, historically weaker IME. |
|
||||
| CodeMirror 6 live-preview (Obsidian style) | Perfect byte fidelity but a different UX than the named references; largely DIY. |
|
||||
|
||||
## Decisions (user-confirmed)
|
||||
|
||||
1. **Raw mode stays**: a per-editor toggle switches between WYSIWYG and the current
|
||||
textarea editor. (Revised from an earlier "full replacement" decision.)
|
||||
2. **Fidelity contract**: lossless for supported syntax — semantic round-trip for
|
||||
everything the editor models; style normalization (e.g. `*` → `-` bullets) is
|
||||
acceptable. Unknown/exotic syntax is preserved verbatim, never dropped.
|
||||
3. **v1 scope — Linear-style core set** rendered live: bold/italic/strike/inline-code,
|
||||
headings, ordered/unordered/task lists, links, blockquotes, code blocks, and memos
|
||||
`#tags`. Tables, `$…$`/`$$…$$` math, and inline HTML remain literal text in the
|
||||
editor (they still render richly in the memo view after save). Mermaid lives in
|
||||
` ```mermaid ` fences, so it is just a code block while editing.
|
||||
|
||||
## Architecture
|
||||
|
||||
### Editor abstraction
|
||||
|
||||
`EditorContent` hosts one of two implementations behind a shared **`EditorController`**
|
||||
interface:
|
||||
|
||||
- `focus()`, `getMarkdown()`, `setMarkdown()`, `insertMarkdown()`, `isEmpty()`
|
||||
- Formatting **intents**: `toggleBold()`, `toggleItalic()`, `toggleTaskList()`, etc.
|
||||
|
||||
The **Tiptap editor** implements intents as ProseMirror commands. The **textarea editor**
|
||||
implements them exactly as today (wrap selection in `**`). The toolbar dispatches intents
|
||||
and never knows which editor is mounted. The textarea's existing string-surgery API
|
||||
(`EditorRefActions`) becomes internal to the textarea implementation rather than the
|
||||
public contract.
|
||||
|
||||
### Mode toggle
|
||||
|
||||
- A small icon button in the editor toolbar switches modes mid-edit.
|
||||
- Switching is a markdown string handoff: WYSIWYG → raw serializes (the same path as
|
||||
save, so no new fidelity risk); raw → WYSIWYG parses.
|
||||
- The preference persists in localStorage (per device). No backend/proto changes; can
|
||||
graduate to a server-side user setting later.
|
||||
|
||||
### Tiptap editor composition
|
||||
|
||||
Built on `@tiptap/react` `useEditor`. Extensions:
|
||||
|
||||
- **StarterKit** (configured): headings, bold/italic/strike/code, lists, blockquote,
|
||||
code block — all with live input rules (`**bold**` converts as you type).
|
||||
- **TaskList / TaskItem**, **Link** (includes URL-paste-over-selection → link),
|
||||
**Placeholder** (i18n'd), **`@tiptap/markdown`**.
|
||||
- **`Tag`** (custom): inline node for `#tag`, rendered as a styled token, serialized
|
||||
back to `#tag` verbatim. `#` triggers a Suggestion-plugin popup backed by the
|
||||
existing tag store.
|
||||
- **`SlashCommand`** (custom): Suggestion-plugin popup on `/`, replacing
|
||||
`SlashCommands.tsx` in WYSIWYG mode.
|
||||
- **`PreservedBlock`** (custom): the fidelity workhorse — unmodeled constructs
|
||||
(tables, math, inline HTML, unrecognized syntax) are captured at parse time into
|
||||
nodes carrying their raw source, displayed as literal text (subtle mono styling),
|
||||
and re-emitted byte-for-byte on serialize.
|
||||
|
||||
Hand-rolled textarea features that become native or config in WYSIWYG mode:
|
||||
Ctrl+B/I keymaps (StarterKit), list auto-continuation (ProseMirror lists),
|
||||
auto-grow height (contenteditable), IME composition (ProseMirror — better CJK
|
||||
behavior than the textarea plumbing).
|
||||
|
||||
### Data flow
|
||||
|
||||
Unchanged at the boundaries. Memo markdown → `setMarkdown()` on load. On update,
|
||||
serialized markdown (debounced) flows into the existing state reducer as
|
||||
`state.content`, so auto-save, the localStorage draft cache (still a plain markdown
|
||||
string), tag extraction, and the save path are untouched. The backend only ever sees
|
||||
markdown. File paste/drag wires Tiptap `handlePaste`/`handleDrop` into the existing
|
||||
`uploadService`.
|
||||
|
||||
## Fidelity contract
|
||||
|
||||
1. **Supported constructs** round-trip semantically; list-marker style may normalize;
|
||||
content never changes meaning.
|
||||
2. **Unmodeled constructs** round-trip byte-for-byte via `PreservedBlock`.
|
||||
3. **A round-trip corpus test enforces this permanently** (see Testing). It is written
|
||||
first, as a spike, and is the go/no-go gate on `@tiptap/markdown` (early release).
|
||||
If the gate fails and custom Marked tokenizers cannot fix it, the fallback is
|
||||
swapping only the parse/serialize layer to remark (already a memos dependency)
|
||||
while keeping the Tiptap editor — editor and serialization are deliberately
|
||||
decoupled for this reason.
|
||||
|
||||
## Error handling
|
||||
|
||||
- **Load guard (tripwire):** on opening an existing memo, parse → re-serialize →
|
||||
compare semantically. If the round-trip would lose content, log it and show a
|
||||
notice — "this memo contains syntax the editor can't safely edit" — with a
|
||||
one-click switch to raw mode for that memo. Expected never to fire.
|
||||
- **Draft safety:** the localStorage draft cache stays a markdown string on the same
|
||||
debounce as today; drafts are interchangeable between both editor modes.
|
||||
- **Save path:** reuses already-serialized `state.content`; save never triggers a
|
||||
fresh parse, so it gains no new failure mode.
|
||||
|
||||
## Testing
|
||||
|
||||
- **Round-trip corpus test (the spike, written first):** fixture markdown files
|
||||
(GFM, tables, math, mermaid, inline HTML, nested lists, CJK, emoji) → parse →
|
||||
serialize → assert semantic equality for supported syntax, byte equality for
|
||||
preserved blocks. Runs in the existing vitest/jsdom setup.
|
||||
- **Extension unit tests:** `Tag` (parse/serialize/suggestion trigger),
|
||||
`PreservedBlock` (verbatim round-trip), link-paste.
|
||||
- **Component tests** (@testing-library): toolbar intents produce expected markdown
|
||||
in both modes; slash/tag popups open and insert correctly; mode toggle hands
|
||||
content across without loss.
|
||||
- **Manual QA:** CJK IME composition, mobile Safari/Chrome soft keyboards, focus
|
||||
mode, paste-image upload.
|
||||
|
||||
## Rollout
|
||||
|
||||
Feature branch, PR series:
|
||||
|
||||
1. **Spike:** corpus test + `@tiptap/markdown` verdict (throwaway if the gate fails).
|
||||
2. **Core editor:** Tiptap behind `EditorController`, StarterKit set, markdown in/out,
|
||||
wired into the state layer.
|
||||
3. **Memos features:** `Tag` + suggestions, `SlashCommand`, `PreservedBlock`,
|
||||
file paste/drop, toolbar intent conversion.
|
||||
4. **Toggle + refactor:** mode toggle UI and persistence; textarea editor refactored
|
||||
to implement `EditorController` (nothing deleted — it powers raw mode).
|
||||
|
||||
Dependency cost: ~100 KB gzipped (`@tiptap/core`, `@tiptap/react`, extensions,
|
||||
`@tiptap/markdown` + MarkedJS). WYSIWYG is the default mode for all users.
|
||||
|
||||
## Out of scope (v1)
|
||||
|
||||
- Interactive table editing, live KaTeX/mermaid rendering while editing.
|
||||
- Collaborative editing (Yjs).
|
||||
- Server-side persistence of the mode preference.
|
||||
- Mobile apps (web/PWA only — native apps are separate codebases).
|
||||
@@ -0,0 +1,257 @@
|
||||
# Design: Expand the CEL filter surface
|
||||
|
||||
- **Date:** 2026-06-15
|
||||
- **Status:** Approved (design); ready for implementation planning
|
||||
- **Area:** `internal/filter` (memo & attachment filter engine)
|
||||
- **Follow-up spec:** CEL engine hardening + native-AST migration (separate, sequenced after this)
|
||||
|
||||
## Summary
|
||||
|
||||
memos lets API clients pass a CEL expression in the `filter` field of list
|
||||
requests. The `internal/filter` engine uses `cel-go` purely as a **parse +
|
||||
type-check frontend**, then walks the AST and translates it into a SQL `WHERE`
|
||||
fragment for the active dialect (SQLite / MySQL / Postgres). cel-go never
|
||||
evaluates anything.
|
||||
|
||||
This spec adds three new CEL constructs that users can write, each with a SQL
|
||||
translation across all three dialects:
|
||||
|
||||
1. `startsWith()` / `endsWith()` on scalar string fields (case-insensitive).
|
||||
2. `all()` comprehension on tag lists (matches only non-empty tag sets).
|
||||
3. `matches(regex)` on string fields.
|
||||
|
||||
## Goals
|
||||
|
||||
- Expose the three constructs above through the existing
|
||||
parse → IR → render pipeline.
|
||||
- Keep parity across SQLite, MySQL, and Postgres, with golden tests for each.
|
||||
- Preserve the engine's invariant: only schema-declared fields and explicitly
|
||||
supported operations are accepted; everything else is rejected with a clear
|
||||
error.
|
||||
|
||||
## Non-goals
|
||||
|
||||
- **Value-producing CEL features with no SQL form** are explicitly out of scope:
|
||||
optional types (`?.`, `optional.of`), `map()` / `filter()` transforms, the
|
||||
math extension, string-manipulation extensions (`replace`, `split`,
|
||||
`substring`, `format`), and two-variable comprehensions. There is nothing to
|
||||
push into a `WHERE` clause for these.
|
||||
- **`lowerAscii()` / `upperAscii()`** — dropped. `contains()` is already
|
||||
case-insensitive on all dialects, and the new `startsWith`/`endsWith` are
|
||||
case-insensitive too (see decisions), so explicit case-folding adds little.
|
||||
Revisit only if users ask.
|
||||
- **Parser hardening and the native-AST proto migration** are a separate
|
||||
follow-up spec. The one exception that rides along here is
|
||||
`cel.ValidateRegexLiterals()`, which feature ③ requires for safety.
|
||||
|
||||
## Background: how the engine works today
|
||||
|
||||
Pipeline (see `internal/filter/README.md`):
|
||||
|
||||
1. **Parse** — `env.Compile(filter)` parses and type-checks against the
|
||||
memo/attachment environment declared in `schema.go`; the AST is converted via
|
||||
`cel.AstToParsedExpr()`.
|
||||
2. **Normalize** — `parser.go` walks the CEL `Expr` and builds a
|
||||
dialect-agnostic IR (`ir.go`): logical ops, comparisons, `IN`, `contains()`,
|
||||
and `exists()` comprehensions over tag lists.
|
||||
3. **Render** — `render.go` walks the IR and emits dialect-specific SQL plus
|
||||
placeholder args.
|
||||
|
||||
Two existing facts that shaped this design:
|
||||
|
||||
- **`contains()` is already case-insensitive** on all three dialects
|
||||
(`render.go` `renderContainsCondition`): SQLite uses the custom
|
||||
`memos_unicode_lower` function, Postgres uses `ILIKE`, MySQL relies on its
|
||||
default case-insensitive collation.
|
||||
- **Custom SQLite scalar functions are already registered**
|
||||
(`store/db/sqlite/functions.go`, `ensureUnicodeLowerRegistered` via
|
||||
`modernc.org/sqlite`'s `RegisterScalarFunction`, invoked from
|
||||
`store/db/sqlite/sqlite.go`). The new `REGEXP` function follows this exact
|
||||
pattern.
|
||||
|
||||
cel-go version: `v0.28.0` (latest is `v0.28.1`, a patch with nothing relevant to
|
||||
memos). No version bump is required for this work.
|
||||
|
||||
## Resolved decisions
|
||||
|
||||
| # | Decision | Choice |
|
||||
|---|----------|--------|
|
||||
| A | Case-sensitivity of new scalar `startsWith`/`endsWith` | **Case-insensitive**, consistent with existing `contains()`. `==` stays case-sensitive (exact match). |
|
||||
| B | `all()` over a memo with zero tags | **Require non-empty**: an untagged memo does NOT match an `all()` filter. (Diverges from strict CEL vacuous-truth, but matches search-box intuition.) |
|
||||
| C | Keep `lowerAscii()` / `upperAscii()`? | **Drop** from this spec. |
|
||||
|
||||
## Detailed design
|
||||
|
||||
### ① `startsWith()` / `endsWith()` on scalar string fields
|
||||
|
||||
**Surface.** Allow `field.startsWith("x")` and `field.endsWith("x")` as
|
||||
top-level boolean calls for scalar string fields. Today these functions are only
|
||||
recognized *inside* tag comprehensions (`parser.go` `extractPredicate`).
|
||||
|
||||
Applicable fields: memo `content`; attachment `filename`, `mime_type`.
|
||||
`creator` is intentionally **excluded**: it is an identity field with `==`/`!=`
|
||||
semantics whose column is wrapped as `'users/' || username`, so prefix/suffix
|
||||
matching there would match against the `users/` prefix and surprise users.
|
||||
|
||||
**Schema.** Generalize the per-field text-matching capability. Today `Field` has
|
||||
`SupportsContains bool`. Replace/extend with a capability that also covers
|
||||
prefix/suffix matching (e.g. a `SupportsTextMatch bool`, or reuse
|
||||
`SupportsContains` to gate all three LIKE-based ops). Fields that already set
|
||||
`SupportsContains: true` gain prefix/suffix support.
|
||||
|
||||
**Parser.** In `buildCallCondition`, recognize `startsWith` / `endsWith` calls
|
||||
whose target is a scalar string field and whose single argument is a string
|
||||
literal. Reject non-literal arguments and fields without the capability.
|
||||
|
||||
**IR.** Generalize `ContainsCondition` into a single node:
|
||||
|
||||
```go
|
||||
type TextMatchMode string
|
||||
const (
|
||||
TextMatchContains TextMatchMode = "contains"
|
||||
TextMatchPrefix TextMatchMode = "prefix"
|
||||
TextMatchSuffix TextMatchMode = "suffix"
|
||||
)
|
||||
|
||||
type TextMatchCondition struct {
|
||||
Field string
|
||||
Mode TextMatchMode
|
||||
Value string
|
||||
}
|
||||
```
|
||||
|
||||
`contains()` migrates to `TextMatchCondition{Mode: TextMatchContains}`.
|
||||
|
||||
**Render.** Build a `LIKE` pattern from the (escaped) literal:
|
||||
|
||||
- prefix → `value%`
|
||||
- suffix → `%value`
|
||||
- contains → `%value%`
|
||||
|
||||
Reuse the existing case-insensitive rendering already used by `contains()`:
|
||||
SQLite `memos_unicode_lower(col) LIKE memos_unicode_lower(?)`, Postgres
|
||||
`col ILIKE $n`, MySQL `col LIKE ?`.
|
||||
|
||||
**LIKE-escaping fix.** The current `contains()` renderer interpolates the raw
|
||||
value into the pattern without escaping `%`, `_`, or `\`. This means a search
|
||||
for `50%` behaves as a wildcard. The new shared path will escape these
|
||||
metacharacters (and emit `ESCAPE '\'` where required by the dialect). This
|
||||
closes a small latent wildcard-injection inconsistency and applies uniformly to
|
||||
contains/prefix/suffix.
|
||||
|
||||
### ② `all()` comprehension on tag lists
|
||||
|
||||
**Surface.** Allow `tags.all(t, <pred>)` where `<pred>` is one of the predicates
|
||||
already supported for `exists()`: `t == "x"`, `t.startsWith("x")`,
|
||||
`t.endsWith("x")`, `t.contains("x")`.
|
||||
|
||||
**Parser.** `detectComprehensionKind` currently accepts only `exists()` and
|
||||
explicitly rejects `all()`. Add a `ComprehensionAll` kind (accumulator inits to
|
||||
`true`, loop step uses `_&&_`). Reuse the existing predicate extraction.
|
||||
|
||||
**IR.** Add `ComprehensionAll` to the `ComprehensionKind` enum; the existing
|
||||
`ListComprehensionCondition` already carries `Kind`.
|
||||
|
||||
**Render — proper per-element semantics.** The existing `exists()`
|
||||
implementation matches the *serialized* JSON array text with `LIKE`, which works
|
||||
for "at least one element matches a substring" but **cannot** express "every
|
||||
element matches." `all()` therefore needs real per-element iteration. Decision B
|
||||
(require non-empty) means: array is non-empty **AND** no element fails the
|
||||
predicate.
|
||||
|
||||
- **SQLite:**
|
||||
```sql
|
||||
(<array> IS NOT NULL AND <array> != '[]'
|
||||
AND NOT EXISTS (SELECT 1 FROM json_each(<array>)
|
||||
WHERE NOT (<predicate on json_each.value>)))
|
||||
```
|
||||
- **Postgres:**
|
||||
```sql
|
||||
(<array> IS NOT NULL AND jsonb_array_length(<array>) > 0
|
||||
AND NOT EXISTS (SELECT 1 FROM jsonb_array_elements_text(<array>) AS e(value)
|
||||
WHERE NOT (<predicate on e.value>)))
|
||||
```
|
||||
- **MySQL:**
|
||||
```sql
|
||||
(<array> IS NOT NULL AND JSON_LENGTH(<array>) > 0
|
||||
AND NOT EXISTS (SELECT 1 FROM JSON_TABLE(<array>, '$[*]'
|
||||
COLUMNS (value VARCHAR(512) PATH '$')) AS j
|
||||
WHERE NOT (<predicate on j.value>)))
|
||||
```
|
||||
|
||||
The per-element predicate reuses LIKE/`=` against the element `value`
|
||||
(case-insensitive for `startsWith`/`endsWith`/`contains`, consistent with ①).
|
||||
Hierarchical-tag prefix behavior should match the existing `exists()` rendering
|
||||
(a prefix matches the exact tag or a `tag/...` child).
|
||||
|
||||
> Note: this introduces correlated subqueries against the same `memo.payload`
|
||||
> column the outer query already reads; confirm the generated SQL composes with
|
||||
> the surrounding `WHERE` and placeholder offsets in `helpers.AppendConditions`.
|
||||
|
||||
### ④ `matches(regex)` on string fields
|
||||
|
||||
**Surface.** Allow `field.matches("pattern")` for the same free-text fields as ①
|
||||
(`content`, `filename`, `mime_type`; `creator` excluded), literal pattern only.
|
||||
|
||||
**Env / validation.** Add `cel.ValidateRegexLiterals()` to the env options in
|
||||
`schema.go` so malformed patterns fail at compile time with a clear message
|
||||
(validated against Go's RE2).
|
||||
|
||||
**Parser / IR.** Recognize `matches` calls; add:
|
||||
|
||||
```go
|
||||
type RegexCondition struct {
|
||||
Field string
|
||||
Pattern string
|
||||
}
|
||||
```
|
||||
|
||||
Reject non-literal patterns and fields without text-match capability.
|
||||
|
||||
**Render.**
|
||||
|
||||
- **Postgres:** `col ~ $n`
|
||||
- **MySQL:** `col REGEXP ?`
|
||||
- **SQLite:** `col REGEXP ?`. SQLite desugars `X REGEXP Y` to the function call
|
||||
`regexp(Y, X)`, so register a 2-arg scalar function named `regexp(pattern,
|
||||
value)` returning 1/0, backed by Go's `regexp` package, following the
|
||||
`ensureUnicodeLowerRegistered` pattern in `store/db/sqlite/functions.go`.
|
||||
Compile patterns lazily with a small cache (or rely on RE2 compile per call;
|
||||
decide during implementation based on measured cost).
|
||||
|
||||
**Documented caveats** (engine differences are inherent, not bugs):
|
||||
|
||||
- Regex *syntax* differs per engine: SQLite uses Go RE2; Postgres uses POSIX
|
||||
ERE; MySQL 8.0+ uses ICU. Portable patterns work everywhere; engine-specific
|
||||
constructs may not. Document this in `internal/filter/README.md`.
|
||||
- ReDoS risk is low: RE2 (SQLite path) is linear-time; Postgres/MySQL POSIX
|
||||
engines do not catastrophically backtrack. `ValidateRegexLiterals()` rejects
|
||||
patterns that don't compile under RE2 as a first-line guard.
|
||||
|
||||
## Testing strategy
|
||||
|
||||
For each feature, add golden tests in
|
||||
`store/db/{sqlite,mysql,postgres}/memo_filter_test.go` (and the attachment
|
||||
filter tests where applicable):
|
||||
|
||||
- **Happy path:** assert the exact SQL fragment and args per dialect.
|
||||
- **Error paths:** non-literal argument, unsupported field, malformed regex,
|
||||
unsupported predicate inside `all()`.
|
||||
- **`all()` empty-set:** confirm an untagged memo does not match (decision B).
|
||||
- **LIKE escaping:** confirm `%`, `_`, `\` in `contains`/`startsWith`/`endsWith`
|
||||
values are treated literally.
|
||||
|
||||
Run `go test ./...` (engine unit tests plus all three dialect suites). The
|
||||
`contains()` → `TextMatchCondition` refactor must keep existing golden outputs
|
||||
unchanged except for the intentional escaping fix.
|
||||
|
||||
## Rollout / sequencing
|
||||
|
||||
This is the first of two specs. The second (already agreed) covers engine
|
||||
hardening: tightened parser limits (`ParserExpressionSizeLimit`,
|
||||
`ParserRecursionLimit`, `ParserErrorRecoveryLimit`), the
|
||||
`ValidateComprehensionNestingLimit` / `ValidateHomogeneousAggregateLiterals`
|
||||
validators, and migrating `parser.go` off the deprecated
|
||||
`genproto/.../expr/v1alpha1` proto to the native `common/ast` API. Building this
|
||||
surface-expansion spec first is acceptable; the hardening migration is a pure
|
||||
refactor that the golden tests written here will help protect.
|
||||
Reference in New Issue
Block a user