Add inspector SQLite shell and property bindings#4410
Add inspector SQLite shell and property bindings#4410NathanFlurry wants to merge 7 commits intomainfrom
Conversation
|
🚅 Deployed to the rivet-pr-4410 environment in rivet-frontend
|
railway-app
bot
temporarily deployed
to
rivet-frontend / rivet-pr-4410
|
PR Review: Add inspector SQLite shell and property bindings This is a solid feature addition. The backend refactoring (PRAGMA-based schema introspection, named bindings, withDatabase lock helper) is a genuine improvement over the previous approach. BUGS parseEditedCellValue regex truncates type names (actor-database.tsx): The regex /\b(int|real|floa|doub|dec|num|bool)\b/ uses truncated fragments 'floa' and 'doub' with word boundaries that prevent matching the actual SQLite type names FLOAT or DOUBLE. Values in those columns will not be coerced to numbers. Fix: use full names 'float' and 'double'. DEFAULT_SQL hardcodes test_data (actor-database.tsx): Leftover development artifact. Any actor without a test_data table will see an error when opening the SQL shell. Better default: SELECT name FROM sqlite_master WHERE type = 'table'. CORRECTNESS applyEdits is not transactional (actor-database.tsx): Staged cell edits are applied as individual sequential UPDATE statements. If one fails midway, earlier updates are already committed and the UI shows the batch as still pending. Wrapping the batch in BEGIN/COMMIT/ROLLBACK would make multi-cell edits atomic. Null round-trip is broken (actor-database.tsx): formatCellDraft returns an empty string for null. When saved, parseEditedCellValue returns empty string not null. A NULL cell cannot be edited back to NULL unless the user types the word null. Consider using 'NULL' as the initial draft value for null cells to match expected parsing behavior. MINOR normalizeSqlitePropertyBindings triple-keys every property (actor-inspector.ts): Each unadorned key is expanded to three entries (colon, at-sign, dollar). Only one is ever used by SQLite. A brief comment explaining the intent would help future readers. withDatabase relies on result as T cast (actor-inspector.ts): The let result pattern works today but is fragile. Returning directly from the lock callback would be more explicit. jsonSafe silently truncates bigints (actor-inspector.ts): BigInts beyond MAX_SAFE_INTEGER lose precision. Acceptable for a debugging tool but worth a comment. Named binding extraction does not skip string literals (actor-database.tsx): extractNamedBindings matches raw SQL, so bindings inside quoted strings generate spurious UI fields. Minor UX issue, not a security concern. TEST COVERAGE: Happy-path tests are good. Suggested additions: empty sql, both args and properties supplied simultaneously, invalid binding type, malformed SQL. DOCUMENTATION: Additions in debugging.mdx, sqlite.mdx, and skill-base-rivetkit.md are clear and accurate. SUMMARY: The two bugs (regex truncation and DEFAULT_SQL) and the null round-trip should be fixed before merge. Non-transactional edit batching is a correctness concern worth addressing. Everything else is minor polish. |
1 participant
Description
This adds a manual SQLite shell to the actor Inspector UI, including query execution, result rendering, and editable named-property inputs that re-run the preview. It also adds
POST /inspector/database/executesupport for both positionalargsand namedproperties, updates the raw SQLite binding helpers, and documents the new flow.Type of change
How Has This Been Tested?
Ran
pnpm exec vitest run tests/driver-memory.test.ts -t "POST /inspector/database/execute|GET /inspector/state returns actor state"inrivetkit-typescript/packages/rivetkit.Checklist: