-
Notifications
You must be signed in to change notification settings - Fork 208
8.x - Multiple terminologies support #2780
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
update: make `createTable` as a reusable component like `createEntity`.
WalkthroughThe PR refactors the console from a table/column model to an entity/field model across backend, SDK, typings, stores, routing, and UI. It introduces DatabaseType and terminology normalization, a database-focused SDK helper ( Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx,js,jsx,svelte,json}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (3)📓 Common learnings📚 Learning: 2025-09-26T06:48:57.938ZApplied to files:
📚 Learning: 2026-01-06T11:49:07.493ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (1)
✏️ Tip: You can disable this entire section by setting 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 15
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.ts (1)
57-76:Query.orderDesc('')is likely invalid and can break listRows.Proposed direction
- Replace
''with the intended default sort attribute (whatever the console historically used for rows).- If you want “server default order”, omit the order query entirely instead of passing an empty attribute.
// don't override if there's a user created sort! if (!hasOrderQuery) { - queryArray.push(Query.orderDesc('')); + // Pick an actual attribute (or omit ordering entirely to use backend defaults). + queryArray.push(Query.orderDesc('$createdAt')); }src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/editPermissions.svelte (1)
15-27: Guard againstrow === nulland keeppermissionsin sync withrow.
rowdefaults to$bindable(null)(Line 17) but is dereferenced immediately (Line 26) and inupdatePermissions()(Line 35). This will throw if the component renders beforerowis set. Also, ifrowis replaced,permissionswon’t resync.Proposed fix
@@ let { table, - row = $bindable(null), + row = $bindable<Models.DefaultRow | Models.Row | null>(null), arePermsDisabled = $bindable(true) }: { table: Entity; - row: Models.DefaultRow | Models.Row; + row: Models.DefaultRow | Models.Row | null; arePermsDisabled?: boolean; } = $props(); @@ let showPermissionAlert = $state(true); - let permissions = $state(row.$permissions); + let permissions = $state<string[] | undefined>(undefined); + + $effect(() => { + // Keep local state aligned when the bound row changes. + permissions = row?.$permissions; + }); @@ export async function updatePermissions() { try { + if (!row) return; const { $databaseId: databaseId, $tableId: tableId, $id: rowId } = row; @@ - } catch (error) { + } catch (error) { addNotification({ - message: error.message, + message: error instanceof Error ? error.message : String(error), type: 'error' }); trackError(error, Submit.RowUpdatePermissions); } } @@ $effect(() => { - if (permissions) { - arePermsDisabled = !symmetricDifference(permissions, row.$permissions).length; + if (permissions && row?.$permissions) { + arePermsDisabled = !symmetricDifference(permissions, row.$permissions).length; } });Also applies to: 33-58, 60-64
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/deleteColumn.svelte (1)
13-37: Fix null-handling forselectedColumnto avoid crashes.With
selectedColumn = $bindable(null)(Line 16-17), the derived arrays can containnull, and the subsequent.key/isRelationship()usage can throw.Proposed fix
@@ let { table, showDelete = $bindable(false), selectedColumn = $bindable(null) }: { table: Entity; showDelete: boolean; - selectedColumn: Columns | string[]; + selectedColumn: Columns | string[] | null; } = $props(); @@ const selectedColumns = $derived( - Array.isArray(selectedColumn) ? selectedColumn : [selectedColumn] + selectedColumn === null + ? [] + : Array.isArray(selectedColumn) + ? selectedColumn + : [selectedColumn] ); @@ const selectedKeys = $derived( - selectedColumns.map((c: string | Columns) => (typeof c === 'string' ? c : c.key)) + selectedColumns + .filter((c): c is string | Columns => c != null) + .map((c) => (typeof c === 'string' ? c : c.key)) ); @@ const requiresTwoWayConfirm = $derived( selectedColumns - .filter((c): c is Columns => typeof c !== 'string') + .filter((c): c is Columns => c != null && typeof c !== 'string') .some((col) => isRelationship(col) && col.twoWay) ); @@ - } catch (e) { - error = e.message; - trackError(e, Submit.ColumnDelete); + } catch (e) { + error = e instanceof Error ? e.message : String(e); + trackError(e, Submit.ColumnDelete); } }Also applies to: 64-67, 92-102
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/edit.svelte (1)
11-22: Unsafe type narrowing oftable.fieldsitems from(Attributes | Columns)[]toColumnswithout type guard.At line 174, the code iterates over
table.fields(typed as(Attributes | Columns)[]per Entity definition) but assumes all items areColumnswithout runtime validation. ThecompareColumnsfunction expectsColumnsand accessescolumn.key, which may differ in structure betweenAttributesandColumnstypes. Add a type guard or ensure type safety to prevent silent mismatches.src/routes/(console)/project-[region]-[project]/databases/database-[database]/settings/+page.svelte (1)
51-70: Database rename must be made type-aware using the abstraction layer to support bothtablesdbanddocumentsdbdatabases.The hardcoded
sdk.forProject(...).tablesDB.update()call on line 53 breaks fordocumentsdbdatabases. While the codebase already usesdatabaseSdkfor type-aware operations likelistEntities()anddelete()(which properly switch betweentablesDBanddocumentsDB), theupdate()method is not yet exposed in theDatabaseSdkResultinterface.This requires:
- Adding an
update()method toDatabaseSdkResultwith type-aware switching (like the existingdelete()method)- Implementing the type-aware logic in
useDatabasesSdk()to calltablesDB.update()fortablesdbanddocumentsDB.update()fordocumentsdb- Using the abstraction: replace
sdk.forProject(...).tablesDB.update()withdatabaseSdk.update()Note: Also fix the variable naming mismatch in
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/init.tsline 15 wherebuildTerminologies()assigns todatabasesSdkbut the type expectsdatabaseSdk.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/editRelated.svelte (1)
90-116: Inconsistent property access between tables/entities and columns/fields.There are inconsistencies in this section:
- Line 91 checks
page.data.tablesbut Line 106 usespage.data.entities- Line 114 accesses
rowTable.columnsbut the Entity type definesfieldsSuggested fix
const uniqueTableIds = [...new Set(existingRows.map((row) => row.$tableId))]; const missingTableIds = uniqueTableIds.filter((tableId) => { - return !page.data.tables?.find((table: Models.Table) => table.$id === tableId); + return !page.data.entities?.find((entity: Entity) => entity.$id === tableId); });- const hasAllColumns = rowTable.columns.every( - (column: Columns) => column.key in row + const hasAllColumns = rowTable.fields?.every( + (field: Field) => field.key in row );src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte (1)
278-381: createFakeData mixesfieldsvscolumns(likely broken after invalidate).Within the same function you treat the schema as
table.fields, but afterinvalidate(Dependencies.TABLE)you readpage.data.table.columns as Columns[], and you also castcurrentFields as Columns[]. That’s very likely wrong post-refactor (and it also underminesgenerateFakeRecords(fields: Field[])).Also, since you now derive
databaseType, please confirm this layout is only used forlegacy/tablesdb. Ifdocumentsdbcan reach this code path, the hardcodedtablesDB.createRows/createRow/createIndexcalls will be incorrect. (Based on learnings: tables always havefieldsand vectordb may be unused today, but this function still needs to be internally consistent.)Proposed direction (keep everything consistently “fields”)
- let columns: Field[] = []; - const currentFields = table.fields; - const hasAnyRelationships = currentFields.some((field: Field) => isRelationship(field)); - const filteredColumns = currentFields.filter( + let fields: Field[] = []; + const currentFields = table.fields; + const filteredFields = currentFields.filter( (field: Field) => field.type !== 'relationship' ); - if (!filteredColumns.length) { + if (!filteredFields.length) { try { const { startWaiting, waitPromise, columnCreationHandler: handler } = setupColumnObserver(); columnCreationHandler = handler; - columns = await generateFields( + fields = await generateFields( $project, page.params.database, page.params.table, databaseType ); - startWaiting(columns.length); + startWaiting(fields.length); await waitPromise; await invalidate(Dependencies.TABLE); - columns = page.data.table.columns as Columns[]; + fields = page.data.table.fields as Field[]; trackEvent(Submit.ColumnCreate, { type: 'faker' }); } catch (e) { addNotification({ type: 'error', message: e.message }); $spreadsheetLoading = false; return; } finally { columnCreationHandler = null; } } else { - columns = currentFields as Columns[]; + fields = currentFields; } let rowIds = []; try { - const { rows, ids } = generateFakeRecords(columns, $randomDataModalState.value); + const hasAnyRelationships = fields.some((field: Field) => isRelationship(field)); + const { rows, ids } = generateFakeRecords(fields, $randomDataModalState.value);
🤖 Fix all issues with AI agents
In @src/lib/helpers/faker.ts:
- Around line 8-82: The generateFields function can return undefined for the
'documentsdb' and 'vectordb' branches because they just break without returning;
change those branches so the function always returns a Field[] (e.g., return an
empty array for databases that don't require individual fields) and/or add a
final explicit return of an empty array after the switch; update the switch
cases for 'documentsdb' and 'vectordb' in generateFields to return [] instead of
breaking so all code paths return a Field[].
In
@src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/init.ts:
- Around line 16-39: The returned object from buildTerminologies uses the wrong
property name `databasesSdk` while the Terminologies type and consumers expect
`databaseSdk`, causing runtime undefined; change buildTerminologies to return
`databaseSdk: useDatabasesSdk(page, terminology)` (and ensure any other
references use `databaseSdk`), so setTerminologies stores the context with the
correctly named property and getTerminologies will return a fully populated
Terminologies object.
In
@src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.ts:
- Around line 1-3: The compile error is caused by importing baseTerminology with
"import type" but then using it in a value context (keyof typeof
baseTerminology); change the import for baseTerminology to a regular value
import (remove "type" so baseTerminology is present in the value namespace)
wherever it appears (e.g., the import line referencing baseTerminology and the
other occurrences around lines 34-38) so that keyof typeof baseTerminology can
be evaluated at compile time.
In
@src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/header.svelte:
- Around line 43-63: basePath currently dereferences entity.$id directly inside
the $derived.by callback which can crash when entity is transiently undefined;
update the basePath derivation to guard against missing entity (use optional
chaining or an early return) so you only call resolveRoute/withPath when
entity?.$id exists, otherwise return a safe fallback (null or empty string) to
match the template's optional handling; ensure you reference the same symbols
(basePath, $derived.by, entity.$id, resolveRoute, withPath, page.params) when
making the check.
- Around line 54-63: The derived store nonSheetPages accesses
terminology.field.lower.plural and terminology.entity.lower.singular without
guarding for missing terminology.field (or .lower), which can crash during route
transitions; update the callback in nonSheetPages to use optional chaining or
safe defaults (e.g., terminology.field?.lower?.plural and
terminology.entity?.lower?.singular or fallbacks like empty strings) before
building resourceSeg and endings so page.route.id?.endsWith checks never operate
on undefined values and the component remains transition-safe.
In
@src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/delete.svelte:
- Around line 19-45: getKeys currently assumes selected is never null and
selectedKeys is read after selectedIndex is cleared, causing crashes and
incorrect counts; update getKeys to accept null/undefined and return an empty
array (i.e., handle Array.isArray(selected) or null -> []), and in cleanup()
capture the keys up-front (e.g., const keys = getKeys(selectedIndex) or snapshot
$selectedKeys) before resetting selectedIndex and showDelete, then use that
captured keys variable for the notification message and any logic; reference
getKeys, selectedKeys, cleanup, and selectedIndex when making these changes.
In
@src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/overview.svelte:
- Around line 6-14: selectedIndex is declared non-nullable but initialized to
null and field-row label uses entity singular (table) instead of a field/column
label; also direct indexing into selectedIndex.orders/lengths can yield
undefined if arrays drift. Change the prop type to allow null (e.g.,
selectedIndex: Index | null), update the label to use the field/column
terminology (or a constant like "Column") instead of
terminology.entity.title.singular, and guard accesses to selectedIndex.orders[i]
and selectedIndex.lengths[i] with optional chaining and sensible defaults (e.g.,
selectedIndex?.orders?.[i] ?? '' or iterate up to Math.max/Math.min of lengths
with fallbacks) so you never read undefined; apply these fixes where
selectedIndex is read (the selectedIndex prop declaration and the rendering loop
that reads orders/lengths).
In
@src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/settings/security.svelte:
- Around line 30-38: The catch block in updateSecurity uses error.message
directly and passes error to trackError, which can throw if error is non-Error;
change it to treat the caught value as unknown, derive a safe message via const
msg = error instanceof Error ? error.message : String(error), call
addNotification({ message: msg, type: 'error' }), and ensure trackError receives
an Error (e.g. trackError(error instanceof Error ? error : new Error(msg),
analytics.submit.entity('UpdateSecurity'))); update the catch in updateSecurity
accordingly.
In
@src/routes/(console)/project-[region]-[project]/databases/database-[database]/+layout.svelte:
- Around line 140-155: createEntity currently always calls tablesDB.createTable
and navigates to the hard-coded `/table-${id}` route; change it to use the new
entity-aware API and routing: replace the direct tablesDB.createTable call with
the generic entity creation method on the SDK (use the entity/terminology
routing layer introduced in the PR) and build the destination URL using the
terminology helper (instead of `/table-${table.$id}`) so DocumentsDB uses its
document-creation path; update references to tablesDB.createTable and the
`/table-` route in createEntity to the new SDK function and terminology-based
route builder (keep invalidation of Dependencies.DATABASE and subsequent goto
logic intact).
- Around line 155-156: setTerminologies(page) is being called unguarded and
triggers useTerminology(page) to throw when page.data?.database?.type is missing
during route transitions; update the reactive statement to first check for the
presence of the required data (e.g., page?.data?.database?.type or
page?.data?.database) before calling setTerminologies(page), or wrap the call in
a safe conditional (or try/catch) so setTerminologies(page) runs only when the
database type exists.
In
@src/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/+page.svelte:
- Around line 28-34: The component declares policyCreateError but never sets it
while using bind:error on the form — either assign errors to policyCreateError
when submissions fail or remove bind:error; also defensively access error
messages (e.g., use (err && err.message) or String(err) instead of err.message)
wherever you read err.message (tags: policyCreateError, bind:error, err.message,
isDisabled) so null/undefined errors don't crash; apply same fixes to the other
occurrences mentioned (lines ~68-89, ~120-161, ~238-249).
In
@src/routes/(console)/project-[region]-[project]/databases/database-[database]/settings/+page.svelte:
- Around line 36-43: The issue is that calling loadEntityCount() directly in the
template’s #await causes a new API call on every re-render; instead create and
store the promise or the resolved value in component state (e.g., const
entityCountPromise = loadEntityCount() or a writable/let entityCount) when the
component initializes or when dependencies change, then use that cached
promise/value in the #await expression; apply the same change for the second
occurrence referenced around lines 121-127 so both places reuse the cached
promise/value rather than invoking loadEntityCount() repeatedly.
In
@src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/settings/+page.svelte:
- Around line 20-45: The current const params includes name which causes stale
name overwrites; remove name from the base payload used by deleteTable and
updateTable. Replace usage of params in deleteTable and updateTable with a base
payload that only contains tableId and databaseId (e.g., derive a baseParams
from page.params), and in updateTable spread baseParams plus ...updates so the
name is only sent when provided in the updates object; keep the const params (or
rename) only if you still need a computed value for display but do not include
it in the API payload.
🟡 Minor comments (7)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/failedModal.svelte-6-16 (1)
6-16: Defaultshowto a boolean + ensure Close can’t submit.
show = $bindable()may start asundefined; prefer an explicit boolean default. Also consider forcing the Close button to be non-submit (depending onButtondefaults / Modal markup).Proposed change
let { - show = $bindable(), + show = $bindable(false), error, title, header }: { show: boolean; error: string; title: string; header: string; } = $props();- <Button secondary on:click={() => (show = false)}>Close</Button> + <Button secondary type="button" on:click={() => (show = false)}>Close</Button>Also applies to: 19-24
src/routes/(console)/project-[region]-[project]/databases/database-[database]/delete.svelte-150-150 (1)
150-150: Potential null reference:entities.totalaccessed whenentitiescould be null.The
entitiesstate is initialized asnulland only set afterlistEntities()completes. If rendering occurs before loading finishes (or if loading fails and setserror = true), this could cause a runtime error.🐛 Proposed fix
- {#if entityItems.length < entities.total} + {#if entities && entityItems.length < entities.total}src/routes/(console)/project-[region]-[project]/databases/database-[database]/breadcrumbs.svelte-10-17 (1)
10-17: Add optional chaining fororganization.nameto prevent runtime errors during route transitions.Based on learnings, shared layout components using
$derived(page.data.*)should use optional chaining since reactive statements can briefly evaluate with differentpage.datastructures during navigation. Line 15 correctly usesorganization?.$idwith fallback, but line 17 accessesorganization.namedirectly.🐛 Proposed fix
- const organization = $derived(page.data.organization) as Organization; + const organization = $derived(page.data.organization) as Organization | undefined; const breadcrumbs = $derived([ { href: resolveRoute('/(console)/organization-[organization]', { organization: organization?.$id ?? project.teamId }), - title: organization.name + title: organization?.name ?? '' },src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/settings/status.svelte-45-50 (1)
45-50: Semantic HTML issue:InputSwitchinside<ul>without<li>.The
<ul>element should only contain<li>children. Either wrap theInputSwitchin an<li>or remove the<ul>wrapper.Suggested fix
<svelte:fragment slot="aside"> - <ul> + <div> <InputSwitch id="toggle" label={enabled ? 'Enabled' : 'Disabled'} bind:value={enabled} /> - </ul> + </div>src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/settings/status.svelte-18-18 (1)
18-18: State initialization may not sync with prop changes.
enabledis initialized fromentity.enabledonce. If theentityprop changes (e.g., after invalidation), the localenabledstate won't automatically update. Consider using$derivedor an$effectto sync whenentitychanges.Suggested fix to sync state with prop
- let enabled: boolean = $state(entity.enabled); + let enabled: boolean = $state(entity.enabled); + + $effect(() => { + enabled = entity.enabled; + });Alternatively, if the component is expected to be destroyed and recreated on entity changes, this may not be necessary.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/settings/permissions.svelte-20-48 (1)
20-48: ResyncentityPermissionsifentityprop changes (avoid stale editor state).
Right now it’s initialized once; if the parent swapsentity, the editor can show previous permissions.Proposed fix
let entityPermissions: string[] = $state(entity.$permissions); + $effect(() => { + entityPermissions = entity.$permissions; + });src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/editRelated.svelte-46-55 (1)
46-55: Duplicate effect logic in onMount and $effect.The
onMount(Lines 46-55) and$effect(Lines 346-352) both callloadRelatedRow()whenrows && tableIdis truthy. This could cause duplicate API calls on initial mount.Suggested fix - remove onMount duplicate
onMount(() => { - if (rows && tableId) { - loadRelatedRow().then(() => { - focusFirstInput(); - }); - } - /* silences the not read error warning */ disabledState; });
🧹 Nitpick comments (27)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/relationship.svelte (1)
123-125: Consider adding null safety forcurrentTableaccess.The
tablesfilter safely handlestableListwith optional chaining, butcurrentTable.$idis accessed directly. Ifpage.data.tableis ever undefined during loading or navigation, this would throw.🛡️ Suggested defensive approach
$: currentTable = page.data.table; -$: tables = tableList?.tables?.filter((n) => n.$id !== currentTable.$id) ?? []; +$: tables = tableList?.tables?.filter((n) => n.$id !== currentTable?.$id) ?? [];src/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/table.svelte (1)
58-58:showDropdownstate appears unused.This array is only ever set to
false(lines 267, 278, 290) but is never read or set totrue. ThePopovercomponent manages its own visibility via thetogglefunction. Consider removing this dead code.Suggested removal
- let showDropdown = $state([]);Also remove the assignments:
- Line 267:
showDropdown[index] = false;- Line 278:
showDropdown[index] = false;- Line 290:
showDropdown[index] = false;src/lib/helpers/string.ts (1)
55-63: Minor edge case note for awareness.For single-character strings like
"y",beforeYwill be an empty string (falsy), causing the function to return"ys"instead of"ies". This is likely acceptable given the function's scope for known terminologies, but worth noting if single-character inputs become possible.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/layouts/store.ts (1)
1-5: Type the stores to avoidanyleaking into consumers.Proposed change
import { writable } from 'svelte/store'; -export const scrollStore = writable(null); +export const scrollStore = writable<HTMLElement | null>(null); export const sheetHeightStore = writable('74.5vh');src/lib/stores/navigation.ts (1)
23-28: Type casting approach is acceptable given SvelteKit's complex conditional types.The comment on line 24 is helpful. Consider documenting why
resolveArgsneeds the conditional structure (i.e.,resolveexpects different tuple shapes based on whether the route has params).♻️ Optional: Slightly cleaner alternative
export function resolveRoute<T extends RouteId>(route: T, params?: Record<string, string>) { - // type cast is necessary here! - const resolveArgs = params ? ([route, params] as [T, RouteParams<T>]) : [route]; - - return resolve(...(resolveArgs as ResolveArgs<T>)); + // Type cast necessary due to SvelteKit's conditional ResolveArgs type + return params + ? resolve(...([route, params] as ResolveArgs<T>)) + : resolve(...([route] as ResolveArgs<T>)); }src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/settings/danger.svelte (2)
42-48: Potential race condition: navigation inside Promise.all.Navigating away while
deleteTableDetailsis still running could cause issues if the preferences store relies on current page context. Consider executing navigation after the promise resolves:♻️ Suggested fix
- await Promise.all([ - preferences.deleteTableDetails($organization.$id, entity.$id), - navigate( - '/(console)/project-[region]-[project]/databases/database-[database]', - page.params - ) - ]); + await preferences.deleteTableDetails($organization.$id, entity.$id); + await navigate( + '/(console)/project-[region]-[project]/databases/database-[database]', + page.params + );
58-61: Add type guard for error handling.Accessing
e.messageassumeseis an Error object. If the promise rejects with a non-Error value, this will throw.♻️ Suggested fix
} catch (e) { - error = e.message; - trackError(e, analytics.submit.entity('Delete')); + const err = e instanceof Error ? e : new Error(String(e)); + error = err.message; + trackError(err, analytics.submit.entity('Delete')); }src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/indexes.svelte (1)
223-227: Consider adding explicit type annotation for columnMap entries.The
field['size']access uses bracket notation to bypass type checking. This works but could be more type-safe:♻️ Optional improvement
const columnMap: Map<string, number> = new Map( table.fields - .filter((field) => field.type === 'string' && 'size' in field) - .map((field) => [field.key, field['size']]) + .filter((field): field is { key: string; type: 'string'; size: number } & typeof field => + field.type === 'string' && 'size' in field) + .map((field) => [field.key, field.size]) );src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/indexes/+page.svelte (1)
77-79: Consider using.set()instead of.update()for simple boolean assignment.♻️ Suggested simplification
onClick={() => { - showIndexesSuggestions.update(() => true); + showIndexesSuggestions.set(true); }} />src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/store.ts (1)
33-47: Consider defensive check forfield.typebeing undefined.Since
FieldisPartial<Attributes> | Partial<Columns>, thetypeproperty could theoretically be undefined. The null check on line 42 handlesfieldbeing falsy, butfield.typecould still be undefined:♻️ Suggested defensive fix
export function isSpatialType( field: Columns | Attributes | Column ): field is | Models.ColumnPoint | Models.ColumnLine | Models.ColumnPolygon | Models.AttributePoint | Models.AttributeLine | Models.AttributePolygon { - if (!field) return false; + if (!field || !field.type) return false; const spatialTypes = ['point', 'linestring', 'polygon']; return spatialTypes.includes(field.type.toLowerCase()); }package.json (1)
25-25: Dependency update is appropriate for DocumentsDB SDK integration.The
@appwrite.io/consolepackage is already integrated into the SDK stores (src/lib/stores/sdk.ts), whereDocumentsDBis imported and instantiated. Consider documenting which SDK features this commit hash enables in the PR description for future reference.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/dependencies.ts (1)
11-14: Potential undefined access when term title doesn't match Dependencies keys.If
term.title.singular.toUpperCase()doesn't correspond to a key inDependencies, the lookup returnsundefinedsilently due to the type assertion. This could lead to unexpectedundefinedvalues being passed downstream.Consider adding validation or a fallback:
♻️ Suggested defensive lookup
const getDependencies = (term: { title: Term }) => ({ - singular: Dependencies[term.title.singular.toUpperCase() as keyof typeof Dependencies], - plural: Dependencies[term.title.plural.toUpperCase() as keyof typeof Dependencies] + singular: Dependencies[term.title.singular.toUpperCase() as keyof typeof Dependencies] ?? null, + plural: Dependencies[term.title.plural.toUpperCase() as keyof typeof Dependencies] ?? null });src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/usage/view.svelte (1)
19-20: Consider adding defensive checks for terminology properties.The code assumes
terminology.recordandterminology.entityare always defined. IfgetTerminologies()returns partial data (e.g., for unsupported database types like vectordb), this could throw at runtime.♻️ Optional defensive pattern
- const records = terminology.record.lower.plural; - const entity = terminology.entity.lower.singular; + const records = terminology.record?.lower?.plural ?? 'records'; + const entity = terminology.entity?.lower?.singular ?? 'entity';src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/view.svelte (1)
118-127: Typo in comment and potentially incomplete realtime event pattern.Line 120 has a grammatical error: "this is needed because
documentsdbdoesn't usedatabaseprefix don't exist" is unclear. Consider rewording.Also, the realtime event pattern uses
${terminology.type}.*.${terminology.entity.lower.plural}.*.indexes.*but the comment on line 118-119 shows patterns likedatabases.*.tables.*.indexes.*. Verify that the terminology-derived pattern correctly matches all expected event formats.Suggested comment fix
// example: databases.*.tables.*.indexes.* // example: documentsdb.*.collections.indexes.* - // this is needed because `documentsdb` doesn't use `database` prefix don't exist + // this is needed because `documentsdb` doesn't use the `database` prefix const derivedEventsForIndex = `${terminology.type}.*.${terminology.entity.lower.plural}.*.indexes.*`;src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/header.svelte (1)
15-15: Type assertion without runtime check.The assertion
as Entityonpage.data.tableassumes the data loader always provides anEntity-compatible object. Based on learnings, shared layout components should use optional chaining during page transitions. Consider adding a guard or using optional chaining.Suggested safer pattern
- const table = $derived(page.data.table) as Entity; + const table = $derived(page.data.table as Entity | undefined);Then the conditional on line 57 already handles the undefined case.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/settings/permissions.svelte (1)
27-34: Drop the “events and notif!” / “invalidate proper dependency.” comments.
They don’t add signal and clash with the project’s “minimal comments” guideline. As per coding guidelines, keep comments for TODOs or complex logic only.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/edit.svelte (1)
115-137: HardencompareColumns()for undefined array/relationship values.
Array.from(workColumn)/workColumn.map(...)will throw if the value is missing/null; safer to coerce to[](or early-return) for array/relationship cases.src/routes/(console)/project-[region]-[project]/databases/database-[database]/store.ts (1)
7-7: Migration toshowCreateEntityis complete—no staleshowCreateTablereferences found.The renaming has been successfully applied across all files. However, consider encoding
entityIdinbuildEntityRouteas a defensive measure, since ifentityIdever contains special characters (/, ?, &, etc.), it could break route generation:Optional defensive enhancement
export function buildEntityRoute(page: Page, entityType: string, entityId: string): string { return withPath( resolveRoute( '/(console)/project-[region]-[project]/databases/database-[database]', page.params ), - `/${entityType}-${entityId}` + `/${entityType}-${encodeURIComponent(entityId)}` ); }src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.svelte (1)
60-73: Remove unnecessary type cast onfield.type.The
as ColumnTypecast on line 65 is defensive but appears unnecessary.field.typealready originates from Models types that should align withColumnTypevalues, and the field is used correctly as a string throughout the codebase without type issues. If the cast is needed due to SDK type limitations, consider extracting a helper or documenting why it's required rather than leaving it as an implicit workaround.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/sdk.ts (1)
32-40: Potential runtime error whendatabaseTypeis not provided in string mode.When
regionOrPageandprojectOrTerminologyare strings,databaseTypeis assigned via non-null assertion (databaseType!). However, the function signature marks it as optional. If a caller passes strings but omitsdatabaseType, this will silently assignundefinedtotype, causing the switch statements to fall through to thedefaultcase and throw "Unknown database type".Consider adding a runtime check or making the overload signatures more explicit:
Suggested validation
} else { - type = databaseType!; + if (!databaseType) { + throw new Error('databaseType is required when using string parameters'); + } + type = databaseType; region = regionOrPage as string; project = projectOrTerminology as string; }src/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.svelte (1)
18-32: Side effect during component initialization may cause issues.The
tableViewColumns.update()call executes immediately when the component script runs. This could cause issues with:
- SSR - the store mutation happens on server
- Multiple component instances - each would mutate the shared store
Consider moving this to
onMountor using a derived store:Move to onMount
+ import { onMount } from 'svelte'; + const { terminology } = getTerminologies(); const entityTitle = terminology.entity.title; const entityLower = terminology.entity.lower; - /** - * init update because `getContext` - * doesn't work on typescript context! - */ - tableViewColumns.update((columns) => { - /* $id */ - columns[0].title = `${entityTitle.singular} ID`; - return columns; - }); + onMount(() => { + tableViewColumns.update((columns) => { + columns[0].title = `${entityTitle.singular} ID`; + return columns; + }); + });src/routes/(console)/project-[region]-[project]/databases/database-[database]/grid.svelte (1)
7-24: Remove unnecessaryonMountworkaround.The
onMountblock referencingshowCreateto silence a TypeScript warning appears unnecessary. The variableshowCreateis actually used in line 30 (on:click={() => (showCreate = true)}), so the warning shouldn't occur. This might be a leftover from development.Remove the workaround
import type { TerminologyResult } from '$database/(entity)'; import { buildEntityRoute } from '$database/store'; -import { onMount } from 'svelte'; let { data, showCreate = $bindable(false), terminology }: { data: PageData; showCreate: boolean; terminology: TerminologyResult; } = $props(); - -onMount(() => { - /* silences `declared but its value is never read` warning. */ - showCreate; -});src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte (1)
80-94: Silent failure in entity loading.The
loadEntitiesfunction catches errors but doesn't display any notification or log the error, making debugging difficult when entity loading fails.Suggested improvement
async function loadEntities() { try { entities = await databasesSdk.listEntities({ databaseId: page.params.database, queries: [Query.orderDesc(''), Query.limit(100)] }); + } catch (error) { + console.error('Failed to load entities:', error); } finally { loading = false; } }src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts (1)
10-25: Consider using more specific types for Entity and Field.The
EntityandFieldtypes usePartial<>wrappers, which makes all properties optional. This could lead to runtime errors when accessing properties that are assumed to exist (e.g.,entity.$id,entity.name).Consider defining required properties explicitly:
Suggested approach
-export type Entity = Partial<Models.Collection | Table> & { +export type Entity = Pick<Models.Collection | Table, '$id' | 'name' | '$createdAt' | '$updatedAt'> & Partial<Omit<Models.Collection | Table, '$id' | 'name' | '$createdAt' | '$updatedAt'>> & { indexes?: Index[]; fields?: (Attributes | Columns)[]; recordSecurity?: Models.Collection['documentSecurity'] | Models.Table['rowSecurity']; };src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte (1)
99-106: Mixed Svelte 4/5 syntax patterns.This file uses Svelte 4 patterns (
export let,$:reactive statements) alongside Svelte 5 patterns ($state). While this works during migration, consider fully migrating to Svelte 5 syntax for consistency:
export let data→ use$props()$: table = data.table→ use$derived()src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/create.svelte (1)
125-129: Complex disabled condition could benefit from clarification.The condition
!fieldList.at(-1)?.order && fieldList.at(-1)?.order !== nullchecks if order is falsy but not explicitlynull. This logic is correct for allowingnullorders for spatial indexes while requiring ASC/DESC for others, but a comment would improve readability.src/lib/helpers/faker.ts (1)
1-6: Use$libalias for helper imports (consistency).
import type { NestedNumberArray } from './types';is insrc/lib/**, so prefer$lib/helpers/typesto match repo conventions.
.../(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/init.ts
Show resolved
Hide resolved
...(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.ts
Outdated
Show resolved
Hide resolved
...nsole)/project-[region]-[project]/databases/database-[database]/(entity)/views/header.svelte
Show resolved
Hide resolved
| const nonSheetPages = $derived.by(() => { | ||
| const field = terminology.field.lower.plural; | ||
| const entityType = terminology.entity.lower.singular; | ||
| const resourceSeg = `${entityType}-[${entityType}]`; | ||
| const endings = [resourceSeg, `${resourceSeg}/${field}`, `${resourceSeg}/indexes`]; | ||
| const isSpreadsheetPage = endings.some((end) => page.route.id?.endsWith(end)); | ||
| return !isSpreadsheetPage; | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential crash: terminology.field is optional—don’t access .lower directly.
This should be guarded (and transition-safe). Based on learnings about optional chaining during route transitions.
Proposed fix
const nonSheetPages = $derived.by(() => {
- const field = terminology.field.lower.plural;
+ const field = terminology.field?.lower.plural;
const entityType = terminology.entity.lower.singular;
const resourceSeg = `${entityType}-[${entityType}]`;
- const endings = [resourceSeg, `${resourceSeg}/${field}`, `${resourceSeg}/indexes`];
+ const endings = [
+ resourceSeg,
+ ...(field ? [`${resourceSeg}/${field}`] : []),
+ `${resourceSeg}/indexes`
+ ];
const isSpreadsheetPage = endings.some((end) => page.route.id?.endsWith(end));
return !isSpreadsheetPage;
});📝 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 nonSheetPages = $derived.by(() => { | |
| const field = terminology.field.lower.plural; | |
| const entityType = terminology.entity.lower.singular; | |
| const resourceSeg = `${entityType}-[${entityType}]`; | |
| const endings = [resourceSeg, `${resourceSeg}/${field}`, `${resourceSeg}/indexes`]; | |
| const isSpreadsheetPage = endings.some((end) => page.route.id?.endsWith(end)); | |
| return !isSpreadsheetPage; | |
| }); | |
| const nonSheetPages = $derived.by(() => { | |
| const field = terminology.field?.lower.plural; | |
| const entityType = terminology.entity.lower.singular; | |
| const resourceSeg = `${entityType}-[${entityType}]`; | |
| const endings = [ | |
| resourceSeg, | |
| ...(field ? [`${resourceSeg}/${field}`] : []), | |
| `${resourceSeg}/indexes` | |
| ]; | |
| const isSpreadsheetPage = endings.some((end) => page.route.id?.endsWith(end)); | |
| return !isSpreadsheetPage; | |
| }); |
🤖 Prompt for AI Agents
In
@src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/header.svelte
around lines 54 - 63, The derived store nonSheetPages accesses
terminology.field.lower.plural and terminology.entity.lower.singular without
guarding for missing terminology.field (or .lower), which can crash during route
transitions; update the callback in nonSheetPages to use optional chaining or
safe defaults (e.g., terminology.field?.lower?.plural and
terminology.entity?.lower?.singular or fallbacks like empty strings) before
building resourceSeg and endings so page.route.id?.endsWith checks never operate
on undefined values and the component remains transition-safe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it should never be null. if it is then something wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ItzNotABug, understood—I trust your knowledge of the data model. My concern wasn't about steady-state data integrity, but rather about transition timing.
There's a related pattern we've seen before: shared layout/header components using $derived(page.data.*) can briefly evaluate with incomplete data during page transitions, before the new route's data is fully loaded. Since this header uses useTerminology(page) and the derived nonSheetPages runs on every reactive update, there's a window where terminology.field might not yet be populated—even if it's always present once navigation completes.
That said, if you're confident useTerminology guarantees all fields synchronously (or this header only renders after terminology is ready), then the guard isn't needed. Just wanted to flag the transition-safety pattern we've applied elsewhere. Let me know if you'd like me to move on! 👍
🧠 Learnings used
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/header.svelte:54-58
Timestamp: 2025-10-13T05:16:07.656Z
Learning: In SvelteKit apps, shared layout components (like headers) that use `$derived(page.data.*)` should use optional chaining when accessing properties that may not be present on all routes. During page transitions, reactive statements can briefly evaluate with different page.data structures, so optional chaining prevents runtime errors when navigating between routes with different data shapes (e.g., between `/databases` and `/databases/database-[database]`).
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Applies to src/routes/**/*.svelte : Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts:28-28
Timestamp: 2025-10-07T14:17:11.597Z
Learning: In src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts, the empty `vectordb: {}` entry in baseTerminology is intentional. The vectordb database type is not currently used because the backend API is not finalized, and no database type returns 'vectordb' at the moment.
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/analytics.ts:17-18
Timestamp: 2025-10-07T14:16:31.893Z
Learning: In the console codebase analytics helpers at `src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/analytics.ts`, the dynamically constructed enum lookups (like `Submit[enumKey as keyof typeof Submit]`) are designed to return `undefined` safely when a terminology entry doesn't exist, because the corresponding views won't be rendered in those scenarios (e.g., DocumentsDB columns/attributes have no view), so the analytics code paths won't be reached.
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/view.svelte:84-84
Timestamp: 2025-10-09T12:22:41.099Z
Learning: In the Appwrite console databases UI, user-facing labels like "Columns" should remain unchanged even when internal terminology changes to "fields". The multi-terminology support refactors internal data models without changing the user-facing text.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/+layout.svelte
Show resolved
Hide resolved
...utes/(console)/project-[region]-[project]/databases/database-[database]/backups/+page.svelte
Show resolved
Hide resolved
src/routes/(console)/project-[region]-[project]/databases/database-[database]/header.svelte
Show resolved
Hide resolved
...tes/(console)/project-[region]-[project]/databases/database-[database]/settings/+page.svelte
Show resolved
Hide resolved
...project-[region]-[project]/databases/database-[database]/table-[table]/settings/+page.svelte
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/delete.svelte (1)
74-79: Fix grammar for plural case.The message says "queries that depend on it" but refers to multiple indexes. Should be "depend on them".
Proposed fix
{:else} <p>Are you sure you want to delete <b>{selectedKeys.join(', ')}</b>?</p> <p> - Deleting these indexes may slow down queries that depend on it. This action is + Deleting these indexes may slow down queries that depend on them. This action is irreversible. </p> {/if}
🤖 Fix all issues with AI agents
In @src/routes/(console)/project-[region]-[project]/databases/+page.ts:
- Around line 63-71: Replace the empty string in the Query.orderDesc call so the
map that iterates over databases.databases uses Query.orderDesc('$createdAt')
with Query.limit(1); update the databases.databases.map async callback where
databaseSdk.listEntities is called (the Query.orderDesc('') argument) to
Query.orderDesc('$createdAt') so entities[$id] captures the most recent entity
ID consistently with the rest of the file.
In
@src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/delete.svelte:
- Line 18: The variable 'error' is annotated as string but initialized to null,
causing a TypeScript mismatch; update the declaration (the line using $state) to
allow null by changing the type to 'string | null' (e.g., use let error: string
| null = $state(null);) or remove the explicit annotation and let the
$state(null) inference set the correct nullable type; ensure references to
'error' handle the nullable type accordingly.
In
@src/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.ts:
- Line 23: The Query.orderDesc('') call is invalid because it uses an empty
attribute; update the queries array in the page loader to either remove
Query.orderDesc('') or replace it with a valid attribute name such as
Query.orderDesc('$createdAt') or Query.orderDesc('$updatedAt'); locate the
queries definition that contains Query.limit(limit), Query.offset(offset),
Query.orderDesc('') and change that entry accordingly so the Appwrite query uses
a real attribute or omit the order query when no sorting is required.
In
@src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte:
- Around line 82-85: The call to Query.orderDesc('') is invalid because it uses
an empty field name; update the listEntities call so Query.orderDesc specifies a
real field (for example '$createdAt' or whichever sortable field you need).
Locate the entities assignment where databaseSdk.listEntities is invoked (using
page.params.database) and replace Query.orderDesc('') with
Query.orderDesc('<fieldName>') and keep Query.limit(100) as-is so results are
properly ordered and limited.
- Around line 41-48: The variables entityTypePlural and entityTypeSingular are
not wrapped in $derived, breaking reactivity; update their assignments to derive
from terminology (similar to table.svelte) so they use
$derived(terminology.entity.lower.plural) and
$derived(terminology.entity.lower.singular) respectively, keeping
useTerminology, useDatabaseSdk and the existing entityId derived expression
unchanged so the reactive chain remains intact.
🧹 Nitpick comments (8)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/delete.svelte (1)
55-57: Consider safer error message extraction.If
eis not anErrorinstance (e.g., a string or unknown type is thrown),e.messagemay beundefined.Proposed fix
} catch (e) { - error = e.message; + error = e instanceof Error ? e.message : String(e); trackError(e, Submit.IndexDelete); }src/lib/helpers/faker.ts (2)
8-82: Switch logic aligns with backend API design.The shared handling of
legacyandtablesdbcorrectly reflects that they use the same tablesDB API. The empty return fordocumentsdbandvectordbis intentional per the backend requirements.Consider adding a
defaultcase or exhaustive check to future-proof against newDatabaseTypevalues:case 'vectordb': /* vector embeddings + metadata defined at collection creation */ { /* no individual field creation needed */ return []; } + default: { + const _exhaustive: never = databaseType; + throw new Error(`Unhandled database type: ${_exhaustive}`); + } }Based on learnings, this aligns with the established patterns.
93-129: Variable naming inconsistent with field terminology.The function parameter is
fields, but internal variables still use column terminology (filteredColumns,column). This creates confusion in a field-centric refactor.♻️ Suggested naming alignment
- const filteredColumns = fields.filter( + const filteredFields = fields.filter( (col) => col.type !== 'relationship' && col.status === 'available' );- for (const column of filteredColumns) { - row[column.key] = generateValueForField(column); + for (const field of filteredFields) { + row[field.key] = generateValueForField(field); }src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte (2)
36-37: Inconsistent import path extensions.Line 36 imports without
.jsextension while line 37 includes it. Keep imports consistent.import { resolveRoute } from '$lib/stores/navigation'; -import { withPath } from '$lib/stores/navigation.js'; +import { withPath } from '$lib/stores/navigation';
249-253: Consider consistent href construction approach.The bottom sheet uses
buildEntityRoute(page, entityTypeSingular, entity.$id)while the sidebar list (line 136-139) useswithPath(databaseBaseRoute, ...). Consider using the same approach for consistency and maintainability.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/overview.svelte (1)
44-52: Inconsistent optional chaining pattern and potential issue withidattributes.
Line 51 accesses
selectedIndex.lengths[i]without optional chaining onselectedIndex, unlike lines 44-45 which useselectedIndex?.orders?.[i]. This inconsistency could cause issues if patterns diverge.The
idattributes on lines 44 and 50 use dynamic values (selectedIndex?.orders?.[i]andselectedIndex?.lengths?.[i]) which could result in duplicate or empty IDs if values are missing or identical across rows.Suggested consistency fix
<InputText required label={i === 0 ? 'Order' : ''} - id={`value-${selectedIndex?.orders?.[i] ?? ''}`} + id={`order-${i}`} value={selectedIndex?.orders?.[i] ?? ''} readonly /> <InputText required label={i === 0 ? 'Length' : ''} - id={`value-${selectedIndex?.lengths?.[i] ?? ''}`} - value={selectedIndex.lengths[i]?.toString() ?? null} + id={`length-${i}`} + value={selectedIndex?.lengths?.[i]?.toString() ?? null} readonly />src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/sdk.ts (2)
32-40: Non-null assertion and empty string fallbacks may mask configuration errors.When string parameters are passed,
databaseType!assumes the caller always provides it, but the function signature marks it as optional. Additionally, empty string fallbacks forregionandproject(lines 34-35) could lead to silent failures in API calls rather than surfacing configuration issues early.Consider adding validation
} else { - type = databaseType!; + if (!databaseType) { + throw new Error('databaseType is required when using string parameters'); + } + type = databaseType; region = regionOrPage as string; project = projectOrTerminology as string; } + + if (!region || !project) { + throw new Error('region and project are required'); + }
91-96: Misleading variable name.The variable is named
tablebut it retrieves a collection fordocumentsdb. Consider renaming toentityorcollectionfor clarity.Suggested fix
case 'documentsdb': { - const table = await baseSdk.documentsDB.getCollection({ + const collection = await baseSdk.documentsDB.getCollection({ databaseId: params.databaseId, collectionId: params.entityId }); - return toSupportiveEntity(table); + return toSupportiveEntity(collection); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/lib/actions/analytics.tssrc/lib/helpers/faker.tssrc/routes/(console)/project-[region]-[project]/databases/+page.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/init.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/sdk.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/header.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/delete.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/overview.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/+page.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/header.svelte
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/init.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx,js,jsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,svelte}: Import reusable modules from the src/lib directory using the $lib alias
Use minimal comments in code; reserve comments for TODOs or complex logic explanations
Use $lib, $routes, and $themes aliases instead of relative paths for module imports
Files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/delete.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.tssrc/lib/helpers/faker.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/sdk.tssrc/lib/actions/analytics.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/overview.sveltesrc/routes/(console)/project-[region]-[project]/databases/+page.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/+page.svelte
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Define types inline or in .d.ts files, avoid creating separate .types.ts files
Use TypeScript in non-strict mode; any type is tolerated in this project
Files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.tssrc/lib/helpers/faker.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/sdk.tssrc/lib/actions/analytics.tssrc/routes/(console)/project-[region]-[project]/databases/+page.ts
**/*.{ts,tsx,js,jsx,svelte,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4 spaces for indentation, single quotes, 100 character line width, and no trailing commas per Prettier configuration
Files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/delete.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.tssrc/lib/helpers/faker.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/sdk.tssrc/lib/actions/analytics.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/overview.sveltesrc/routes/(console)/project-[region]-[project]/databases/+page.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/+page.svelte
src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Configure dynamic routes using SvelteKit convention with [param] syntax in route directory names
Files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/delete.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/sdk.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/overview.sveltesrc/routes/(console)/project-[region]-[project]/databases/+page.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/+page.svelte
src/routes/**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/delete.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/overview.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/+page.svelte
**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development
Files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/delete.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/overview.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/+page.svelte
src/lib/helpers/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Implement pure functions in src/lib/helpers/ directory and use camelCase for helper function names
Files:
src/lib/helpers/faker.ts
🧠 Learnings (17)
📓 Common learnings
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/view.svelte:84-84
Timestamp: 2025-10-09T12:22:41.099Z
Learning: In the Appwrite console databases UI, user-facing labels like "Columns" should remain unchanged even when internal terminology changes to "fields". The multi-terminology support refactors internal data models without changing the user-facing text.
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.ts:17-21
Timestamp: 2025-10-10T12:47:52.651Z
Learning: In the database type handling for the console project, the 'legacy' database type maps to the same API as 'tablesdb' (not 'documentsdb'). Legacy is what tablesDB is based upon - they share the same API with just naming differences. The console does not use legacy SDKs in practice.
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts:28-28
Timestamp: 2025-10-07T14:17:11.597Z
Learning: In src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts, the empty `vectordb: {}` entry in baseTerminology is intentional. The vectordb database type is not currently used because the backend API is not finalized, and no database type returns 'vectordb' at the moment.
📚 Learning: 2025-10-07T14:17:11.597Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts:28-28
Timestamp: 2025-10-07T14:17:11.597Z
Learning: In src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts, the empty `vectordb: {}` entry in baseTerminology is intentional. The vectordb database type is not currently used because the backend API is not finalized, and no database type returns 'vectordb' at the moment.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/delete.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.tssrc/lib/helpers/faker.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/sdk.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/overview.sveltesrc/routes/(console)/project-[region]-[project]/databases/+page.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/+page.svelte
📚 Learning: 2025-11-25T03:15:27.539Z
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Applies to src/routes/**/*.svelte : Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/+page.svelte
📚 Learning: 2025-10-10T12:47:52.651Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.ts:17-21
Timestamp: 2025-10-10T12:47:52.651Z
Learning: In the database type handling for the console project, the 'legacy' database type maps to the same API as 'tablesdb' (not 'documentsdb'). Legacy is what tablesDB is based upon - they share the same API with just naming differences. The console does not use legacy SDKs in practice.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/sdk.tssrc/routes/(console)/project-[region]-[project]/databases/+page.ts
📚 Learning: 2025-09-25T04:21:57.071Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2373
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte:536-552
Timestamp: 2025-09-25T04:21:57.071Z
Learning: In the Appwrite console database suggestions flow, after successfully creating columns via `createColumns()`, the `await invalidate(Dependencies.TABLE)` call causes the view to be destroyed and another view (populated table view) to be rendered, automatically cleaning up component state without needing manual reset.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/delete.svelte
📚 Learning: 2025-10-26T10:20:29.792Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2509
File: src/routes/(console)/project-[region]-[project]/auth/teams/+page.svelte:47-61
Timestamp: 2025-10-26T10:20:29.792Z
Learning: When deleting teams in the Appwrite Console codebase, only `Dependencies.TEAMS` needs to be invalidated. Additional invalidations for `Dependencies.TEAM` (detail route) and `Dependencies.MEMBERSHIPS` are not necessary because the deleted teams and their memberships no longer exist.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/delete.svelte
📚 Learning: 2025-10-07T14:16:31.893Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/analytics.ts:17-18
Timestamp: 2025-10-07T14:16:31.893Z
Learning: In the console codebase analytics helpers at `src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/analytics.ts`, the dynamically constructed enum lookups (like `Submit[enumKey as keyof typeof Submit]`) are designed to return `undefined` safely when a terminology entry doesn't exist, because the corresponding views won't be rendered in those scenarios (e.g., DocumentsDB columns/attributes have no view), so the analytics code paths won't be reached.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/delete.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.tssrc/lib/actions/analytics.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/overview.svelte
📚 Learning: 2025-10-09T12:22:41.099Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/view.svelte:84-84
Timestamp: 2025-10-09T12:22:41.099Z
Learning: In the Appwrite console databases UI, user-facing labels like "Columns" should remain unchanged even when internal terminology changes to "fields". The multi-terminology support refactors internal data models without changing the user-facing text.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.sveltesrc/lib/helpers/faker.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/overview.svelte
📚 Learning: 2025-09-30T07:41:06.679Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2425
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte:454-468
Timestamp: 2025-09-30T07:41:06.679Z
Learning: In `src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte`, the column suggestions API (console.suggestColumns) has a maximum limit of 7 columns returned, which aligns with the initial placeholder count of 7 in customColumns.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.sveltesrc/lib/helpers/faker.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/overview.svelte
📚 Learning: 2025-10-13T05:13:54.542Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/table.svelte:33-39
Timestamp: 2025-10-13T05:13:54.542Z
Learning: In Svelte 5, `import { page } from '$app/state'` provides a reactive state proxy that can be accessed directly (e.g., `page.params`), unlike the older `import { page } from '$app/stores'` which returns a readable store requiring the `$page` syntax for auto-subscription in components.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/+page.svelte
📚 Learning: 2025-10-05T09:41:40.439Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2398
File: src/routes/(console)/verify-email/+page.svelte:48-51
Timestamp: 2025-10-05T09:41:40.439Z
Learning: In SvelteKit 5, `page` imported from `$app/state` is a reactive state object (using runes), not a store. It should be accessed as `page.data` without the `$` prefix, unlike the store-based `$page` from `$app/stores` in earlier versions.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/+page.svelte
📚 Learning: 2025-10-13T05:16:07.656Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/header.svelte:54-58
Timestamp: 2025-10-13T05:16:07.656Z
Learning: In SvelteKit apps, shared layout components (like headers) that use `$derived(page.data.*)` should use optional chaining when accessing properties that may not be present on all routes. During page transitions, reactive statements can briefly evaluate with different page.data structures, so optional chaining prevents runtime errors when navigating between routes with different data shapes (e.g., between `/databases` and `/databases/database-[database]`).
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/overview.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/+page.svelte
📚 Learning: 2025-09-25T04:33:19.632Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2373
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/createTable.svelte:28-33
Timestamp: 2025-09-25T04:33:19.632Z
Learning: In the Appwrite console createTable component, manual submit state management (like setting creatingTable = true) is not needed because: 1) The Modal component handles submit state internally via submissionLoader, preventing double submissions, and 2) After successful table creation, the entire view is destroyed and replaced with the populated table view, ending the component lifecycle.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte
📚 Learning: 2025-11-25T03:15:27.539Z
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Applies to **/*.ts : Define types inline or in .d.ts files, avoid creating separate .types.ts files
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.ts
📚 Learning: 2025-10-13T05:13:55.279Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte:378-378
Timestamp: 2025-10-13T05:13:55.279Z
Learning: In the codebase, tables always have a `fields` property. It can be an empty array but is never null or undefined.
Applied to files:
src/lib/helpers/faker.ts
📚 Learning: 2025-10-04T11:46:32.504Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2442
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/float.svelte:88-88
Timestamp: 2025-10-04T11:46:32.504Z
Learning: In the Appwrite Console codebase, for float/double column inputs in database table column configuration files (like float.svelte), use step={0.1} for InputNumber components, not step="any". This is the established pattern for float/double precision inputs.
Applied to files:
src/lib/helpers/faker.ts
📚 Learning: 2025-09-29T06:30:25.867Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2407
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/indexes.svelte:95-104
Timestamp: 2025-09-29T06:30:25.867Z
Learning: AppwriteException from appwrite.io/console has guaranteed properties including message, code, and type - error.message can be used directly without null checks.
Applied to files:
src/lib/actions/analytics.ts
🧬 Code graph analysis (5)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.ts (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/+layout.ts (1)
load(8-21)src/lib/stores/database.ts (1)
database(11-11)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.ts (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts (2)
DatabaseType(8-8)baseTerminology(27-49)
src/lib/helpers/faker.ts (2)
src/lib/stores/sdk.ts (1)
sdk(175-198)src/lib/helpers/types.ts (1)
NestedNumberArray(84-84)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/sdk.ts (4)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts (4)
DatabaseType(8-8)Entity(10-14)EntityList(22-25)toSupportiveEntity(86-99)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.ts (1)
TerminologyResult(13-23)src/routes/(console)/project-[region]-[project]/store.ts (1)
project(11-11)src/lib/stores/sdk.ts (1)
sdk(175-198)
src/routes/(console)/project-[region]-[project]/databases/+page.ts (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/sdk.ts (1)
useDatabaseSdk(23-119)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: e2e
🔇 Additional comments (26)
src/lib/actions/analytics.ts (4)
54-57: LGTM!Good defensive guard to prevent tracking events with empty names. This ensures analytics data quality.
78-85: LGTM!The added
eventcheck ensurestrackErrorwon't attempt to track errors without a valid form context. This aligns with the function's purpose of associating errors with specific submission events.
285-285: LGTM!New
DatabaseBackupPolicyCreateenum value properly follows the existing naming convention and is consumed by the backups page for policy creation analytics.
300-307: LGTM!Table-related Submit enum members are correctly added to support the multi-terminology analytics system. Based on retrieved learnings, these enum values work with dynamically constructed lookups that safely return
undefinedwhen terminology entries don't exist for certain database types.src/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/+page.svelte (4)
28-33: LGTM!Proper Svelte 5 migration using
$props(),$state(), and$derived(). The derivedisDisabledcomputation correctly combines self-hosted and cloud plan checks.
116-116: LGTM!Good use of the typed
Submit.DatabaseBackupPolicyCreateenum instead of a magic string for analytics tracking.
152-158: Improved UX: form state preserved on error.The error handling correctly:
- Sets
policyCreateErrorwhich binds to the modal'serrorprop- Tracks the error with
trackError- Intentionally does not clear
totalPoliciesor close the modal, allowing users to see the error and retry without losing their inputThis is a good UX improvement over clearing the form on failure.
236-246: LGTM!The Modal correctly binds
policyCreateErrorto display server-side errors, and the Create button's disabled state properly depends ontotalPolicies.length.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/delete.svelte (3)
23-26: LGTM!The
getKeysfunction correctly handles all input variants with proper null safety.
28-49: LGTM!The cleanup function correctly captures keys before resetting state, handles both single and bulk deletion notifications, and uses the terminology-driven invalidation pattern aligned with the PR's multi-terminology support goals.
6-6: The$databasealias is properly configured insvelte.config.jsand the import is valid. No action needed.src/lib/helpers/faker.ts (4)
1-6: Imports look correct for the refactor.The
$database/(entity)alias usage aligns with the project's alias configuration. The relative import for./typesis acceptable within the samehelpersdirectory.
160-179: Correctly handles array field generation.The function properly creates non-array copies of the field for generating individual array items, then delegates to
generateSingleValue.
198-204: Safe enum handling with proper fallback.The optional chaining on
enumAttr.elements?.lengthand thenullfallback for empty enums is correct given thePartialnature of theFieldtype.
214-234: No action needed —isWithinSafeRangecorrectly handles undefined values.When
intAttr.minorintAttr.maxisundefined(due toFieldbeingPartial),Math.abs(undefined)returnsNaN, andNaN < Number.MAX_SAFE_INTEGERevaluates tofalse. This causes the fallback logic to trigger, using the default values (0or the computedfallbackMax). The code functions as intended.src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte (3)
91-94: LGTM!The subscription pattern correctly returns the unsubscribe function for cleanup on component unmount.
130-161: LGTM!Entity iteration and href construction are consistent. The
data-privateattribute appropriately masks entity names.
292-325: LGTM!CSS class renames from
table-*toentity-*align well with the entity-driven refactor.Also applies to: 382-388
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/overview.svelte (1)
4-13: LGTM - Clean terminology integration.The refactoring to use
getTerminologies()and theIndextype aligns well with the multi-terminology support. The dynamicfieldLabelderivation maintains flexibility for different database types.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.ts (1)
1-67: Well-structured type definitions for terminology system.The type utilities for deriving analytics action types from the base terminology are sophisticated and provide good type safety. The pattern aligns with the learning that undefined analytics lookups are handled safely because corresponding views won't render.
One observation: Per coding guidelines, types should typically be defined inline or in
.d.tsfiles rather than separate.types.tsfiles. However, given the complexity and shared nature of these types across multiple modules (terminology, analytics, dependencies), a centralized types module is pragmatic here.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/sdk.ts (1)
62-78: Repetitive switch-case pattern across methods.The switch statement for database type dispatching is duplicated across
listEntities,getEntity, anddelete. This could be centralized, but given the different return types and parameters for each method, the current approach is acceptable for clarity.src/routes/(console)/project-[region]-[project]/databases/+page.ts (1)
10-10: LGTM - Proper use of path alias.The import uses the
$database/(entity)alias as per coding guidelines instead of relative paths.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.ts (1)
6-21: LGTM - Clean SDK abstraction integration.The refactoring properly:
- Retrieves database context from parent layout via
await parent()- Initializes the SDK with the correct database type
- Uses the new
getEntitymethod for fetchingThis aligns well with the broader entity-driven model while maintaining backward compatibility by returning
tablein the data payload.src/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.ts (3)
5-5: LGTM on the new alias usage.The
$database/(entity)alias import is consistent with the PR's introduction of the new database entity SDK layer.
7-8: LGTM on parent context usage.Using
parent()to access the database from the parent layout is the correct SvelteKit pattern for sharing data across route hierarchy.
26-32: LGTM on the return shape update.The change from
tablestoentitiesaligns with the PR's entity-driven data model refactor.
| databases.databases.map(async ({ $id, type }) => { | ||
| const res = await databaseSdk.listEntities({ | ||
| databaseId: $id, | ||
| databaseType: type, | ||
| queries: [Query.limit(1), Query.orderDesc('')] | ||
| }); | ||
|
|
||
| tables[$id] = res.tables?.[0]?.$id ?? null; | ||
| entities[$id] = res.entities?.[0]?.$id ?? null; | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
cat -n "src/routes/(console)/project-[region]-[project]/databases/+page.ts" | sed -n '50,85p'Repository: appwrite/console
Length of output: 1500
🏁 Script executed:
head -30 "src/routes/(console)/project-[region]-[project]/databases/+page.ts"Repository: appwrite/console
Length of output: 1192
🏁 Script executed:
rg "Query\.orderDesc" --max-count 20 -B 2 -A 2Repository: appwrite/console
Length of output: 28791
🏁 Script executed:
rg "orderDesc" --type ts --type tsx -A 3 -B 3 node_modules/@appwrite.io 2>/dev/null | head -50Repository: appwrite/console
Length of output: 42
🏁 Script executed:
fd "Query" node_modules/@appwrite.io -type f | head -10Repository: appwrite/console
Length of output: 230
🏁 Script executed:
cat -n "src/routes/(console)/project-[region]-[project]/databases/+page.ts" | sed -n '60,100p'Repository: appwrite/console
Length of output: 1732
🏁 Script executed:
rg "listEntities" --max-count 10 -B 5 -A 5 | head -100Repository: appwrite/console
Length of output: 11774
🏁 Script executed:
grep -n "Query.orderDesc" "src/routes/(console)/project-[region]-[project]/databases/+page.ts"Repository: appwrite/console
Length of output: 283
🏁 Script executed:
rg "Query.orderDesc\(''\)" --context 5 | grep -A 5 -B 5 "Query.limit\(1\)" | head -50Repository: appwrite/console
Length of output: 42
🏁 Script executed:
rg "orderDesc\(''\)" -B 3 -A 3 | grep -E "(Query.limit|Query.offset)" | head -30Repository: appwrite/console
Length of output: 3846
🏁 Script executed:
grep -B 10 -A 10 "Query.orderDesc\(''\)" "src/routes/(console)/project-[region]-[project]/databases/+page.ts" | head -40Repository: appwrite/console
Length of output: 42
🏁 Script executed:
find . -name "*.d.ts" -o -name "*.ts" | xargs grep -l "orderDesc" | grep -v node_modules | head -5Repository: appwrite/console
Length of output: 307
🏁 Script executed:
rg "export.*Query" --type ts --type tsx -A 5 | head -50Repository: appwrite/console
Length of output: 87
🏁 Script executed:
rg "TODO.*orderDesc|FIXME.*orderDesc|BUG.*orderDesc"Repository: appwrite/console
Length of output: 42
🏁 Script executed:
rg "Query\.order" --context 3 | grep -E "(DESC|ASC).*''|''.*DESC" -A 2 -B 2 | head -40Repository: appwrite/console
Length of output: 42
🏁 Script executed:
find node_modules/@appwrite.io -type f -name "*.d.ts" 2>/dev/null | head -10Repository: appwrite/console
Length of output: 42
🏁 Script executed:
cat node_modules/@appwrite.io/console/dist/query.d.ts 2>/dev/null | head -50Repository: appwrite/console
Length of output: 42
🏁 Script executed:
ls -la node_modules/@appwrite.io/console/ 2>/dev/null | head -20Repository: appwrite/console
Length of output: 42
🏁 Script executed:
rg "const Query|class Query|export Query" --max-count 5Repository: appwrite/console
Length of output: 42
🏁 Script executed:
cat -n "src/routes/(console)/project-[region]-[project]/databases/+page.ts" | sed -n '59,90p'Repository: appwrite/console
Length of output: 1339
🏁 Script executed:
rg "Query.limit\(1\)" -A 3 -B 3 | grep -E "(Query.order|Query.limit)" | head -50Repository: appwrite/console
Length of output: 1234
🏁 Script executed:
rg "orderDesc" "src/routes/(console)/project-[region]-[project]/databases/+page.ts" -B 5 -A 5Repository: appwrite/console
Length of output: 1332
🏁 Script executed:
find . -name "*.test.*" -o -name "*.spec.*" | xargs grep -l "orderDesc\(''\)" 2>/dev/null | head -5Repository: appwrite/console
Length of output: 42
🏁 Script executed:
rg "Query\.orderDesc" "src" -B 2 -A 2 | grep -i "comment\|todo\|fixme\|note"Repository: appwrite/console
Length of output: 42
🏁 Script executed:
python3 << 'EOF'
import re
# Patterns to check
patterns = {
'with_field': r"Query\.orderDesc\('(?!\s*')[^']+'\)",
'empty_string': r"Query\.orderDesc\(''\)"
}
# Read the file
with open('src/routes/(console)/project-[region]-[project]/databases/+page.ts', 'r') as f:
content = f.read()
lines = content.split('\n')
print("=== Context around orderDesc usages ===\n")
for i, line in enumerate(lines, 1):
if 'orderDesc' in line:
print(f"Line {i}: {line.strip()}")
# Check if it's in a context where only 1 result is fetched
for j in range(max(0, i-5), min(len(lines), i+5)):
if 'Query.limit(1)' in lines[j]:
print(f" ^ Has Query.limit(1)")
break
print()
EOFRepository: appwrite/console
Length of output: 146
Use '$createdAt' in Query.orderDesc() for consistency.
Line 67 uses Query.orderDesc('') with Query.limit(1) to fetch an entity. However, line 131 in the same file demonstrates the correct pattern for this operation: Query.orderDesc('$createdAt') with Query.limit(1). The empty string should be replaced with '$createdAt' to retrieve the most recent entity and align with the established pattern in this file.
Suggested fix
const res = await databaseSdk.listEntities({
databaseId: $id,
databaseType: type,
- queries: [Query.limit(1), Query.orderDesc('')]
+ queries: [Query.limit(1), Query.orderDesc('$createdAt')]
});📝 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.
| databases.databases.map(async ({ $id, type }) => { | |
| const res = await databaseSdk.listEntities({ | |
| databaseId: $id, | |
| databaseType: type, | |
| queries: [Query.limit(1), Query.orderDesc('')] | |
| }); | |
| tables[$id] = res.tables?.[0]?.$id ?? null; | |
| entities[$id] = res.entities?.[0]?.$id ?? null; | |
| }) | |
| databases.databases.map(async ({ $id, type }) => { | |
| const res = await databaseSdk.listEntities({ | |
| databaseId: $id, | |
| databaseType: type, | |
| queries: [Query.limit(1), Query.orderDesc('$createdAt')] | |
| }); | |
| entities[$id] = res.entities?.[0]?.$id ?? null; | |
| }) |
🤖 Prompt for AI Agents
In @src/routes/(console)/project-[region]-[project]/databases/+page.ts around
lines 63 - 71, Replace the empty string in the Query.orderDesc call so the map
that iterates over databases.databases uses Query.orderDesc('$createdAt') with
Query.limit(1); update the databases.databases.map async callback where
databaseSdk.listEntities is called (the Query.orderDesc('') argument) to
Query.orderDesc('$createdAt') so entities[$id] captures the most recent entity
ID consistently with the rest of the file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thats the default without explicitly specifying afaik.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for other usages of Query.orderDesc('') to see if it's an established pattern
rg "Query\.orderDesc\(''\)" -C 3
Length of output: 27321
🏁 Script executed:
#!/bin/bash
# Search for any comments or documentation about orderDesc with empty string
rg -i "orderDesc.*default|default.*order" --type ts -C 2
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check if there are similar patterns in other database-related files
rg "Query\.orderDesc\(" "src/routes/(console)/project-[region]-[project]/databases/" -C 2 | head -100
Length of output: 7379
@ItzNotABug, you're absolutely right! I can see from the codebase that Query.orderDesc('') is used extensively (40+ occurrences) as the default sort pattern throughout the application. The comment in table-[table]/+page.ts makes this clear:
// don't override if there's a user created sort!
if (!hasOrderQuery) {
queryArray.push(Query.orderDesc(''));
}And the fallback pattern in spreadsheet.svelte confirms it's intentional default behavior. My suggestion was incorrect—the empty string invokes the SDK's default ordering, and the current code at line 67 is fine as-is. Thank you for the clarification!
🧬 Code Graph Analysis Results
[src/lib/helpers/load.ts] lines 23-31
export function getView(url: URL, route: Page['route'], fallback: View, overwrite?: View): View {
const view = url.searchParams.get('view') ?? preferences.getForRoute(route).view;
return view === View.Grid
? View.Grid
: view === View.Table
? View.Table
: (overwrite ?? View.Table ?? fallback);
}
[src/lib/constants.ts] line 3
export const CARD_LIMIT = 6;
[src/routes/(console)/project-[region]-[project]/databases/+page.ts] (summary)
- Description: The page load uses getView(url, route, View.Grid) to determine the current view, effectively applying a default of Grid unless overridden by URL parameters or preferences.
✏️ Learnings added
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2780
File: src/routes/(console)/project-[region]-[project]/databases/+page.ts:63-71
Timestamp: 2026-01-13T11:48:39.107Z
Learning: In Appwrite SDK Query methods, `Query.orderDesc('')` with an empty string is the standard way to apply default sorting behavior (typically by primary key or creation time). This pattern is used throughout the console codebase when no specific field sort is needed, and should not be considered an error or replaced with an explicit field name unless a specific ordering is required.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts:28-28
Timestamp: 2025-10-07T14:17:11.597Z
Learning: In src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts, the empty `vectordb: {}` entry in baseTerminology is intentional. The vectordb database type is not currently used because the backend API is not finalized, and no database type returns 'vectordb' at the moment.
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.ts:17-21
Timestamp: 2025-10-10T12:47:52.651Z
Learning: In the database type handling for the console project, the 'legacy' database type maps to the same API as 'tablesdb' (not 'documentsdb'). Legacy is what tablesDB is based upon - they share the same API with just naming differences. The console does not use legacy SDKs in practice.
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2425
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte:454-468
Timestamp: 2025-09-30T07:41:06.679Z
Learning: In `src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte`, the column suggestions API (console.suggestColumns) has a maximum limit of 7 columns returned, which aligns with the initial placeholder count of 7 in customColumns.
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/view.svelte:84-84
Timestamp: 2025-10-09T12:22:41.099Z
Learning: In the Appwrite console databases UI, user-facing labels like "Columns" should remain unchanged even when internal terminology changes to "fields". The multi-terminology support refactors internal data models without changing the user-facing text.
What does this PR do?
(Provide a description of what this PR does.)
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)
Related PRs and Issues
(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)
Have you read the Contributing Guidelines on issues?
(Write your answer here.)
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.