Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
f0ff080
feat: integrate excalidraw-mcp as first-class diagramming feature
GeneralJerel Mar 19, 2026
27e91f7
fix: give excalidraw skill its own section in agent system prompt
GeneralJerel Mar 19, 2026
e20a409
fix: center widget content in iframe
GeneralJerel Mar 19, 2026
7fd3913
fix: resolve hydration mismatch and cap widget height
GeneralJerel Mar 19, 2026
293135a
feat: add zoom controls to widget renderer
GeneralJerel Mar 19, 2026
c585bde
fix: add maxDuration to prevent route timeout during MCP calls
GeneralJerel Mar 19, 2026
99576ed
feat: move zoom controls from widgetRenderer to MCP widget only
GeneralJerel Mar 19, 2026
c50fe38
feat: add markdown canvas editor with LangGraph integration
GeneralJerel Mar 19, 2026
8adf9e5
fix: add MCPAppsMiddleware to document agent for Excalidraw support
GeneralJerel Mar 19, 2026
248b6ac
improve: add visualization instructions to document agent system prompt
GeneralJerel Mar 19, 2026
b898857
feat: integrate excalidraw diagram skill into document agent
GeneralJerel Mar 19, 2026
4d7eb0e
feat: merge document_agent into sample_agent for unified canvas exper…
GeneralJerel Mar 19, 2026
acb9aa6
fix: prevent state updates after component unmount in canvas editor
GeneralJerel Mar 19, 2026
3c84feb
style: refactor canvas page layout - add header, improve padding and …
GeneralJerel Mar 19, 2026
4c34c07
style: rename canvas page to 'Document to Diagram'
GeneralJerel Mar 19, 2026
4b7b73d
ux: improve editor visibility and interaction feedback
GeneralJerel Mar 19, 2026
47c2624
fix: hide placeholder text when editor is focused
GeneralJerel Mar 19, 2026
dda5a86
feat: add 'Get started' button to canvas header
GeneralJerel Mar 19, 2026
2ddc6bb
refactor: simplify and strengthen Excalidraw skill with overlap preve…
GeneralJerel Mar 19, 2026
b8619ce
fix: add isMountedRef checks to all state-updating effects
GeneralJerel Mar 19, 2026
de80d48
feat: add WebSocket document as default content on canvas page
GeneralJerel Mar 19, 2026
45fd46d
fix: replace suggestions with single WebSocket document chip
GeneralJerel Mar 19, 2026
ef85a6d
feat: add more technical document generation prompts
GeneralJerel Mar 19, 2026
036554f
fix: hide suggestions when chat has messages
GeneralJerel Mar 19, 2026
280e8f2
fix: show only 3 prompt suggestions initially
GeneralJerel Mar 19, 2026
0c72925
fix: resolve AbortError and fix suggestion chips visibility
GeneralJerel Mar 19, 2026
6eb355a
feat: add streaming animation to document editor during generation
GeneralJerel Mar 19, 2026
d546f20
fix: fix markdown rendering and improve streaming
GeneralJerel Mar 19, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions .chalk/reviews/sessions/pr-13/handoff.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# Handoff

## Scope
- Item: PR #13 — fix: resolve lint errors in widget-renderer.tsx and layout.tsx
- Branch: fix/lint-errors-2026-03-18
- Commit: b834bc2
- Goal: Fix lint errors introduced in prior commits and add community/docs files

## What Changed
- Added CODE_OF_CONDUCT.md (new community document)
- Added CONTRIBUTING.md (new contributor guide)
- Added Makefile with dev/build/lint/clean targets
- Updated README.md Quick Start to use Makefile commands
- Updated asset video link in README.md
- layout.tsx: Migrated from Google Fonts `<link>` tag to Next.js `next/font/google` (Plus_Jakarta_Sans)
- widget-renderer.tsx: Renamed `description` prop to `_description` to suppress unused-var lint warning
- widget-renderer.tsx: Changed `useEffect` → `useLayoutEffect` for iframe srcdoc mutation
- widget-renderer.tsx: Refactored loading phrase reset to use `prevActiveRef` to avoid reset-on-every-render

## Files Changed
- CODE_OF_CONDUCT.md (+117)
- CONTRIBUTING.md (+207)
- Makefile (+36)
- README.md (+29, -7)
- apps/app/src/app/layout.tsx (+9, -5)
- apps/app/src/components/generative-ui/widget-renderer.tsx (+14, -4)

## Risk Areas
- **Lint still fails** — the PR claims to fix lint errors but `pnpm lint` exits with 2 errors and 1 warning after the changes
- CONTRIBUTING.md has inconsistent repo URLs (mixes `OpenGenerativeUI` and `open-generative-ui` casing)
- Font variable `--font-plus-jakarta` is set on body but may not be referenced in Tailwind/CSS config
- `useLayoutEffect` calling `setLoaded(false)` + `setHeight(0)` still triggers `react-hooks/set-state-in-effect` lint error

## Commands Run
- `pnpm lint` — **FAILED** (2 errors, 1 warning in widget-renderer.tsx)

## Known Gaps
- Lint errors not actually resolved despite PR title claiming they are
- No TypeScript build check run
- No tests added or run

## Suggested Focus For Reviewers
1. **[BLOCKING]** Lint failure — PR goal unmet; `setIndex(0)` in useEffect (line 417) and `setLoaded(false)` in useLayoutEffect (line 465) still violate `react-hooks/set-state-in-effect`
2. **[BLOCKING]** `_description` warning still present despite renaming
3. CONTRIBUTING.md URL inconsistency (`OpenGenerativeUI` vs `open-generative-ui`)
4. Font variable wiring — confirm `--font-plus-jakarta` is consumed in Tailwind config
157 changes: 157 additions & 0 deletions .chalk/reviews/sessions/pr-13/maintainer-reply.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
# Maintainer Review Reply — PR #13

> fix: resolve lint errors in widget-renderer.tsx and layout.tsx

---

Thanks for the PR! Great that you're tackling the lint errors — this is exactly the kind of housekeeping that keeps the codebase healthy.

I went through the changes and have a few things to address before we merge. Two of them are blockers because they still trip the lint gate we're trying to fix.

---

### Blocking

**`react-hooks/set-state-in-effect` — two violations remain in `widget-renderer.tsx`**

`eslint-plugin-react-hooks@7` introduced a new rule that disallows synchronous `setState` calls directly inside an effect body. The PR currently has two spots that trigger it:

1. **~line 417** — `setIndex(0)` inside `useEffect`. The intent (reset to 0 when `active` flips true) is right, but the reset needs to happen outside the synchronous effect body. One clean approach: track `active` in a ref and only call `setIndex` inside the `setInterval` callback — start the interval from index 0 and let it cycle from there.

2. **~lines 465–466** — `setLoaded(false)` and `setHeight(0)` inside `useLayoutEffect`. These could be combined into a single `useReducer` reset, or the reset state stored in refs and derived at render time.

These two cause CI to exit non-zero, so the PR goal isn't quite met yet.

---

### Minor (non-blocking, but worth cleaning up)

**~line 431** — `_description` in the destructuring still produces an unused-var warning. The simplest fix is to just omit it from destructuring (`{ title, html }`). The prop can stay in `WidgetRendererProps` for API compatibility — only the binding needs to go.

**`globals.css` font variable** — `layout.tsx` correctly injects `--font-plus-jakarta` onto `<body>`, but `globals.css` still references `'Plus Jakarta Sans'` as a hardcoded string instead of `var(--font-plus-jakarta)`. This means Next.js's self-hosted font optimization is wired up but never actually used. Easy one-liner fix:
```css
--font-family: var(--font-plus-jakarta), system-ui, sans-serif;
```

---

Once the two `set-state-in-effect` issues are sorted, this should be good to go. Happy to help think through the hook refactors if useful — the `useLayoutEffect` one in particular has a few valid approaches depending on how you want to handle the reset.

---

<details>
<summary><strong>Ask AI to Fix These Issues</strong></summary>

Copy and paste the prompt below into Claude, ChatGPT, Cursor, or any AI assistant. It includes full context so the AI can fix all remaining lint failures without additional back-and-forth.

---

````
I need help fixing 3 remaining ESLint failures in a Next.js / React project.
The branch is `fix/lint-errors-2026-03-18` on https://github.com/CopilotKit/OpenGenerativeUI.
Running `pnpm lint` currently exits with 2 errors and 1 warning — all in one file:
apps/app/src/components/generative-ui/widget-renderer.tsx

The ESLint config extends `eslint-config-next/core-web-vitals` and `eslint-config-next/typescript`,
which pulls in `eslint-plugin-react-hooks@7.0.1`. That version introduced a new rule —
`react-hooks/set-state-in-effect` — that flags any synchronous setState call made directly inside
an effect body (useEffect or useLayoutEffect).

─── FAILURE 1 (Error) — widget-renderer.tsx ~line 414 ───────────────────────

The `useLoadingPhrase` hook manages a cycling loading message. When `active` flips from false → true
it should reset to phrase index 0, then start an interval to cycle phrases every 1800ms.

Current (broken) code:
```tsx
function useLoadingPhrase(active: boolean) {
const [index, setIndex] = useState(0);
const prevActiveRef = useRef<boolean | null>(null);

useEffect(() => {
if (active && !prevActiveRef.current) {
setIndex(0); // ← lint error: setState inside effect body
}
prevActiveRef.current = active;

if (!active) return;
const interval = setInterval(() => {
setIndex((i) => (i + 1) % LOADING_PHRASES.length);
}, 1800);
return () => clearInterval(interval);
}, [active]);

return LOADING_PHRASES[index];
}
```

Fix goal: eliminate the synchronous `setIndex(0)` call from inside the effect body while keeping
the same observable behaviour (index resets to 0 when active starts, then cycles).

─── FAILURE 2 (Error) — widget-renderer.tsx ~line 460 ───────────────────────

The `WidgetRenderer` component writes HTML into a sandboxed iframe imperatively (to preserve iframe
JS state across React re-renders). When `html` changes it resets `loaded` and `height` back to
false/0 so the loading overlay re-appears.

Current (broken) code:
```tsx
useLayoutEffect(() => {
if (!html || !iframeRef.current) return;
if (html === committedHtmlRef.current) return;
committedHtmlRef.current = html;
iframeRef.current.srcdoc = assembleDocument(html);
setLoaded(false); // ← lint error: setState inside effect body
setHeight(0); // ← lint error: setState inside effect body
}, [html]);
```

State declarations (for context):
```tsx
const [height, setHeight] = useState(0);
const [loaded, setLoaded] = useState(false);
```

Fix goal: eliminate synchronous setState calls from inside the useLayoutEffect body while keeping
the reset-on-html-change behaviour. The iframe srcdoc write must stay synchronous (that's why
useLayoutEffect is correct here). Consider combining loaded+height into a single reducer, or
storing reset state in refs and deriving render values from them.

─── FAILURE 3 (Warning) — widget-renderer.tsx ~line 431 ───────────────────────

The `description` prop is accepted in the function signature but never used. A previous attempt
renamed it to `_description` but the project's ESLint config does not have `argsIgnorePattern: "^_"`
configured so the warning still fires.

Current (broken) code:
```tsx
export function WidgetRenderer({ title, _description, html }: WidgetRendererProps) {
```

Fix goal: silence the unused-var warning. The simplest fix is to remove `description` / `_description`
from the destructuring entirely (just use `{ title, html }`). The prop can remain in the
`WidgetRendererProps` type for API compatibility — only the destructuring binding needs to change.

─── Bonus fix (globals.css) ────────────────────────────────────────────────

In `apps/app/src/app/globals.css`, the font-family custom property currently reads:
```css
--font-family: 'Plus Jakarta Sans', system-ui, sans-serif;
```

Change it to consume the Next.js font CSS variable injected by layout.tsx:
```css
--font-family: var(--font-plus-jakarta), system-ui, sans-serif;
```

─── Constraints ───────────────────────────────────────────────────────────────

- Do NOT add eslint-disable comments as a workaround — fix the code properly.
- Keep all existing observable behaviour identical (loading phrase cycling, iframe reload guard,
loaded/height reset on html change).
- After your changes, `pnpm lint` must pass with 0 errors and 0 warnings.
- Only edit `apps/app/src/components/generative-ui/widget-renderer.tsx` and
`apps/app/src/app/globals.css`.
````

</details>
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
# Chalk Agent Self-Review Findings

- Branch: fix/lint-errors-2026-03-18
- Commit: b834bc2
- Base: 6df7350 (merge-base with main)
- Session: pr-13
- PR: #13 — fix: resolve lint errors in widget-renderer.tsx and layout.tsx
- Date: 2026-03-19 UTC

---

## Verdict
- Block merge: **yes**
- Blocking findings: P0=2, P1=1

---

## Findings

| ID | Severity | Category | File:Line | Issue | Failure mode | Suggested fix | Confidence |
|---|---|---|---|---|---|---|---|
| F-01 | P0 | Lint / Correctness | [widget-renderer.tsx:417](apps/app/src/components/generative-ui/widget-renderer.tsx#L417) | `setIndex(0)` called synchronously inside `useEffect` body — violates `react-hooks/set-state-in-effect`; lint exits non-zero | CI lint gate fails; PR goal unmet | Move reset out of effect: use a derived index or call `setIndex` only inside the `setInterval` callback (start at 0, interval handles cycling). Or split into two `useEffect`s — one that watches `active` going true and one that runs the interval. | High |
| F-02 | P0 | Lint / Correctness | [widget-renderer.tsx:465-466](apps/app/src/components/generative-ui/widget-renderer.tsx#L465) | `setLoaded(false)` and `setHeight(0)` called synchronously inside `useLayoutEffect` body — same rule violation | CI lint gate fails | Store `loaded`/`height` as refs for the reset path, or batch by computing a single state object. Since these resets only happen when `html` changes, a combined reducer state (`{ loaded: bool, height: number }`) reset in a single `setState` call may satisfy the linter. Alternatively, `startTransition` wrapping won't help here — the real fix is avoiding direct setState in the effect body. | High |
| F-03 | P1 | Lint / Style | [widget-renderer.tsx:431](apps/app/src/components/generative-ui/widget-renderer.tsx#L431) | `_description` renamed with underscore prefix to suppress unused-var, but ESLint still warns — the project's `@typescript-eslint/no-unused-vars` config doesn't honor the `_` prefix convention here | Warning remains in lint output | Simply omit `description` from the destructuring: `{ title, html }`. If the prop must remain in the TypeScript type for API compatibility, add `// eslint-disable-next-line @typescript-eslint/no-unused-vars` or configure `argsIgnorePattern: "^_"` in the ESLint config. | High |
| F-04 | P2 | Correctness / DX | [layout.tsx:13](apps/app/src/app/layout.tsx#L13) & [globals.css:63](apps/app/src/app/globals.css#L63) | Next.js font variable `--font-plus-jakarta` is injected onto `<body>` but `globals.css` references the font via hardcoded string `'Plus Jakarta Sans'` in `--font-family`, not `var(--font-plus-jakarta)`. Next.js font optimization (self-hosting, preloading, CLS prevention) is wired up but the CSS never consumes it. | Font still loads (hardcoded name falls back to Google CDN or cached), but Next.js's optimized self-hosted font is unused — the whole point of migrating away from the `<link>` tag is lost. | In `globals.css` change `--font-family: 'Plus Jakarta Sans', ...` to `--font-family: var(--font-plus-jakarta), system-ui, sans-serif` | High |
| ~~F-05~~ | ~~P2~~ | ~~Docs~~ | ~~CONTRIBUTING.md~~ | ~~RETRACTED~~ — `CopilotKit/OpenGenerativeUI` is the correct official repo slug. Links are accurate. | — | — | — |

---

## Testing Gaps
- No tests added or modified; the two changed source files have no automated test coverage
- No TypeScript build (`pnpm build`) was run to confirm type safety after prop rename

## Open Questions
- Should `react-hooks/set-state-in-effect` be disabled for specific lines as a short-term fix, or is there appetite to restructure the hooks? The `useLoadingPhrase` and `useLayoutEffect` cases have different ideal solutions.
- Is `WidgetRendererProps` exported or used externally? If `description` is part of a public API contract (e.g. CopilotKit tool parameter schema), removing it from destructuring is safe but the prop type should remain for consumers.

---

<details>
<summary><strong>Ask AI to Resolve These Issues</strong></summary>

Copy and paste the prompt below into Claude, ChatGPT, Cursor, or any AI assistant. It includes full context so the AI can fix all remaining lint failures without additional back-and-forth.

---

````
I need help fixing 3 remaining ESLint failures in a Next.js / React project.
The branch is `fix/lint-errors-2026-03-18` on https://github.com/CopilotKit/OpenGenerativeUI.
Running `pnpm lint` currently exits with 2 errors and 1 warning — all in one file:
apps/app/src/components/generative-ui/widget-renderer.tsx

The ESLint config extends `eslint-config-next/core-web-vitals` and `eslint-config-next/typescript`,
which pulls in `eslint-plugin-react-hooks@7.0.1`. That version introduced a new rule —
`react-hooks/set-state-in-effect` — that flags any synchronous setState call made directly inside
an effect body (useEffect or useLayoutEffect).

─── FAILURE 1 (Error) — widget-renderer.tsx ~line 414 ───────────────────────

The `useLoadingPhrase` hook manages a cycling loading message. When `active` flips from false → true
it should reset to phrase index 0, then start an interval to cycle phrases every 1800ms.

Current (broken) code:
```tsx
function useLoadingPhrase(active: boolean) {
const [index, setIndex] = useState(0);
const prevActiveRef = useRef<boolean | null>(null);

useEffect(() => {
if (active && !prevActiveRef.current) {
setIndex(0); // ← lint error: setState inside effect body
}
prevActiveRef.current = active;

if (!active) return;
const interval = setInterval(() => {
setIndex((i) => (i + 1) % LOADING_PHRASES.length);
}, 1800);
return () => clearInterval(interval);
}, [active]);

return LOADING_PHRASES[index];
}
```

Fix goal: eliminate the synchronous `setIndex(0)` call from inside the effect body while keeping
the same observable behaviour (index resets to 0 when active starts, then cycles).

─── FAILURE 2 (Error) — widget-renderer.tsx ~line 460 ───────────────────────

The `WidgetRenderer` component writes HTML into a sandboxed iframe imperatively (to preserve iframe
JS state across React re-renders). When `html` changes it resets `loaded` and `height` back to
false/0 so the loading overlay re-appears.

Current (broken) code:
```tsx
useLayoutEffect(() => {
if (!html || !iframeRef.current) return;
if (html === committedHtmlRef.current) return;
committedHtmlRef.current = html;
iframeRef.current.srcdoc = assembleDocument(html);
setLoaded(false); // ← lint error: setState inside effect body
setHeight(0); // ← lint error: setState inside effect body
}, [html]);
```

State declarations (for context):
```tsx
const [height, setHeight] = useState(0);
const [loaded, setLoaded] = useState(false);
```

Fix goal: eliminate synchronous setState calls from inside the useLayoutEffect body while keeping
the reset-on-html-change behaviour. The iframe srcdoc write must stay synchronous (that's why
useLayoutEffect is correct here). Consider combining loaded+height into a single reducer, or
storing reset state in refs and deriving render values from them.

─── FAILURE 3 (Warning) — widget-renderer.tsx ~line 431 ───────────────────────

The `description` prop is accepted in the function signature but never used. A previous attempt
renamed it to `_description` but the project's ESLint config does not have `argsIgnorePattern: "^_"`
configured so the warning still fires.

Current (broken) code:
```tsx
export function WidgetRenderer({ title, _description, html }: WidgetRendererProps) {
```

Fix goal: silence the unused-var warning. The simplest fix is to remove `description` / `_description`
from the destructuring entirely (just use `{ title, html }`). The prop can remain in the
`WidgetRendererProps` type for API compatibility — only the destructuring binding needs to change.

─── Constraints ───────────────────────────────────────────────────────────────

- Do NOT add eslint-disable comments as a workaround — fix the code properly.
- Keep all existing observable behaviour identical (loading phrase cycling, iframe reload guard,
loaded/height reset on html change).
- After your changes, `pnpm lint` must pass with 0 errors and 0 warnings.
- Only edit `apps/app/src/components/generative-ui/widget-renderer.tsx`.
````

</details>
Loading
Loading