feat(console): CodeEditor completions prop for as-you-type suggestions - #597
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 33 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe PR adds a registered read-only database browser with schema exploration, paginated table data, row inspection, SQL execution, query history, and responsive styling. It also adds optional Monaco completion terms to the shared code editor. ChangesDatabase browser
Editor completions
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
skill-check — worker0 verified, 49 skipped (no docs/).
Four for four. Nicely done. |
rohitg00
added
the
no-ticket
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@console/web/src/components/ui/CodeEditor.tsx`:
- Around line 221-224: Update the completions cleanup logic around
editor.updateOptions to capture the editor’s original quickSuggestions and
wordBasedSuggestions values before applying completion-specific options, then
restore both values when completions becomes empty. Preserve the existing
behavior for active completions while ensuring cleanup returns the editor to its
prior completion settings.
- Around line 227-246: Update the completion provider created in CodeEditor’s
monaco.languages.registerCompletionItemProvider call to return no suggestions
unless the provideCompletionItems model is the owning editor model. Compare the
callback’s model against the editor’s current model, then preserve the existing
words mapping and range behavior for the owning model.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6b40c2e4-2a9c-45c6-a2fd-31a347f41811
📒 Files selected for processing (2)
console/web/src/components/ui/CodeEditor.tsxpackages/console-ui/index.d.ts
Adds an opt-in `completions?: readonly string[]` to the shared CodeEditor (#579). Non-empty turns on the suggest popup (the default stays prose-quiet, so markdown/json/yaml behavior is byte-for-byte unchanged) and registers a Monaco completion provider for the editor's language offering those words, disposed on unmount. The database SQL panel is the first consumer, feeding it the live table names plus SQL keywords.
bbb1442 to
9a4a1ed
Compare
…598) * feat(database): injectable console page (schema browser + SQL panel) Ships the database worker's #/ext/database page over the injectable-UI protocol (#570): a schema tree with lazy per-driver column/PK/FK/index introspection, a sortable paginated row grid with a row inspector, and a read-only SQL panel (shared Monaco CodeEditor, EXPLAIN, per-db history, client-side write-verb gate). Reads the live database via database::query and database::listDatabases. Registered through host.pages.register in the existing database/ui setup, alongside the function-trigger renderer. The SQL editor feeds the live table names plus SQL keywords to the CodeEditor completions prop for as-you-type suggestions. * fix(database): address review on the injectable page - read-only SQL gate hardened: single-statement only (semicolons in strings/comments ignored), reject any write/DDL keyword anywhere so mutating CTEs and EXPLAIN ANALYZE cannot slip past the leading keyword, and reject writable PRAGMA forms (journal_mode=WAL) while keeping read PRAGMAs - listDbs throws on a shape mismatch like runQuery, so a malformed response is distinguishable from an empty database list - fetchTablePage fetches a pageSize+1 sentinel row so pagination stays bounded when the row count is unknown; normalize page/size to non-negative integers before interpolating - copyText swallows the async clipboard rejection via ?.catch, not just the sync call - icons preserve caller aria-hidden/aria-label (labeled icons stay accessible) instead of forcing aria-hidden - result grid sets aria-sort on sortable headers Worker-side read-only transaction enforcement is out of scope here (it is a database worker change); this tightens the client gate as defense in depth. SqlPanel already remounts per database via its key, so no separate db-change reset is needed.
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
database/ui/src/page/SchemaTree.tsx (1)
65-82: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winFailed schema loads never retry.
The guard only skips when the entry is
undefined, so once a table is marked'error're-expanding it is a no-op and the user is stuck with "failed to read schema" until the parent remounts. Allow a retry when the cached value is'error'.♻️ Retry on re-expand after an error
- if (schemaByTable[table.name] === undefined) { + const cached = schemaByTable[table.name] + if (cached === undefined || cached === 'error') { setSchemaByTable((cur) => ({ ...cur, [table.name]: 'loading' }))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@database/ui/src/page/SchemaTree.tsx` around lines 65 - 82, Update the schema-loading guard in the expansion logic around tableColumns and tableIndexes so loading starts when schemaByTable[table.name] is undefined or 'error'. Preserve the existing loading, success, and error state transitions so re-expanding a failed table retries the request.database/ui/src/page/result-grid.tsx (2)
240-254: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCopy-on-click is silently disabled whenever
onRowClickis set.In that mode cells render bare content, so
copied/copyCellare unreachable — yet the file header states every cell copies its value on click. SinceTableDataPanelalways passesonRowClick, the copy affordance never exists in the table browser. Either update the header comment to describe the two modes, or keep a non-nested copy trigger (e.g. copy on the cell'sdblclick, or a hover icon) so the behavior matches the docs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@database/ui/src/page/result-grid.tsx` around lines 240 - 254, Update the cell rendering in the result-grid component so copyCell remains reachable when onRowClick is provided, preserving row-click behavior while adding a separate copy trigger such as double-click or a hover control; alternatively revise the file header to accurately document the two interaction modes. Ensure the implemented behavior matches the chosen documentation and does not leave copied state unreachable.
139-145: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated "copied for 1200ms" logic in
result-grid.tsxandRowDetail.tsx. Both sites reimplement the same copy-then-flag-then-timeout block; the shared piece belongs next tocellText/copyTextincells.tsas a smalluseCopyFeedback()hook (which would also give a single place to clear the timer).
database/ui/src/page/result-grid.tsx#L139-L145: replacecopyCelland the localcopiedstate with the shared hook.database/ui/src/page/RowDetail.tsx#L26-L32: replacecopyValueand the localcopiedstate with the same hook.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@database/ui/src/page/result-grid.tsx` around lines 139 - 145, The copy-feedback behavior is duplicated across both components. Add a shared useCopyFeedback() hook beside cellText/copyText in cells.ts that performs copying, tracks the copied key/value, and manages a single timeout cleanup; then replace copyCell and its local copied state in database/ui/src/page/result-grid.tsx#L139-L145 and copyValue and its local copied state in database/ui/src/page/RowDetail.tsx#L26-L32 with the hook.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@database/ui/src/page/index.tsx`:
- Around line 89-94: Update the subtitle rendering around activeDb?.url so the
fallback is gated on whether activeDb exists, not whether its optional url is
truthy: show the configured database state when activeDb is present without a
URL, and retain the worker-not-connected and no-database-configured messages for
the appropriate absent-database cases.
- Around line 206-207: Update the SqlPanel key in the active database rendering
to use only activeDb.name, removing bump from the key so refreshes reload
metadata without remounting SqlPanel or resetting its in-progress SQL editor
state.
- Around line 235-240: Export the existing quoteTableRef helper from db-data.ts
and update the onOpenInSql handler to use quoteTableRef(activeDb.driver, table)
when constructing seed SQL. Keep quoteIdent for single identifiers, but ensure
schema-qualified PostgreSQL table names are quoted component-wise.
In `@database/ui/src/page/result-grid.tsx`:
- Around line 222-227: Update the row rendering in the result-grid table around
the onRowClick handler so clickable rows are focusable and expose appropriate
row semantics, and add keyboard activation for Enter and Space that invokes
onRowClick with the same index as mouse clicks. Preserve non-clickable row
behavior and avoid triggering activation for unrelated keys.
In `@database/ui/src/page/SqlPanel.tsx`:
- Around line 126-130: Update the history state update in the setHistory
callback to enforce HISTORY_LIMIT before returning the list, while continuing to
persist the same bounded history through saveHistory. Ensure both component
state and storage retain only the most recent entries after deduplication.
- Line 109: Update the history state flow in SqlPanel around the history
useState initializer so it reloads from loadHistory(db) whenever the selected db
changes, replacing stale entries from the previous database and preserving the
existing database-keyed persistence behavior.
---
Nitpick comments:
In `@database/ui/src/page/result-grid.tsx`:
- Around line 240-254: Update the cell rendering in the result-grid component so
copyCell remains reachable when onRowClick is provided, preserving row-click
behavior while adding a separate copy trigger such as double-click or a hover
control; alternatively revise the file header to accurately document the two
interaction modes. Ensure the implemented behavior matches the chosen
documentation and does not leave copied state unreachable.
- Around line 139-145: The copy-feedback behavior is duplicated across both
components. Add a shared useCopyFeedback() hook beside cellText/copyText in
cells.ts that performs copying, tracks the copied key/value, and manages a
single timeout cleanup; then replace copyCell and its local copied state in
database/ui/src/page/result-grid.tsx#L139-L145 and copyValue and its local
copied state in database/ui/src/page/RowDetail.tsx#L26-L32 with the hook.
In `@database/ui/src/page/SchemaTree.tsx`:
- Around line 65-82: Update the schema-loading guard in the expansion logic
around tableColumns and tableIndexes so loading starts when
schemaByTable[table.name] is undefined or 'error'. Preserve the existing
loading, success, and error state transitions so re-expanding a failed table
retries the request.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4551f869-81b9-488b-89e1-63129bf49892
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
console/web/src/components/ui/CodeEditor.tsxdatabase/ui/package.jsondatabase/ui/page.tsxdatabase/ui/src/page/RowDetail.tsxdatabase/ui/src/page/SchemaTree.tsxdatabase/ui/src/page/SqlPanel.tsxdatabase/ui/src/page/TableDataPanel.tsxdatabase/ui/src/page/cells.tsdatabase/ui/src/page/db-data.tsdatabase/ui/src/page/icons.tsxdatabase/ui/src/page/index.tsxdatabase/ui/src/page/page-styles.tsdatabase/ui/src/page/pagination.tsxdatabase/ui/src/page/result-grid.tsxdatabase/ui/src/page/useDatabaseRead.tspackages/console-ui/index.d.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/console-ui/index.d.ts
- console/web/src/components/ui/CodeEditor.tsx
| const [running, setRunning] = useState(false) | ||
| const [error, setError] = useState<string | null>(null) | ||
| const [outcome, setOutcome] = useState<AdhocResult | null>(null) | ||
| const [history, setHistory] = useState<string[]>(() => loadHistory(db)) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reload history when the selected database changes.
Line 109 only loads history on mount. After a db switch, entries from the previous database remain in state and are subsequently saved under the new database’s key.
Proposed fix
+ useEffect(() => {
+ setHistory(loadHistory(db))
+ setHistoryOpen(false)
+ }, [db])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [history, setHistory] = useState<string[]>(() => loadHistory(db)) | |
| const [history, setHistory] = useState<string[]>(() => loadHistory(db)) | |
| useEffect(() => { | |
| setHistory(loadHistory(db)) | |
| setHistoryOpen(false) | |
| }, [db]) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@database/ui/src/page/SqlPanel.tsx` at line 109, Update the history state flow
in SqlPanel around the history useState initializer so it reloads from
loadHistory(db) whenever the selected db changes, replacing stale entries from
the previous database and preserving the existing database-keyed persistence
behavior.
- Subtitle: a database configured without a `url` no longer reads "no database configured"; it falls back to the database name, and the worker-unavailable / nothing-configured copy stays for the absent cases. - SQL panel is keyed by database only, so a refresh reloads metadata instead of remounting the editor and discarding an in-progress statement and its results. - "open in sql" seeds through quoteTableRef, so a schema-qualified postgres table quotes component-wise instead of becoming one identifier. - Selectable grid rows are tabbable and activate on Enter/Space, matching the click path; non-clickable rows are unchanged. - SQL history state is bounded by HISTORY_LIMIT, matching what is persisted. - Re-expanding a table whose schema read failed retries instead of staying on the error row forever. - Copy feedback moves into a shared useCopyFeedback hook that cancels its pending timer on unmount, replacing the duplicate implementations in the grid and the row inspector.
1 participant
Adds an opt-in
completions?: readonly string[]prop to the shared CodeEditor (#579). Non-empty turns on the as-you-type suggest popup (the default stays prose-quiet, so markdown/json/yaml behaviour is byte-for-byte unchanged) and registers a Monaco completion provider for the editor's language offering those words, disposed on unmount.First consumer is the database worker's SQL panel, which feeds it the live table names plus SQL keywords for schema-aware autocomplete.
Test
tsc + biome clean. Verified live in the console SQL editor: typing a keyword or table prefix pops the suggest list, and Ctrl+Space triggers it explicitly. Prose editors (markdown/json) are unaffected since they pass no completions.
Summary by CodeRabbit