Skip to content

feat: inline chunk editor and table batch ops with undo/redo#3504

Merged
waleedlatif1 merged 12 commits intofeat/mothership-copilotfrom
feat/inline-chunk-editor-and-table-batch-ops
Mar 10, 2026
Merged

feat: inline chunk editor and table batch ops with undo/redo#3504
waleedlatif1 merged 12 commits intofeat/mothership-copilotfrom
feat/inline-chunk-editor-and-table-batch-ops

Conversation

@waleedlatif1
Copy link
Collaborator

@waleedlatif1 waleedlatif1 commented Mar 10, 2026

Summary

  • Inline Chunk Editor: Replace EditChunkModal and CreateChunkModal with an inline editor that renders within the Resource layout, matching the Files tab pattern (state-based view toggle with ResourceHeader + ChunkEditor)
  • Table Batch Operations: Add PATCH endpoint for batch row updates, undo/redo support via useTableUndo hook and Zustand store, and batch insert with position support
  • Popover Context Menus: Migrate table context menus from DropdownMenu to Popover components with disable flags, and add view schema/rename actions to table context menu

Test plan

  • Click a chunk → inline editor appears with correct breadcrumbs and content
  • Edit content → "Save" button enables, Cmd+S works
  • Click back breadcrumb with unsaved changes → discard modal appears
  • Prev/next chunk navigation works, including across page boundaries
  • "New Chunk" opens inline editor in create mode with auto-focused cursor
  • Tokenizer toggle works with consistent font sizing
  • Context menu actions (edit, enable/disable, delete) work correctly
  • Table batch update API returns correct response
  • Table undo/redo (Cmd+Z / Cmd+Shift+Z) works for row operations
  • Table context menus render correctly with Popover components

Replace modal-based chunk editing/creation with inline editor following
the files tab pattern (state-based view toggle with ResourceHeader).
Add batch update API endpoint, undo/redo support, and Popover-based
context menus for tables.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vercel
Copy link

vercel bot commented Mar 10, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Mar 10, 2026 8:55am

Request Review

@cursor
Copy link

cursor bot commented Mar 10, 2026

PR Summary

Medium Risk
Moderate risk due to a new PATCH /api/table/[tableId]/rows batch-update endpoint and a broad refactor of Knowledge Base/Document UIs onto the shared Resource shell, which could affect table row ordering and chunk editing flows.

Overview
Adds table batch row operations: batch inserts can now accept per-row positions (validated for length/uniqueness) and a new PATCH /api/table/[tableId]/rows endpoint performs up-to-1000 row updates by ID with permission/workspace checks and structured error handling.

Refactors Knowledge Base documents list and Document chunk list/editor onto the shared Resource layout: Resource now supports breadcrumbs, optional external sort, row selection via checkbox column, an extras slot in the options bar, and built-in pagination; Knowledge Base gets inline rename and connector badges/modal within the header/filter area.

Replaces EditChunkModal/CreateChunkModal with a new inline ChunkEditor (edit/create modes, tokenizer view, Cmd+S save, dirty-state/unsaved-changes guard, and prev/next chunk navigation across pages) and updates chunk context menus to support disabling edit actions.

Written by Cursor Bugbot for commit bb73f62. Configure here.

Icons were incorrectly carried over from the DropdownMenu migration.
PopoverItems in this codebase use text-only labels.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The table-level context menu was incorrectly migrated to Popover during
conflict resolution. Only the row-level context menu uses Popover; the
table context menu should remain DropdownMenu with icons, matching the
base branch.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@waleedlatif1
Copy link
Collaborator Author

@greptile

@waleedlatif1
Copy link
Collaborator Author

@cursor review

Prevent indefinite polling if page data never loads during
chunk navigation across page boundaries.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 10, 2026

Greptile Summary

This PR delivers three major features: an inline chunk editor replacing the EditChunkModal/CreateChunkModal dialogs with a state-driven view toggle inside the existing Resource layout; table batch operations including a new PATCH /api/table/[tableId]/rows endpoint, a useTableUndo hook backed by a Zustand store for full undo/redo support, and positions support for batch inserts; and Popover-based context menus across tables and knowledge base views with consistent disable flags.

Key points:

  • The shared Resource component is significantly enhanced with SelectableConfig, PaginationConfig, sortOverride, breadcrumbs, and extras props — making it the single layout primitive for KB base, document, and table views.
  • The undo/redo design correctly handles the stale-row-ID problem after create-row redo (via patchUndoRowId) and delete-rows undo (via patchRedoRowId), and uses a runWithoutRecording mutex to prevent recursive stack entries.
  • One logic bug identified: handleChunkCreated in document.tsx captures totalPages from a stale closure. useCreateChunk uses invalidateQueries (async background refetch), so when onCreated fires the component has not yet re-rendered with the new page count. If the newly created chunk is the first on a new page, goToPage(totalPages) navigates to the wrong page and setSelectedChunkId leaves the UI stuck in "Loading chunk…" indefinitely.

Confidence Score: 3/5

  • PR is mostly safe to merge; one logic bug in handleChunkCreated can cause a stuck loading state in an edge case.
  • The undo/redo system, batch PATCH endpoint, and Resource component enhancements are all well-implemented and address previously flagged issues. The one unfixed logic bug — stale totalPages in handleChunkCreated causing a permanently stuck "Loading chunk…" state when a new chunk creates a new page — is a real user-facing failure path, but it only triggers in an edge case (creating the first chunk of a new page). All other prior review issues have been addressed.
  • apps/sim/app/workspace/[workspaceId]/knowledge/[id]/[documentId]/document.tsx — handleChunkCreated stale totalPages closure (lines 505-517).

Important Files Changed

Filename Overview
apps/sim/app/api/table/[tableId]/rows/route.ts Adds PATCH handler for batch row updates with proper auth, workspace validation, schema validation via BatchUpdateByIdsSchema, and distinct error message routing; also adds positions array support to the existing batch insert path.
apps/sim/app/workspace/[workspaceId]/knowledge/[id]/[documentId]/document.tsx Replaces EditChunkModal/CreateChunkModal with an inline editor view; migrates chunk table to the Resource component pattern. Contains a stale-closure bug in handleChunkCreated that causes an infinite "Loading chunk…" spinner when a newly created chunk lands on a new page.
apps/sim/app/workspace/[workspaceId]/knowledge/[id]/[documentId]/components/chunk-editor/chunk-editor.tsx New inline chunk editor component extracted from the old modals; handles create/edit modes, tokenizer toggle, dirty tracking, and exposes a saveRef imperative handle. Clean implementation.
apps/sim/hooks/use-table-undo.ts New hook wiring the undo/redo Zustand store to React Query mutations; correctly handles the stale row-ID problem for create-row redo via patchUndoRowId and for delete-rows undo via patchRedoRowId.
apps/sim/stores/table/store.ts New Zustand store for ephemeral per-table undo/redo stacks; includes patchUndoRowId and patchRedoRowId helpers, a 100-entry FIFO capacity guard, and a runWithoutRecording mutex to prevent recursive undo entries.
apps/sim/app/workspace/[workspaceId]/components/resource/resource.tsx Adds SelectableConfig, PaginationConfig, sortOverride, breadcrumbs, and extras props to the shared Resource layout; implements a sliding-window Pagination component and per-row checkbox selection — all using design-system CSS variable tokens consistently.
apps/sim/app/workspace/[workspaceId]/knowledge/[id]/base.tsx Major refactor migrating the Knowledge Base document list from a custom table to the shared Resource component; replaces old loading skeletons, pagination, and filter popover with Resource props; moves connectors section into a modal.
apps/sim/lib/table/service.ts Adds batchUpdateRows service function with row existence validation, per-row size/schema checks, unique-constraint enforcement, and a bulk Drizzle UPDATE for all rows in one DB transaction.

Sequence Diagram

sequenceDiagram
    participant U as User
    participant Doc as Document.tsx
    participant CE as ChunkEditor
    participant API as POST /api/.../chunks
    participant Store as QueryCache

    U->>Doc: Click "New chunk"
    Doc->>Doc: setIsCreatingNewChunk(true)
    Doc-->>CE: render ChunkEditor (mode=create)
    U->>CE: type content, Cmd+S
    CE->>API: createChunk(content)
    API-->>CE: { id: newChunkId }
    CE->>Store: invalidateQueries (async)
    CE->>Doc: onCreated(newChunkId)
    Doc->>Doc: handleChunkCreated runs\n(totalPages = stale value)
    Doc->>Doc: goToPage(staleTotalPages)
    Note over Doc: If new chunk is on page N+1\nbut code navigates to page N,\nselectedChunk = null → stuck spinner

    U->>Doc: (Table view) right-click row
    Doc->>Doc: handleChunkContextMenu(e, rowId)
    Doc-->>Doc: setContextMenuChunk(chunk)
    U->>Doc: click "Edit"
    Doc->>Doc: setSelectedChunkId(chunk.id)
    Doc-->>CE: render ChunkEditor (mode=edit)
    U->>CE: edit content, Cmd+S
    CE->>API: updateChunk(chunkId, content)
    API-->>CE: success
    CE->>Store: invalidateQueries (async)
    Doc->>Doc: setSaveStatus("saved")
Loading

Last reviewed commit: bb73f62

After creating a chunk, navigate to the last page (where new chunks
append) before selecting it. This prevents the editor from showing
"Loading chunk..." when the new chunk is not on the current page.
The loading state breadcrumb remains as an escape hatch for edge cases.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds a .refine() check to reject duplicate rowIds in batch update
requests, consistent with the positions uniqueness check on batch insert.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix disableEdit logic: use || instead of && so connector doc chunks
  cannot be edited from context menu (row click still opens viewer)
- Add uniqueness validation for rowIds in BatchUpdateByIdsSchema
- Fix inconsistent bg token: bg-background → bg-[var(--bg)] in Pagination

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@waleedlatif1
Copy link
Collaborator Author

@greptile

@waleedlatif1
Copy link
Collaborator Author

@cursor review

The refine was applied both on the inner updates array and the outer
object. Keep only the inner array refine which is cleaner.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix stale rowId after create-row redo: patch undo stack with new row
  ID using patchUndoRowId so subsequent undo targets the correct row
- Fix text color tokens in Pagination: use CSS variable references
  (text-[var(--text-body)], text-[var(--text-secondary)]) instead of
  Tailwind semantic tokens for consistency with the rest of the file

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove unused `onAddData` prop and `isEmptyCell` variable from row context
menu (introduced in PR but never wired to JSX). Fix type errors in
optimistic update spreads by removing unnecessary `as Record<string, unknown>`
casts that lost the RowData type.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@waleedlatif1
Copy link
Collaborator Author

Addressed the dead onAddData prop and isEmptyCell variable flagged in the summary — removed both from the context menu component and the unused handleAddData callback from table.tsx. Also fixed type errors in the optimistic update spreads in tables.ts. See commit 34caf3b.

@waleedlatif1
Copy link
Collaborator Author

@greptile

@waleedlatif1
Copy link
Collaborator Author

@cursor review

…d-forget goToPage calls

ChunkEditor.handleSave now throws on empty/oversized content instead of
silently returning, so the parent's catch block correctly sets saveStatus
to 'error'. Also added explicit `void` to unawaited goToPage(1) calls
in filter handlers to signal intentional fire-and-forget.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@waleedlatif1
Copy link
Collaborator Author

@greptile

@waleedlatif1
Copy link
Collaborator Author

@cursor review

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

…case

When creating a chunk that spills onto a new page, totalPages in the
closure is stale. Now polls displayChunksRef for the new chunk, and if
not found, checks totalPagesRef for an updated page count and navigates
to the new last page before continuing to poll.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@waleedlatif1 waleedlatif1 merged commit 8afa184 into feat/mothership-copilot Mar 10, 2026
3 checks passed
@waleedlatif1 waleedlatif1 deleted the feat/inline-chunk-editor-and-table-batch-ops branch March 10, 2026 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant