feat(tables): typed predicate filter grammar, cursor pagination, and the v2 table surface - #6067
Merged
Conversation
…gine, cursor pagination - Unify row-matching on one `fieldPredicate` leaf (filter compiler, upsert conflict probe, unique checks) — fixes the upsert wedge on case-mismatched unique values; equality/in is case-sensitive everywhere, text matches stay ILIKE - v2 nestable all/any predicate grammar: types, `buildPredicateClause`, structured contract schema, query-builder converters (+ `predicateToFilter` bridge) - Cursor pagination: opaque codec + `QueryResult.nextCursor` (offset gone on v2 surface) - `table_v2` block + `table_query_rows_v2` tool + POST /api/table/[tableId]/query route; v1 `table` hidden from toolbar; bulk update/delete author predicates too - Agent grammar: regenerate copilot tool-catalog TS; `user_table` server tool parses predicates (query → predicate, bulk → predicateToFilter) - Fix `replaceTableRowsWithTx` row[col.name] → row[getColumnId(col)] keying bug Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
# Conflicts: # apps/sim/blocks/registry.ts # scripts/check-api-validation-contracts.ts
…d query with byte guard - New parser lib/table/query-builder/postgrest.ts: PostgREST querystring (wins=gte.10&status=in.(active,pending), or=()/and=() groups, not. prefix) -> TablePredicate IR; serializers for the visual builder path - Contract/tool/route/copilot user_table: filter/order are PostgREST strings, parsed + validated server-side - table_v2 block: Builder/Editor filter mode dropdown (visual builders serialize to PostgREST), builder-only fields hidden from the LLM - queryRows: no default row limit; 10MB result byte guard; engine like/ilike ops; bulk update/delete deterministic (order_key, id) ordering under limit Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bd25zCzh21omjFhRz32ov6
# Conflicts: # apps/sim/blocks/registry-maps.minimal.ts # apps/sim/lib/table/rows/service.ts # apps/sim/lib/table/validation.ts # scripts/check-api-validation-contracts.ts
…d cursor hardening
queryRows now drains rows in adaptively-sized bounded batches instead of one
unbounded SELECT. Omitted limit returns the entire result and fails fast (400)
past the 5MB byte budget; bounded pages byte-cut early and signal remaining
rows via nextCursor (witnessed by a peek row, never inferred from page size).
Cursor codec gains a compound {k,i,o} shape for unkeyed-row resumes and a
keysetValid gate closing the fractional-flag-off keyset/order mismatch.
Council must-fix cluster:
- Serializer round-trip: whole-pattern quoting (contains with dots), backslash
quote escaping, nlike/nilike engine ops (ncontains parses again), isEmpty
desugars to or=(f.is.null,f.eq."") preserving null-or-empty semantics
- Unconditional name→id translation on PostgREST string paths (session-auth
filters no longer silently match zero rows)
- Cursor decode hardening (object guard, o>=0), 400 on keyset cursor + order
- Block: cursor "null" artifact guard, NaN limit fails fast, filterBuilder
required-flag removed (serializer hard-block with agent-set filter string)
- Parser: reject empty or=()/and=()/in.(), Number.isFinite, strict sort dirs
- TableQueryValidationError moved to client-safe lib/table/errors.ts (drops
the drizzle-orm edge from client-bundled block defs)
Consumers: export stops on nextCursor (byte-cut pages no longer truncate),
legacy/v1 routes expose nextCursor additively, v2 route drops executions,
copilot query_rows reports partial pages with a continue offset, v1 Table
block tools registered in the minimal registry. Agent catalog regenerated
from the copilot fork (PostgREST string filter, order param, new pagination
semantics).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bd25zCzh21omjFhRz32ov6
Replace the PostgREST querystring wire with the typed
{all|any:[{field,op,value}]} predicate object across the engine, contracts,
internal query/bulk routes, the public v2 read API (GET /api/v2/tables + POST
.../query), the table_v2 block (canonical Builder/JSON filter toggle), the
query_rows_v2 tool, and the copilot user_table tool. Adds validatePredicate
schema-aware validation. Track A engine hardening: read-path statement timeout,
comparator negation (not.gt family), createdAt/updatedAt filtering, machine
error codes, cursor version field. Mothership pagination is now cursor-only
(offset removed from the agent surface).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UBRahKrk4BV23spVMckskA
Brings 97 commits incl. #5503 (remove tables-fractional-ordering flag — fractional ordering now unconditional), #5465 (date canonicalization + effective-timezone render), #5492 (server-authoritative run badge). Conflicts resolved keeping our predicate-object grammar + cursor pagination: - rows/service.ts: dropped staging's offset buildPageQuery (we use the byte-bounded cursor drain); made fractional ordering unconditional per #5503 (removed fractionalOrdering flag threads + isFeatureEnabled calls). - sql.ts: legacy simple-equality routes through the unified fieldPredicate leaf (+ staging's JsonValue cast). - validation.ts: kept both USER_TABLE_ROWS_SQL_NAME (our fieldPredicate) and normalizeDateCellValue (staging dates). - index.ts: export both dates + errors. - check-api-validation baseline 919 -> 922 (+v2 list/query + internal query). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UBRahKrk4BV23spVMckskA
Adds a runtime feature flag gating the three v2 HTTP surfaces (GET /api/v2/tables, POST /api/v2/tables/[tableId]/query, POST /api/table/[tableId]/query), returning 404 when off. Gated by userId + the workspace's org cohort via AppConfig; off-AppConfig falls back to the TABLES_V2_API secret (off by default). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011M563cbFy2S74GvSDf2C3R
Brings 326 commits. Conflict resolutions: - registry.minimal.ts: union of our table tools and staging's slack tools. - database.mock.ts: took staging's chain-spy rewrite (already supports the .limit(n).offset(m) chain the drain loop needs). - service-filter-threading.test.ts: dropped the dead tables-fractional-ordering feature-flag mock (#5503 removed the flag) and the redundant @sim/db mock. - env.ts: kept TABLE_MAX_PAGE_BYTES plus staging's dispatch-concurrency vars. - tool-schemas-v1.ts: took staging's generated output; the table grammar is regenerated from the Go contract once that branch lands. - feature-flags.test.ts: ported the tables-v2-api tests to setEnvFlags. - check-api-validation baseline 975 -> 978 (+v2 list/query + internal query). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011M563cbFy2S74GvSDf2C3R
table_v2 was registered ungated in the toolbar while v1 Table was hidden, so a deployment with tables-v2-api off left users with only a Table block whose default Query Rows operation 404s behind the flag. Marks table_v2 `preview: true` — hidden from every discovery surface until revealed via the hosted block-visibility AppConfig document or PREVIEW_BLOCKS, fail-closed, with execution of placed instances never gated. Drops the premature `hideFromToolbar` from v1 Table so it stays available during rollout; v1 gets marked superseded at table_v2 GA, alongside its BlockMeta and docs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011M563cbFy2S74GvSDf2C3R
`order_key` is nullable — rows predating the backfill script-migration, and
forked rows that inherit a NULL key. A bare `(order_key, id) > (:k, :i)` row
comparison evaluates to NULL for those rows, so WHERE drops them; because NULLs
sort LAST, the whole unkeyed tail became unreachable and the drain terminated
early reporting `hasMore: false`. Reproduced on a 121-row table: 52 rows
returned, `nextCursor: null`, no error. Affected the grid, CSV export, the
snapshot cache, the v2 API, and copilot queries.
Admits `order_key IS NULL` in both seeks (`fetchRowsBounded`'s drain and
`selectExportRowPage`), which restores the tail and makes the compound
`{k,i,o}` cursor's offsetFromAnchor accounting resolve — it was unreachable
before. `selectExportRowPage` additionally seeks by anchor kind, since its
anchor is the previous page's last row and can itself be unkeyed; its return
type no longer casts the nullable column to `string`.
Also stops workspace forking minting new NULLs: it spread `...row` into a fresh
tableId that the one-shot backfill never revisits, so copied rows now get keys
appended after the source's max, preserving visual order.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011M563cbFy2S74GvSDf2C3R
…ole table `runTableDelete` built its WHERE as `filter ? buildFilterClause(...) : undefined` with no guard, unlike `runTableUpdate` and the inline paths. A filter that was supplied but compiled to nothing therefore deleted every row past the cutoff — `and()` drops an undefined clause silently. Tables at or under the inline bulk cap 400'd, so only larger tables were affected. Three inputs reached that state while passing every check: - `predicateToFilter` claimed to be lossless but emitted leaves that `buildFilterClause` discards: `op:'eq'` with an array value (the realistic trigger — an LLM reaching for `in` and writing `eq`) and a value-taking op with no value. It now throws instead. - `predicateSchema` allowed an empty `all`/`any` group; both now require `.min(1)`. - `validateLeaf` only checked `in`/`nin` emptiness. It now also rejects a missing value and an array on a scalar op, so the copilot path — which has no Zod — fails the same way the HTTP boundary does, and caps `in`/`nin` list length at 1000 (each element becomes its own containment clause). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011M563cbFy2S74GvSDf2C3R
Two shapes let a destructive filter execute differently than it validated. A node carrying both a group key and `field` was read group-first by the engine and validator but leaf-first by predicateToFilter/predicateNamesToIds, so the gate validated one predicate while the bulk-write path executed another — bypassing the unknown-column, json-op, and empty-list checks on the copilot's delete/update path. validateNode now rejects it, and both converters discriminate group-first so unvalidated callers can't disagree either. filterRulesToPredicate skipped any builder rule without `column`, which silently dropped predicate-shaped members when the two grammars were mixed — turning "delete archived rows for tenant acme" into "delete archived rows for every tenant". It now throws and points at the predicate object; a genuinely blank builder row is still ignored. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011M563cbFy2S74GvSDf2C3R
Closes #5920 and applies the pre-ship remediation from the branch review. Built-in columns (#5920) - Add `id` as a system column alongside `createdAt`/`updatedAt`. It previously fell through to `data->>'id'` — a JSONB key that never exists — so filtering by id matched nothing and sorting by id ordered by NULL, despite the docs advertising all three as filterable and sortable. - Normalize timestamp bounds with `::timestamptz AT TIME ZONE 'UTC'`. `created_at`/`updated_at` are `timestamp WITHOUT time zone` holding UTC, so a bare `::timestamptz` promoted the column using the session TimeZone GUC and shifted every bound by the server's offset. Verified on a real 121-row table: the reporter's UTC-3 day range returned 114 rows under America/Sao_Paulo vs 112 under UTC; the fix returns 112 in every session timezone. - Range operators on `string` columns now compare as text instead of falling back to a `::numeric` cast that produced the misleading `... (string) requires a number, got string`. `boolean`/`json` ranges are rejected with a message naming the real type. Rollout safety - Move the `tables-v2-api` flag gate below the authz check on all three routes. Ahead of authz it did a primary-DB read on a caller-supplied workspaceId and its 404-vs-403 split leaked which orgs are in the rollout cohort. - Remove `match`/`imatch` entirely. They were newly added to the legacy `$`-allowlist on this branch, making POSIX regex reachable on the *ungated* v1 public API while the gated v2 route rejected it as a pool-pinning risk. Nothing shipped depends on them; the route-local `assertNoRegexOps` goes away with them. - Restore the bounded-page byte cut as opt-in (`TABLE_MAX_PAGE_BYTES`, default off) to match staging. A short page is only safe for clients terminating on `nextCursor === null`; a pre-existing v1 pager stopping at `rows.length < limit` would read the cut as end-of-data. Unbounded queries still fail fast at the 5MB budget rather than return a partial result. DoS bounding - Cap predicate nesting depth (10) and total nodes (500) with an iterative pre-check. The recursive `z.lazy` union overflowed the stack inside `safeParse` on a deeply nested tree — a RangeError escaping a parser is a 500, not a 400. - Cap the row-query body at 1MB (the 50MB platform default let a caller buffer two orders of magnitude more before any schema check ran). - Route the bulk PUT/DELETE bodies through `parseJsonBody` so the destructive surface gets the platform body cap instead of a raw `request.json()`. Hygiene - Rename the query-builder's `SORT_DIRECTIONS` option list to `SORT_DIRECTION_OPTIONS`; it collided with the wire-level tuple and both were star-exported through `lib/table/index.ts`, so the name resolved to nothing. - Drop stale PostgREST references from comments and error messages. Tests - System-column coverage for id (filter, sort, pattern ops), UTC normalization, and the legacy `$`-grammar path. - The NULL-admitting keyset seek — asserted to fail against the pre-fix comparison — plus cursor round-trip continuity across pages. - `nlike`/`nilike` SQL, predicate depth/size rejection, flag-off 404 and gate-ordering on all three v2 routes, and the byte cut being off by default. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011M563cbFy2S74GvSDf2C3R
…he v2 gate
A `{ status: { $eq: 'x' } }` filter is neither a group nor a leaf, so it fell
through to `validateLeaf` with `field: undefined` and came back as
`Unknown filter column "undefined"` — a message that sends an LLM caller
retrying column names instead of switching grammars.
This is reachable today: the copilot's `query_user_table` catalog entry still
advertises `filter: MongoDB-style filter for query_rows` plus `offset`/`sort`,
while the tool now routes through `validatePredicate` (rejects the $-grammar)
and reads only `order`/`cursor` (so `offset`/`sort` silently no-op). Fixing the
catalog belongs upstream in the mothership repo; this makes the failure legible
in the meantime.
Also:
- Point both table blocks' docsLink at docs.sim.ai/integrations/table. They were
the last two blocks in the repo still on the dead docs.simstudio.ai domain.
- Fix the openapi `sort` example, which showed `{"created_at": "desc"}`. The
built-in column is `createdAt`; the snake_case form is treated as a user
column, so anyone copying the example sorted by a JSONB key that never exists
and got NULL ordering — the same silent-wrong-answer class as #5920, on the
already-shipped v1 surface.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011M563cbFy2S74GvSDf2C3R
…ntly narrowing
Found by running the HTTP suite: a node carrying BOTH a group key and a leaf's
`field`/`op`/`value` returned 200, not 400.
Zod strips unrecognized keys by default, so `{ all: [...], field, op, value }`
parsed clean against the group branch with the leaf half quietly deleted. The
hybrid guard added in eaf4179 could never fire — the keys were gone before
`validatePredicate` ran. On the bulk paths that turns "delete archived rows for
tenant acme" into "delete EVERY row for tenant acme".
Both node shapes are now `strictObject`. Strict on the group alone would be
worse than the bug: the union would fall through to the leaf branch, which is
the more dangerous reading of the two.
The bulk schemas are unaffected by design — their legacy `$`-object branch
accepts any non-empty object, so it absorbs the hybrid WITHOUT stripping, the
route's `isTablePredicate` check routes it back to `validatePredicate`, and the
runtime guard rejects it there. Tests now pin both layers so removing either
one fails loudly.
Verified against a running server on the `hello` table: 18/18 HTTP checks pass,
including the previously-failing hybrid case.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011M563cbFy2S74GvSDf2C3R
Cross-version safety net. If a client speaking the predicate grammar reaches a
server that predates it, the predicate arrives at the legacy `$`-compiler as
`{ all: [...] }`. That was skipped as "an array on a regular field", so the
filter compiled to NO WHERE CLAUSE — which on a bulk delete means every row
rather than none. `update-runner` has always had an `if (!filterClause) throw`;
`delete-runner` does not, so the background delete path (tables over 1000 rows)
was the one that could actually wipe a table.
`buildFilterClause` is the single choke point every filter path shares —
`queryRows`, `update-runner`, `delete-runner`, inline and background — so one
guard there covers all of them, and it names the mismatch instead of failing
with a generic "filter required".
Scoped to the `all`/`any` discriminators specifically: an ordinary column that
happens to hold an array stays a silent skip, so no working legacy filter
changes behaviour.
This matters for deploy ordering. The copilot and sim deploy independently, and
if the copilot ships the new grammar first it starts sending predicates to a sim
that cannot parse them. With this guard that is a loud 400 on every path instead
of a silent table wipe on one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011M563cbFy2S74GvSDf2C3R
Clearing the Filter or Order field in Editor mode is the ordinary way to say
'no filter'. `JSON.parse('')` throws, so it surfaced at run time as
`Invalid JSON in Filter: Unexpected end of JSON input` plus a quoting hint
that has nothing to do with the actual problem.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011M563cbFy2S74GvSDf2C3R
37 commits. Staging landed two table features that had to be woven into the v2
work rather than merged around it:
**`select` columns.** Staging replaced `rowDataIdToName` with `namedRowMapper`,
which fuses the id→name key remap with select-cell VALUE formatting, and added
`resolveFilterSelectValues` for the inbound direction. Both v2 read paths were
still on the old key-only mapper, so a select cell would have surfaced its
stored option id instead of the option name.
The inbound half had no predicate-grammar equivalent at all, so a v2 filter like
`{field:'status', op:'eq', value:'Open'}` compared the option NAME against the
stored option ID and silently matched nothing. Added
`resolvePredicateSelectValues` beside its `$`-grammar sibling and wired it into
`row-wire`, the v2 query route, and the copilot executor.
Select-column operator gating and the multi-select array-membership clause moved
from `buildFieldCondition` down into `fieldPredicate`, so the predicate grammar
gets them too rather than only the `$` grammar. `fieldPredicate` now takes the
full `ColumnDefinition` instead of just its type — `options`/`multiple` are what
the select branches need. The equality shorthand on a multi-select still maps to
membership while an explicit `eq` still errors, matching staging.
**Per-table mutation locks.** Additive; `tables-v2-api` and `table-locks` sit
side by side in the flag registry.
Also: `TableQueryValidationError` moved to `lib/table/errors` on our side, so
staging's v1 rows route import needed repointing, and `formatCsvValue` was
renamed `formatCsvCell` upstream.
Route-count baseline 978 + staging's 979 → recomputed, not added.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011M563cbFy2S74GvSDf2C3R
…elect
Caught by driving mothership at a real multi-select table. It sent a correctly
formed predicate — {field:'Color', op:'contains', value:'Teal'} — and got zero
rows with success, against a table where 15 rows hold Teal.
resolvePredicateSelectValues only resolved eq/ne/in/nin. I excluded
contains/ncontains as 'pattern ops that match the raw stored cell', which holds
for a string column but not for a multi-select: there the cell is an array of
option ids and those two ops express MEMBERSHIP, so their operand is an option
name that has to become an option id. It is the primary way to filter a
multi-select, so the one op that mattered most was the one left out.
The $-grammar sibling resolveFilterSelectValues has always handled
$contains/$ncontains for this exact reason; the omission was mine, porting it.
The remaining pattern ops (like/startsWith/...) never reach here — fieldPredicate's
select allowlist rejects them on select columns first.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011M563cbFy2S74GvSDf2C3R
table_query_rows_v2 starts with 'table_query_rows_', so the resource-suffix strip normalized it to the v1 tool. The executor then logged the v2 id while issuing v1's request shape — GET /rows?filter=<predicate> instead of POST /query with a predicate body — so a correctly configured table_v2 block 400'd with 'Filter looks like a v2 predicate tree but reached the legacy filter compiler'. The guard was right; the tool resolution was wrong. A trailing _v<n> is a version marker, not a resource id, so it is no longer stripped. Versioned ops are matched longest-first and listed alongside their unversioned form, so table_query_rows_v2_<tableId> still normalizes to table_query_rows_v2 rather than collapsing to v1. Applies to the knowledge ops too — same loop shape, same latent trap the first time one of them is versioned. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011M563cbFy2S74GvSDf2C3R
The block's own filter — {"all":[{"field":"Color","op":"contains","value":"Teal"}]}
— returned 0 rows against a table where 15 rows hold Teal.
Translating a name-keyed predicate to storage keys is two steps: column names →
column ids, and select operands → option ids. Both are required, neither is
useful alone, but they were two separate calls each boundary had to remember to
pair. Three did not: the internal query route (the table_v2 block's own path),
the bulk update/delete resolver, and — before this branch — nothing else needed
it, so the gap was invisible until select columns landed.
Replaced with a single `predicateToStorage(predicate, schema)` and migrated
every call site, so the pair cannot be split again. `predicateNamesToIds` now
has no direct callers outside it.
Also fixes four type errors that a failed inference in contracts/tables.ts was
masking — once the leaf schema type-checked, tsc surfaced the rest:
- `ColumnType` was imported from lib/table/types but never exported there (it
lived as a local alias in sql.ts). Now exported once, next to ColumnDefinition.
- rows/service.ts referenced TableRowsCursor in three signatures without
importing it.
- export-runner and snapshot-cache still declared their paging cursor's orderKey
as non-null, after selectExportRowPage was corrected to return the nullable it
always had.
- the predicate leaf's `z.unknown()` value infers wider than Predicate['value'];
narrowed with an annotated cast, runtime unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011M563cbFy2S74GvSDf2C3R
30 commits: saved table views (#5961), the generic folder engine, desktop app, and the raw-sql Date-bind fix. Two conflicts: bulk-filter contract fields keep our dual-grammar bulkFilterSchema while adopting staging's workspaceIdSchema primitive; route-count baseline recomputed by running the audit (996 = staging's 993 + our 3 v2 routes). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011M563cbFy2S74GvSDf2C3R
…commit The 79d0dd0 merge recorded the pre-merge env-flags.ts: the path was reset out of the index mid-merge to keep a local debug edit unstaged, which also discarded staging's isSessionPoliciesEnabled / isCopilotToolPermissionsEnabled exports and broke six importers added by the desktop-app PR (#5998). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011M563cbFy2S74GvSDf2C3R
Views shipped (#5961) storing the legacy `$`-object filter and `{col: dir}` sort record — a brand-new persistent store of the grammar this branch is retiring, created days before the wire moved to predicates. The feature is still dark (`table-views` is UI-only and off), so the stored shape can change now without a data migration; once the flag flips, it cannot. `TableViewConfig` now carries `TablePredicate` + `SortSpec`. The wire contract uses `predicateSchema`/`sortSpecSchema`, which also brings the strict-object node shapes and depth/size bounds to the view routes — previously a view's filter was accepted as an arbitrary domain object. The grid still runs on the legacy pair internally; translation happens at the view boundary. Apply: `predicateToFilter` (total here — stored predicates are builder-authored). Save: `filterToRules ∘ filterRulesToPredicate`, the builder round-trip. SortSpec keeps priority order the record never could. Dev-era rows written before the switch are normalized on read: legacy filters convert through the builder round-trip and are dropped if the result's leaf fields fail the column-name pattern — the rule converters accept garbage (`{$bogus: …}` becomes a rule on a column literally named `$bogus`), so the conversion is validated rather than trusted. Also folds `sortQuery`'s single-entry record out of the save path in favour of the sort params directly, so a saved view records the same thing the URL says. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011M563cbFy2S74GvSDf2C3R
The grid was the last surface authoring the legacy `$`-grammar, which forced views to downgrade on apply and upgrade on save, and kept five wire fields legacy-only. Its runtime state, filter bar, and every request it makes now speak `TablePredicate`/`SortSpec`. Wire: the five grid-carrying fields (rows GET filter+sort, find filter+sort, delete-async, cancel-runs, columns-run) accept a dual-grammar union — strict predicate tree first, legacy fallback — so external v1 callers are untouched. The rows read path takes predicates NATIVELY into queryRows (no downgrade); the job/dispatch routes downgrade via predicateToFilter at entry, which throws on any leaf the legacy compiler would silently discard, so persisted job payloads stay legacy and the runners are untouched. Grid: filter state is TablePredicate, the filter bar converts rules with filterRulesToPredicate — now select-aware (a numeric-looking option id is no longer scalar-coerced, matching filterRulesToFilter) — and stale-operator pruning uses a new prunePredicateForColumns that fails CLOSED to "no filter" on malformed values instead of taking the page down. The view apply/save boundary conversions added earlier are deleted: views and grid now share one grammar end to end. isTablePredicate moved from a route-local into converters as the shared dual-wire discriminator; toLegacyFilter/toLegacySort live there too (pure grammar code — keeping them in app/api/table/utils broke every test that wholesale-mocks that module). Legacy converters now have exactly one live consumer: the v1 table block, whose tools still speak the $-wire by contract. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011M563cbFy2S74GvSDf2C3R
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryHigh Risk Overview New read surfaces (feature-gated Safety and validation tighten destructive paths: strict predicate schemas, depth/node caps, hybrid group+leaf rejection, wire-filter validation before UI and blocks: filter bar, views, and hooks use Reviewed by Cursor Bugbot for commit 96d49b8. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 35187658 | Triggered | Username Password | 79d0dd0 | apps/desktop/src/main/browser-credentials/vault.test.ts | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
Greptile SummaryFollow-up re-check of the three prior dual-grammar review threads against HEAD.
Confidence Score: 5/5The PR appears safe to merge with respect to the previously reported dual-grammar issues; those fixes hold at HEAD. No blocking failure remains from the prior hybrid-guard, all/any discrimination, or bulk name-vs-id keying threads; current code and tests establish each is addressed.
|
| Filename | Overview |
|---|---|
| apps/sim/lib/table/query-builder/converters.ts | Hybrid lossless-or-throw conversion and array-based predicate discrimination land the prior hybrid and all/any fixes. |
| apps/sim/lib/table/query-builder/validate.ts | Shape and storage-key validation choke points cover hybrid nodes, empty groups, and post-wire field checks. |
| apps/sim/app/api/table/[tableId]/rows/route.ts | Bulk PUT/DELETE and GET resolve predicates through wire translation then storage validation as required by the prior bulk-keying thread. |
| apps/sim/app/api/table/[tableId]/delete-async/route.ts | Async delete downgrades with toLegacyFilter and shape/storage validation so hybrids 400 instead of widening. |
| apps/sim/app/api/table/utils.ts | Shared tableFilterError validates wire predicates against storage keys for the async destructive routes. |
Reviews (5): Last reviewed commit: "Merge remote-tracking branch 'origin/sta..." | Re-trigger Greptile
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
table_create and table_list were the only two of the 13 Sim table tools absent from minimal mode. No block routes to them today, but minimal mode should not silently lack table operations the full registry has — that asymmetry is exactly how the earlier 'table block missing in minimal registry' round trips happened. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011M563cbFy2S74GvSDf2C3R
Documents the two public v2 endpoints exactly as implemented — the predicate grammar (full operator enum with per-type and select-cardinality semantics, the strict-node and depth/size bounds), cursor pagination with its null-only termination contract, limit=0 unbounded semantics with the 5MB fail-fast, the camelCase built-in columns, and the flag-off 404 behaviour. Deliberately NOT wired into the docs site loader: the surface is dark behind tables-v2-api, and publishing reference docs for an endpoint that 404s would be premature. The filename matches PR #5273's multi-spec layout so adoption is a one-line loader change at GA. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011M563cbFy2S74GvSDf2C3R
From the PR #6067 review (greptile + bugbot), all verified before fixing: 1. Hybrid nodes on the async destructive routes (P1/High). The dual union's legacy branch accepts any non-empty object WITHOUT stripping, so a node carrying both a group key and leaf keys reached toLegacyFilter Zod-approved — and predicateToFilter converted it group-first, silently DROPPING the leaf and widening a select-all delete. predicateToFilter now throws on hybrids (lossless-or-throw, like its other rules), toLegacyFilter shape-validates first, and the GET native-predicate path shape-validates too. 2. A column literally named all/any (P1). NAME_PATTERN allows it, and isTablePredicate routed any object with those keys to the predicate compiler. It now requires the group value to be an ARRAY: the legacy equality shorthand and operator objects on such a column keep compiling as legacy, and an array-valued legacy condition was always a dropped no-op, so predicate precedence on arrays regresses nothing. 3. Bulk keying (P2). resolveBulkFilter validated predicates as NAME-keyed and translated unconditionally — wrong for the ID-keyed grid (session wire is identity). Validation now runs AFTER wire translation against STORAGE keys (new validateStoragePredicate), which is keying-correct for every caller and keeps the property that a typo'd column on a destructive path is a 400, not a silent match-nothing no-op. 4. 500s on downgrade rejection (Medium). delete-async called toLegacyFilter outside its try, and cancel-runs/columns-run mapped the throw to the generic 500. All three now return the validation message as a 400. 5. SortSpec broke the wire (High). requestJson threw on arrays of objects, so any active grid sort died client-side before the request — and the server contract rejected string-encoded values anyway, on both grammars. Arrays containing objects now travel as one JSON-string param (exactly what the serializer's own guard comment prescribed), and the rows/find query contracts decode JSON-string filter/sort/after before the union runs. Proven end-to-end with a real NextRequest for both grammars. Session bulk predicates are now id-keyed pass-through (matching the grid); name-keyed translation remains for INTERNAL_JWT workflow tools — tests updated to the corrected contract and extended for every finding. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011M563cbFy2S74GvSDf2C3R
Collaborator
Author
|
@greptile review |
Collaborator
Author
|
@cursor review |
Collaborator
Author
|
@greptile review |
Bugbot round 2 on PR #6067, both verified: Empty groups (High). `{all: []}` fails the strict predicate branch (.min(1)) but slips the dual union via the legacy branch — an empty ARRAY inside a non-empty OBJECT — then downgraded to `{$and: []}`, which compiles to no WHERE clause: a run/cancel/delete scope silently widened to every row. validatePredicateShape now mirrors the contract's .min(1), which closes it at every dual-grammar boundary at once (toLegacyFilter, resolveBulkFilter, the GET native path). Cursor↔sort binding (Medium). CURSOR_SORT_CONFLICT only fired for keyset cursors; offset cursors — the shape sorted views actually emit — carried no record of their ordering, so one minted under sort A replayed under sort B (or none) silently paged the wrong sequence. Offset cursors are now stamped with a canonical fingerprint of their sort at mint (queryRows), and a shared assertCursorSortBinding enforces the match at all three consumers (both query routes and the copilot executor), replacing the three hand-rolled keyset-only checks. Keyset/compound cursors stay default-order-only by construction. The OpenAPI cursor wording now states the binding rather than overclaiming. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011M563cbFy2S74GvSDf2C3R
…odes, resolve coerced select names Bugbot round 3 on PR #6067, all three verified: Async routes (Medium). delete-async / cancel-runs / columns/run validated only the toLegacyFilter downgrade, which compiles a typo'd field into a clause that silently matches nothing — a filtered delete/run/stop no-ops where the sync bulk routes 400. tableFilterError is now grammar-aware and takes the WIRE filter: predicates go through validateStoragePredicate (same keying the sync routes enforce), legacy filters keep the buildFilterClause check. Dual all/any node (High). {all:[...], any:[...]} fails both strictObject branches, survives the legacy union, and every group-first traversal reads `all` and silently DROPS `any` — half the conditions vanish, widening a bulk delete/update. validateNode (covering every boundary and the copilot tool's validatePredicate) and predicateToFilter (lossless-or-throw) both reject it; nesting expresses the same intent unambiguously. Coerced select names (Medium). The block builder serializes without schema access, so an option NAME that looks numeric/boolean ("123") arrives scalar-coerced and resolveSelectOptionId bailed on non-strings — the filter compared 123 against the stored option id and matched nothing. Stringify scalar operands before matching, fixing every name-keyed caller (v1 and v2 blocks) at the resolution seam instead of per-surface. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011M563cbFy2S74GvSDf2C3R
Collaborator
Author
Collaborator
Author
|
@cursor review |
Bugbot round 4: the native predicate path on GET /rows ran only the shape check before wire translation, and find ran none before its toLegacyFilter downgrade — so a typo'd field compiled to a clause matching nothing and read back as a plausible empty page, where the bulk write paths 400. GET now runs validateStoragePredicate post-translation (name-keyed JWT callers validate their translated form, same recipe as resolveBulkFilter); find reuses the grammar-aware tableFilterError gate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011M563cbFy2S74GvSDf2C3R
# Conflicts: # scripts/check-api-validation-contracts.ts
Collaborator
Author
Collaborator
Author
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 96d49b8. Configure here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
{all|any: [{field, op, value}]}— replacing Mongo-style$-objects across the engine, contracts, routes, block, and copilot. Both grammars compile through one shared SQL leaf, so semantics are identical; the$-grammar remains only on the v1 public API / v1 block (contract-frozen) and as the fallback branch of dual-grammar wire fieldsGET /api/v2/tables+POST /api/v2/tables/[tableId]/query, gated behind a newtables-v2-apifeature flag (404 when off, gate runs after authz). InternalPOST /api/table/[tableId]/queryfor first-party callers(order_key, id)) replacing offset on the new surfaces;limitomitted returns the entire result and fails fast past a 5MB budget instead of truncatingtable_v2block (preview-gated viaPREVIEW_BLOCKS/AppConfig) with canonical Builder ⟷ JSON toggles for Filter and Order; v1 Table block unhidden and unchangedFixes folded in
order_key(reproduced: 52 of 121 rows returned) — seek now admits the unkeyed tail; same fix applied to CSV export and the snapshot cacheidis now a real filterable/sortable system column, and timestamp bounds normalize viaAT TIME ZONE 'UTC'so results no longer shift with the session timezonecontains/ncontainson multi-select included); name→storage translation is now one operation so the pair can't be half-applied againnormalizeToolIdstripped_v2as a resource suffix, silently executing the v1 tool under the v2 tool's namein-list cap, 1MB query body cap, strict-object predicate nodes (Zod key-stripping could silently widen a bulk delete)Rollout notes
TABLES_V2_APIoff by default;table_v2hidden everywhere until revealed. Kill switch verified: flag off → all three routes 404, v1 untouchedlower()removed from unique-constraint checks, so upserts distinguish case-variant values — tables already holding case-variant duplicates keep working, but the previous silent case-folding is goneType of Change
Testing
Type-check clean, 3147 tests passing across the table/contract/route/hook suites,
check:api-validation:strict+ full audit suite green. Live HTTP suite against a real DB: 18/18 (NULL-order_key paging returns all 121 rows, #5920 ranges match SQL ground truth, DoS bounds, flag gating). Manually drove the block + grid + mothership agent against a local Go build on the new grammar.Checklist
🤖 Generated with Claude Code