Conversation
…367) * perf(leads): partial index to avoid seq scan on tags-other exclusion Every /api/v1/leads request runs `NOT (tags @> '{"other"}')`, a negated GIN predicate that can't use idx_leads_tags. The exact-count query (no other selective filter) fell back to a full seq scan — measured 1467.9ms on stage's Admizz tenant (16,683 active rows). Added a partial index carrying the same predicate as the WHERE clause, so the planner can prove implication and use it directly. Verified in a rolled-back transaction before committing to this approach, then applied for real to stage: exact-count query 1467.9ms -> 3.6ms. Row-count parity confirmed (16,687 non-other + 22 other = 16,709 active total). Pure perf change, zero semantic difference, no code changes needed. Co-Authored-By: Sadin Shrestha <sadin@zunkireelabs.com> * fix(migrations): renumber 200 -> 201, resolve collision with #366 PR #366 (feature/classes-managers-fees-lockdown, Anish's work) already owns 200_class_managers.sql and it's already applied to the stage ledger (2026-08-06). `ls supabase/migrations | sort | tail` alone isn't enough to pick a number — it misses unmerged PRs' migration files. Verified 201 is free across the repo, every open PR's migrations, and the stage ledger before using it. Renamed the file, updated its self-record INSERT to the new filename, filled in the stage-applied date, deleted the stale 200_leads_tags_other_partial_index.sql ledger row on stage (only that row — 200_class_managers.sql and the index itself untouched), and re-ran scripts/migrate-apply.sh to confirm 201 applies as a true no-op (CREATE INDEX CONCURRENTLY IF NOT EXISTS skips since the index already exists). Co-Authored-By: Sadin Shrestha <sadin@zunkireelabs.com>
…368) Builds the single filter compiler that the four hand-maintained lead predicate mirrors (getLeads/getLeadsPage, the route.ts inline chain, lead_aggregates(), search-leads.ts) are meant to eventually route through. Pure TypeScript under src/lib/filters/ — no React, no DOM, no Supabase import, and zero consumers wired up yet; that's deliberate. - types.ts: the depth-2 FilterTree AST + FieldDef/FieldSource registry contract + CompileCtx - schema.ts: zod discriminated-union-on-op validation, with the URL-size defense caps (list values capped, 25-conditions-total, is_any_of [] is a 422 not a silent no-op) - operators.ts: the operator x field-type allow-list - serialize.ts: base64url encode/decode with a 4096-char cap - pgrst.ts: the security-critical escaping/quoting layer (pgVal/pgLike/ pgCol) — properly escapes rather than deleting characters, fixing a live bug in route.ts's `search.replace(/[,().]/g, "")` - compile.ts: compileFilter(builder, tree, registry, ctx) -> builder. Never calls .from()/.select()/.rpc() — tenant scoping and the leads_visible_to_user RPC stay owned by the caller. Native builder calls for the pure-AND fast path, falling back to constructed .or() strings for negation (every negative op includes the NULL-inclusive leg, so "status is not X" doesn't silently exclude unset rows), promoted dual-read fields (De Morgan across the real column and the legacy custom_fields leg), embedded relations, and tz-aware date math (day boundaries computed from ctx.tz using an injected ctx.now, correct across a DST transition day) - legacy-leads-params.ts: converts the ~9 existing toolbar params into a tree, excluding every scope param (list/funnel/stage/branch_id/ assigned_to/etc.) so the pipeline allow-list keeps failing closed 149 vitest tests (compile/pgrst/serialize/legacy-leads-params), all pure-function, environment: node.
… leads route (#369) Kills mirror #2 (docs/ADVANCED-FILTERS-BRIEF.md): the ~145-line inline .eq/.in/.or/.contains/.gte chain in GET /api/v1/leads is replaced by a single compileFilter() call. Legacy toolbar params route through the same tree via legacyLeadsParamsToTree() (Phase 1), so the existing route.test.ts suite acts as a full-fidelity regression harness against real production semantics. - planFilter(tree, registry, ctx) added to src/lib/filters/compile.ts — Phase 1 omitted it. Validates every condition up front (all errors in one 422) and returns the embed selects needed before .select() runs. - src/lib/filters/registry/leads.ts — the lead field registry: the 9 legacy toolbar axes, field_of_study/destinations (promoted dual-read), collaborators (embed), first-class sortable columns (folds the old SORT_COLUMNS map in), and 3 explicitly not-filterable stubs. - No visible surface — ?f= has no UI until Phase 3. This is a behavior-preserving refactor of the existing toolbar params. Two documented, deliberate simplifications from the brief's literal registry wording (both explained in leads.ts's doc comment): "status" and "source" are virtual fields that dispatch by value shape (UUID -> stage_id/form_config_id, else -> status/intake_source) rather than doing a real per-row coalesce — legacy toolbar values are never UUID-shaped, so this is a no-op for every existing caller and preserves byte-identical behavior. Co-authored-by: Anish Balami <anishbalami38@gmail.com>
…totals (#366) * feat(classes): admin-managed per-capability access + owner-only fees totals Replaces two hardcoded classes-access mechanisms (class_attendance_markers allowlist, CLASS_ENROLL_POSITIONS slug list) with a single class_managers table (enroll_students/mark_attendance/view_roster per user), managed via new Settings > Academic Operations > Class Managers UI. Existing markers (Purnima, Kamana, Pratima) backfilled with their prior grants only. Also hides total/aggregate fees-collected figures (stat card + per-class %) from everyone except role === "owner"; per-student fee stays visible to anyone with roster access. Co-Authored-By: Anish Balami <anishbalami38@gmail.com> * fix(classes): close review blockers - fees leak + enroll-access regression - Move fees-collected total + per-class % computation server-side (page.tsx), gated to role === "owner"; non-owners no longer get a precomputed aggregate handed to them via props (per-student fee stays visible as required — that requirement inherently limits how far this can go, since summing visible per-student amounts still yields the total for anyone determined to do the arithmetic). - Widen migration 200's backfill: the old CLASS_ENROLL_POSITIONS check granted enroll access to any user in branch-manager/lead-executive/ counselor/application-executive positions, not just the 3 attendance markers. Backfill now grants enroll_students=true to that same population so cutover doesn't silently revoke existing access. - Fix audit log always recording old:null on grant updates (fetch prior row before upsert). - Fix settings UI toggle race: revert-on-failure now touches only the toggled field, not the whole row snapshot, so two rapid toggles on the same user don't clobber each other. Co-Authored-By: Anish Balami <anishbalami38@gmail.com> * chore: retrigger CI Previous CI run stuck queued against a stale SHA after the follow-up fix commit didn't trigger a new run. Co-Authored-By: Anish Balami <anishbalami38@gmail.com> --------- Co-authored-by: Anish Balami <anishbalami38@gmail.com>
📝 WalkthroughWalkthroughThis PR adds a shared advanced-filter system for leads, including validation, serialization, compilation, registry-based field handling, and route integration. It also adds tenant-scoped class-manager permissions, administrative controls, updated authorization checks, and owner-only fee visibility. ChangesAdvanced lead filtering
Tenant-scoped class management
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant LeadsClient
participant LeadsRoute
participant FilterPlanner
participant QueryBuilder
LeadsClient->>LeadsRoute: Send encoded or legacy lead filters
LeadsRoute->>FilterPlanner: Validate tree and plan embeds
FilterPlanner-->>LeadsRoute: Return filter plan
LeadsRoute->>QueryBuilder: Compile predicates and execute query
QueryBuilder-->>LeadsRoute: Return filtered leads
LeadsRoute-->>LeadsClient: Return lead results and facet data
sequenceDiagram
participant Admin
participant ClassManagersUI
participant ClassManagersAPI
participant ClassManagersTable
Admin->>ClassManagersUI: Change a manager capability
ClassManagersUI->>ClassManagersAPI: Submit grant update
ClassManagersAPI->>ClassManagersTable: Verify membership and upsert grant
ClassManagersTable-->>ClassManagersAPI: Return updated grant
ClassManagersAPI-->>ClassManagersUI: Return success or failure
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (9)
src/lib/filters/registry/leads.ts (2)
230-240: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename the
first_namekey to match its multi-column meaning.This
FieldDefuses keyfirst_name, label"Name", and acolumnssource over["first_name", "last_name"]. A?f=author who writesfield: "first_name"gets a predicate that also matcheslast_name. A key such asnamestates the behavior and leavesfirst_namefree for a true single-column field later. The registry has no consumers outside this PR yet, so renaming is cheap now and breaking after the UI ships.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/filters/registry/leads.ts` around lines 230 - 240, Rename the FieldDef key in the multi-column name entry from "first_name" to "name", while preserving its label, columns source, and sorting configuration. Ensure any references within this registry to the renamed key are updated consistently.
83-92: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse
pgLikefor the location filter patterns.
compileLocationcurrently builds contain patterns with an inlinedpgVal/escape expression, while compile.ts delegates the samecontainsshape topgLike(String(value), "contains"). ImportpgLikefrom../pgrstand use it here so the location predicate and othercontainsfilters use the same escaping behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/filters/registry/leads.ts` around lines 83 - 92, Update compileLocation to import and reuse pgLike from ../pgrst for its contains pattern instead of the inline pgVal and escaping expression. Preserve the existing city/country predicate construction and not_contains handling while applying the same contains escaping behavior as compile.ts.src/lib/filters/registry/leads.test.ts (1)
12-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated
FakeBuilderinto a shared test helper.This class is identical to the
FakeBuilderinsrc/lib/filters/compile.test.tsLine 11-60, except for theorPayloads()helper. Two copies will drift asQueryBuildergains methods, and a missing method surfaces as a confusing runtime failure rather than a type error in one place. Move it to a shared module, for examplesrc/lib/filters/__fixtures__/fake-builder.ts, and import it in both test files.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/filters/registry/leads.test.ts` around lines 12 - 57, Extract the duplicated FakeBuilder class from leads.test.ts and compile.test.ts into a shared test fixture module, preserving all QueryBuilder method implementations and the existing orPayloads() helper where needed. Import the shared FakeBuilder in both test files so QueryBuilder changes are enforced through one implementation.src/lib/filters/compile.ts (2)
260-274: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the module-scoped
currentCtxRefwith an explicitctxparameter.
renderAgainstColumnreadscurrentCtxRef, whichrenderConditionassigns immediately before dispatch. This works today because every path is synchronous and every entry point goes throughrenderCondition. It is still module-level mutable state that any future direct caller, or any future async virtualcompile, can read while stale.renderConditionalready receivesctx, so threading it throughrenderAgainstColumn,renderPromotedPredicate, andrenderColumnsPredicateis mechanical and removes the hazard.♻️ Proposed refactor sketch
-function renderAgainstColumn(col: string, field: FieldDef, cond: FilterCondition): string { +function renderAgainstColumn(col: string, field: FieldDef, cond: FilterCondition, ctx: CompileCtx): string { const isArrayColumn = field.source.kind === "array_column" || field.type === "tags" || field.type === "multiselect"; - if (field.type === "date") return renderDateOpAgainstColumn(col, cond.op, cond.value, currentCtxRef); + if (field.type === "date") return renderDateOpAgainstColumn(col, cond.op, cond.value, ctx);Then remove
let currentCtxRef: CompileCtx;and passctxdown fromrenderConditionintorenderConditionPredicate,renderPromotedPredicate, andrenderColumnsPredicate.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/filters/compile.ts` around lines 260 - 274, Replace the module-scoped currentCtxRef state with an explicit CompileCtx parameter. Thread ctx from renderCondition through renderConditionPredicate, renderPromotedPredicate, and renderColumnsPredicate into renderAgainstColumn, and use that parameter for date rendering; remove the assignment and currentCtxRef declaration while preserving existing synchronous behavior.
91-101: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueSingle-pass offset resolution can be off by one hour for zones whose DST transition happens at local midnight.
localMidnightUtccomputes the zone offset at the UTC-midnight instant, then applies it. For most zones this is correct, and the tests coverAmerica/New_Yorkwhere the transition happens at 02:00 local. For zones that transition at 00:00 local (for exampleAmerica/SantiagoorAsia/Beirut), the offset at the probe instant can differ from the offset at the real local midnight, sostart/endshift by one hour. The route currently pinstzto"UTC", so this is not reachable today, but Phase 3 per-tenant timezones would expose it.A second resolution pass removes the edge case.
♻️ Optional: re-resolve the offset at the candidate instant
function localMidnightUtc(dateStr: string, tz: string): Date { const datePart = dateStr.slice(0, 10); - // naiveUtc: the target date's midnight, misinterpreted as a UTC instant. const naiveUtc = new Date(`${datePart}T00:00:00.000Z`); - const asTzWallClock = naiveUtc.toLocaleString("sv-SE", { timeZone: tz }).replace(" ", "T") + "Z"; - const offsetMs = naiveUtc.getTime() - new Date(asTzWallClock).getTime(); - return new Date(naiveUtc.getTime() + offsetMs); + const offsetAt = (instant: Date): number => { + const wall = instant.toLocaleString("sv-SE", { timeZone: tz }).replace(" ", "T") + "Z"; + return instant.getTime() - new Date(wall).getTime(); + }; + // Resolve twice: the offset at the probe instant may differ from the offset + // at the real local midnight when a zone transitions at 00:00 local. + const first = new Date(naiveUtc.getTime() + offsetAt(naiveUtc)); + return new Date(naiveUtc.getTime() + offsetAt(first)); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/filters/compile.ts` around lines 91 - 101, Update localMidnightUtc to re-resolve the timezone offset using the candidate midnight instant after the initial offset calculation, then apply the corrected offset before returning the Date. Preserve the existing datePart parsing and DST-aware timezone formatting while ensuring transitions occurring at local midnight use the offset at the actual candidate instant.src/app/(main)/api/v1/leads/route.test.ts (1)
802-807: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe malformed-
?f=test does not exercise the base64url branch it names.
Buffer.from(value, "base64url")does not throw on invalid characters. It ignores them and decodes whatever remains."not-valid-base64url-json!!"therefore reachesJSON.parse, which fails, anddecodeFilterTreereturns the "not valid JSON after decoding" error. The test still asserts 422 correctly, but the title claims coverage of the base64url branch, which is unreachable through this input.Rename the test to describe JSON-decoding failure, and add a case for a valid-base64url payload that decodes to a non-conforming object, so the
filterTreeSchemarejection path is covered too.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/`(main)/api/v1/leads/route.test.ts around lines 802 - 807, Rename the existing malformed ?f= test to describe failure when decoded content is not valid JSON, since its input reaches JSON.parse rather than base64url validation. Add a separate test using valid base64url encoding of a JSON object that violates filterTreeSchema, and assert that GET returns 422 to cover the schema-rejection path in decodeFilterTree.src/lib/filters/compile.test.ts (1)
605-621: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd the matching
compileFiltercase for avisibleTo-denied field.This test proves
planFilterdenies a gated field. There is no equivalent test forcompileFilter, andcompileFilterdoes not perform the check. Add a test that assertscompileFilterthrows forgated, together with theresolveAndValidatefix requested onsrc/lib/filters/compile.tsLine 379-392. The test then locks the fail-closed behavior for callers that skipplanFilter.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/filters/compile.test.ts` around lines 605 - 621, Add a matching test near the existing gated-field test that invokes compileFilter with the visibleTo-denied “gated” field and asserts it throws. Update resolveAndValidate in compile.ts to enforce visibleTo access validation so compileFilter fails closed even when planFilter is skipped, preserving the existing error context for inaccessible fields.src/app/(main)/api/v1/leads/route.ts (2)
346-347: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winDerive the collaborator-embed flag from a shared constant, not a duplicated literal.
hasCollaboratorsEmbedcompares against the literal"lead_collaborators!inner(user_id)". The same string is defined insrc/lib/filters/registry/leads.tsLine 174 as the field'sembedSelect. If the registry adds a column to that embed, this exact-match check silently returns false, the strip block at Line 560 is skipped, and the rawlead_collaboratorsarray leaks into every row of the API response. The failure is silent and only shows up in the client payload.Export the embed string once and reference it from both files, or match on the relation prefix.
♻️ Proposed fix
- const hasCollaboratorsEmbed = filterPlan.embeds.includes("lead_collaborators!inner(user_id)"); + const hasCollaboratorsEmbed = filterPlan.embeds.some((e) => e.startsWith("lead_collaborators"));A shared exported constant, for example
LEAD_COLLABORATORS_EMBEDinsrc/lib/filters/registry/leads.ts, is the stronger option because it also keeps the test assertion insrc/app/(main)/api/v1/leads/route.test.tsLine 799 in sync.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/`(main)/api/v1/leads/route.ts around lines 346 - 347, Define and export a shared collaborator embed constant in the leads filter registry, using it for the registry field’s embedSelect value and the route’s hasCollaboratorsEmbed check. Replace the duplicated literal in the leads route (and update related test references if needed) so changes to the embed selection remain synchronized and the collaborator stripping logic continues to run.
201-206: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe double cast is stronger than the type bridge needs.
src/lib/filters/types.tsLine 106-109 declaresResolvedPermissionsas{ leadScope?: string; [key: string]: unknown }. That index signature already acceptsauth.permissionsstructurally in most shapes, soas unknown asis likely unnecessary. The double cast also suppresses any future incompatibility between the two types instead of surfacing it at compile time. Trypermissions: auth.permissionsfirst, and fall back to a singleas FilterResolvedPermissionsiftscobjects.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/`(main)/api/v1/leads/route.ts around lines 201 - 206, Replace the double cast on permissions in the registry object with direct assignment from auth.permissions; if TypeScript rejects the structural assignment, use a single as FilterResolvedPermissions cast instead. Preserve the existing FilterResolvedPermissions bridge without using unknown, so future incompatibilities remain visible to the compiler.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/ADVANCED-FILTERS-BRIEF.md`:
- Line 102: Update the fenced code block at the documented file-layout section
in ADVANCED-FILTERS-BRIEF.md to include the text language identifier, changing
the opening fence to ```text while preserving the block contents.
In `@src/app/`(main)/(dashboard)/classes/page.tsx:
- Around line 138-155: Update the enrollment aggregation loop before calculating
class fee percentages to deduplicate rows by (class_id, lead_id), preferring the
actual enrollment over any demo enrollment for the same student and class. Then
compute total fees, paid counts, and payable counts from only the selected
enrollment records while preserving the existing inactive-status and classFeePct
behavior.
In `@src/app/`(main)/api/v1/class-managers/route.ts:
- Around line 116-129: Prevent stale capability snapshots from overwriting
grants: in src/app/(main)/api/v1/class-managers/route.ts lines 116-129, update
the PATCH flow to accept field-level mutations or enforce a revision check
before replacing all capabilities; in
src/components/dashboard/settings/class-managers.tsx lines 65-70, fail loading
when either response is not OK instead of substituting an empty grant list; in
lines 101-138, serialize per-user mutations or disable all three controls during
saves and refresh state from the server response.
In `@src/app/`(main)/api/v1/leads/route.test.ts:
- Around line 745-785: Update the three equivalence tests for tag, industry, and
assignees to capture both GET responses and assert each response has status 200
before comparing query-call arrays. Keep the existing byte-identical call
assertions and native query-operation checks unchanged.
In `@src/app/`(main)/api/v1/leads/route.ts:
- Around line 451-461: Update all successful facets=source responses in the
source-facet handling flow, including the empty-pipeline and legacy RPC paths
near the rawFilterParam branch, to always include counts. Use the appropriate
counted value for each existing path while preserving the ?f= response’s counts:
null behavior, so clients receive a consistent counts field without changing
filtering behavior.
- Around line 418-428: Wrap the compileFilter call in the GET handler with the
route’s existing validation-error handling, catch FilterCompileError, and return
the same 422 response shape used for other invalid filter inputs. Keep
successful query compilation and unrelated errors unchanged.
In `@src/components/dashboard/settings/class-managers.tsx`:
- Around line 52-233: The ClassManagers feature is education-consultancy-only
and must be moved out of the shared dashboard settings directory. Move the
ClassManagers component into
src/industries/education-consultancy/features/class-managers/, preserving its
behavior, and update the import in
src/components/dashboard/settings/modal/panels/academic-operations-panel.tsx at
line 5 to reference the new module.
In `@src/lib/filters/compile.ts`:
- Around line 379-392: Update src/lib/filters/compile.ts lines 379-392: change
resolveAndValidate to accept the filter context, reject fields when
visibleTo(ctx.permissions) is false, and thread ctx through
applyConditionToBuilder and applyOrConditions. Add a neighboring compileFilter
test in src/lib/filters/compile.test.ts lines 605-621 asserting that the gated
field raises FilterCompileError, alongside the existing planFilter coverage.
- Around line 224-227: Update compileFilter’s date handling for “before”,
“after”, “on”, and date_between to validate parsed dates before calling
toISOString or dayBoundsInTz. Catch invalid-date parsing failures and throw
FilterCompileError, including the applicable bounds for date_between, so invalid
filter values produce the established validation response instead of RangeError.
In `@src/lib/filters/pgrst.ts`:
- Around line 29-37: Update pgLike to escape PostgREST’s literal * wildcard
alongside \, %, and _ before applying the mode-specific pattern. Add regression
coverage verifying literal * handling for contains, prefix, suffix, and exact
LikeMode values.
In `@src/lib/filters/registry/leads.ts`:
- Around line 70-79: Update compileAssignees to fail closed when no valid UUID
or "unassigned" token is present, matching compileSource instead of returning
the always-true id.not.is.null predicate; preserve valid assigned and unassigned
combinations. Update the affected leads tests, including the assertion around
the current tautology, and if legacy no-op behavior must remain byte-for-byte,
implement the dropping behavior in legacyLeadsParamsToTree instead.
In `@supabase/migrations/201_leads_tags_other_partial_index.sql`:
- Around line 6-10: Choose and implement one supported migration strategy for
this file: replace the concurrent index creation with ordinary CREATE INDEX so
the DDL and schema_migrations self-record remain transactional, or update
scripts/check-migrations.sh and the deployment SOP to explicitly support
non-transactional CONCURRENTLY execution, including invalid-index recovery.
Ensure the rollback statement and migration comments match the selected policy.
---
Nitpick comments:
In `@src/app/`(main)/api/v1/leads/route.test.ts:
- Around line 802-807: Rename the existing malformed ?f= test to describe
failure when decoded content is not valid JSON, since its input reaches
JSON.parse rather than base64url validation. Add a separate test using valid
base64url encoding of a JSON object that violates filterTreeSchema, and assert
that GET returns 422 to cover the schema-rejection path in decodeFilterTree.
In `@src/app/`(main)/api/v1/leads/route.ts:
- Around line 346-347: Define and export a shared collaborator embed constant in
the leads filter registry, using it for the registry field’s embedSelect value
and the route’s hasCollaboratorsEmbed check. Replace the duplicated literal in
the leads route (and update related test references if needed) so changes to the
embed selection remain synchronized and the collaborator stripping logic
continues to run.
- Around line 201-206: Replace the double cast on permissions in the registry
object with direct assignment from auth.permissions; if TypeScript rejects the
structural assignment, use a single as FilterResolvedPermissions cast instead.
Preserve the existing FilterResolvedPermissions bridge without using unknown, so
future incompatibilities remain visible to the compiler.
In `@src/lib/filters/compile.test.ts`:
- Around line 605-621: Add a matching test near the existing gated-field test
that invokes compileFilter with the visibleTo-denied “gated” field and asserts
it throws. Update resolveAndValidate in compile.ts to enforce visibleTo access
validation so compileFilter fails closed even when planFilter is skipped,
preserving the existing error context for inaccessible fields.
In `@src/lib/filters/compile.ts`:
- Around line 260-274: Replace the module-scoped currentCtxRef state with an
explicit CompileCtx parameter. Thread ctx from renderCondition through
renderConditionPredicate, renderPromotedPredicate, and renderColumnsPredicate
into renderAgainstColumn, and use that parameter for date rendering; remove the
assignment and currentCtxRef declaration while preserving existing synchronous
behavior.
- Around line 91-101: Update localMidnightUtc to re-resolve the timezone offset
using the candidate midnight instant after the initial offset calculation, then
apply the corrected offset before returning the Date. Preserve the existing
datePart parsing and DST-aware timezone formatting while ensuring transitions
occurring at local midnight use the offset at the actual candidate instant.
In `@src/lib/filters/registry/leads.test.ts`:
- Around line 12-57: Extract the duplicated FakeBuilder class from leads.test.ts
and compile.test.ts into a shared test fixture module, preserving all
QueryBuilder method implementations and the existing orPayloads() helper where
needed. Import the shared FakeBuilder in both test files so QueryBuilder changes
are enforced through one implementation.
In `@src/lib/filters/registry/leads.ts`:
- Around line 230-240: Rename the FieldDef key in the multi-column name entry
from "first_name" to "name", while preserving its label, columns source, and
sorting configuration. Ensure any references within this registry to the renamed
key are updated consistently.
- Around line 83-92: Update compileLocation to import and reuse pgLike from
../pgrst for its contains pattern instead of the inline pgVal and escaping
expression. Preserve the existing city/country predicate construction and
not_contains handling while applying the same contains escaping behavior as
compile.ts.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0f7c1f8e-31b3-4965-8683-415b40856116
📒 Files selected for processing (30)
docs/ADVANCED-FILTERS-BRIEF.mdsrc/app/(main)/(dashboard)/classes/page.tsxsrc/app/(main)/(dashboard)/leads/[id]/page.tsxsrc/app/(main)/api/v1/class-enrollments/[id]/route.tssrc/app/(main)/api/v1/class-enrollments/route.tssrc/app/(main)/api/v1/class-managers/route.tssrc/app/(main)/api/v1/leads/[id]/classes/route.tssrc/app/(main)/api/v1/leads/route.test.tssrc/app/(main)/api/v1/leads/route.tssrc/components/dashboard/settings/class-managers.tsxsrc/components/dashboard/settings/modal/panels/academic-operations-panel.tsxsrc/industries/education-consultancy/features/classes/pages/classes-workspace.tsxsrc/lib/api/class-attendance.tssrc/lib/api/permissions.tssrc/lib/filters/compile.test.tssrc/lib/filters/compile.tssrc/lib/filters/legacy-leads-params.test.tssrc/lib/filters/legacy-leads-params.tssrc/lib/filters/operators.tssrc/lib/filters/pgrst.test.tssrc/lib/filters/pgrst.tssrc/lib/filters/registry/index.tssrc/lib/filters/registry/leads.test.tssrc/lib/filters/registry/leads.tssrc/lib/filters/schema.tssrc/lib/filters/serialize.test.tssrc/lib/filters/serialize.tssrc/lib/filters/types.tssupabase/migrations/200_class_managers.sqlsupabase/migrations/201_leads_tags_other_partial_index.sql
|
|
||
| ## Files to create — `src/lib/filters/` | ||
|
|
||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a language identifier to the fenced block.
Line 102 fails MD040. Use text because the block documents a file layout.
Proposed fix
-```
+```text📝 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.
| ``` |
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 102-102: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/ADVANCED-FILTERS-BRIEF.md` at line 102, Update the fenced code block at
the documented file-layout section in ADVANCED-FILTERS-BRIEF.md to include the
text language identifier, changing the opening fence to ```text while preserving
the block contents.
Source: Linters/SAST tools
| for (const e of enrollments) { | ||
| const feePaid = e.fee_paid as boolean; | ||
| const feeAmount = e.fee_amount as number | null; | ||
| const status = e.status as string; | ||
| const classId = e.class_id as string; | ||
| if (feePaid && feeAmount != null) total += feeAmount; | ||
| if (status !== "inactive") { | ||
| const entry = byClass[classId] ?? { paid: 0, payable: 0 }; | ||
| entry.payable += 1; | ||
| if (feePaid) entry.paid += 1; | ||
| byClass[classId] = entry; | ||
| } | ||
| } | ||
| feesCollected = total; | ||
| classFeePct = {}; | ||
| for (const cls of classes) { | ||
| const entry = byClass[cls.id]; | ||
| classFeePct[cls.id] = entry && entry.payable > 0 ? Math.round((entry.paid / entry.payable) * 100) : null; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Calculate fee percentage per student.
A lead can have both demo and actual enrollment rows. This loop counts both rows in paid and payable, but the workspace displays student counts and treats actual enrollment as primary. The fee percentage can therefore be incorrect.
Deduplicate by (class_id, lead_id) and prefer the actual enrollment before calculating paid and payable.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/`(main)/(dashboard)/classes/page.tsx around lines 138 - 155, Update
the enrollment aggregation loop before calculating class fee percentages to
deduplicate rows by (class_id, lead_id), preferring the actual enrollment over
any demo enrollment for the same student and class. Then compute total fees,
paid counts, and payable counts from only the selected enrollment records while
preserving the existing inactive-status and classFeePct behavior.
| const { data: upserted, error } = await db | ||
| .from("class_managers") | ||
| .upsert( | ||
| { | ||
| user_id: userId, | ||
| enroll_students: enrollStudents, | ||
| mark_attendance: markAttendance, | ||
| view_roster: viewRoster, | ||
| granted_by: auth.userId, | ||
| }, | ||
| { onConflict: "tenant_id,user_id" } | ||
| ) | ||
| .select("*") | ||
| .single(); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Prevent stale capability snapshots from overwriting grants.
The PATCH handler treats all three capability values as an authoritative snapshot. The UI can send false defaults after a failed grants request. Two field toggles can also complete out of order because savingKey locks only one control. A late request can re-grant or revoke another capability.
src/app/(main)/api/v1/class-managers/route.ts#L116-L129: accept a field-level mutation or enforce a revision check before replacing all capability values.src/components/dashboard/settings/class-managers.tsx#L65-L70: fail the load when either response is not OK. Do not replace failed grant data with an empty list.src/components/dashboard/settings/class-managers.tsx#L101-L138: serialize mutations per user, or lock all three controls until the request completes and refresh from the server response.
📍 Affects 2 files
src/app/(main)/api/v1/class-managers/route.ts#L116-L129(this comment)src/components/dashboard/settings/class-managers.tsx#L65-L70src/components/dashboard/settings/class-managers.tsx#L101-L138
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/`(main)/api/v1/class-managers/route.ts around lines 116 - 129,
Prevent stale capability snapshots from overwriting grants: in
src/app/(main)/api/v1/class-managers/route.ts lines 116-129, update the PATCH
flow to accept field-level mutations or enforce a revision check before
replacing all capabilities; in
src/components/dashboard/settings/class-managers.tsx lines 65-70, fail loading
when either response is not OK instead of substituting an empty grant list; in
lines 101-138, serialize per-user mutations or disable all three controls during
saves and refresh state from the server response.
| it("?tag=vip and its equivalent ?f= tree produce byte-identical query calls (native .contains() path)", async () => { | ||
| const legacyCalls: Call[] = []; | ||
| createServiceClientMock.mockResolvedValue(fakeDb({ leadsCalls: legacyCalls })); | ||
| const { GET } = await import("./route"); | ||
| await GET(fakeReq({ tag: "vip" })); | ||
|
|
||
| const fCalls: Call[] = []; | ||
| createServiceClientMock.mockResolvedValue(fakeDb({ leadsCalls: fCalls })); | ||
| await GET(fakeReq({ [FILTER_PARAM]: encodedTreeFor({ tag: "vip" }) })); | ||
|
|
||
| expect(fCalls).toEqual(legacyCalls); | ||
| expect(legacyCalls).toContainEqual(["contains", ["tags", ["vip"]]]); | ||
| }); | ||
|
|
||
| it("?industry=__none__ and its equivalent ?f= tree produce byte-identical query calls (native .is(null) path)", async () => { | ||
| const legacyCalls: Call[] = []; | ||
| createServiceClientMock.mockResolvedValue(fakeDb({ leadsCalls: legacyCalls })); | ||
| const { GET } = await import("./route"); | ||
| await GET(fakeReq({ industry: "__none__" })); | ||
|
|
||
| const fCalls: Call[] = []; | ||
| createServiceClientMock.mockResolvedValue(fakeDb({ leadsCalls: fCalls })); | ||
| await GET(fakeReq({ [FILTER_PARAM]: encodedTreeFor({ industry: "__none__" }) })); | ||
|
|
||
| expect(fCalls).toEqual(legacyCalls); | ||
| expect(legacyCalls).toContainEqual(["is", ["prospect_industry", null]]); | ||
| }); | ||
|
|
||
| it("?assignees=unassigned,<uuid> and its equivalent ?f= tree produce byte-identical query calls", async () => { | ||
| const uuid = "11111111-2222-4333-8444-555555555555"; | ||
| const legacyCalls: Call[] = []; | ||
| createServiceClientMock.mockResolvedValue(fakeDb({ leadsCalls: legacyCalls })); | ||
| const { GET } = await import("./route"); | ||
| await GET(fakeReq({ assignees: `unassigned,${uuid}` })); | ||
|
|
||
| const fCalls: Call[] = []; | ||
| createServiceClientMock.mockResolvedValue(fakeDb({ leadsCalls: fCalls })); | ||
| await GET(fakeReq({ [FILTER_PARAM]: encodedTreeFor({ assignees: `unassigned,${uuid}` }) })); | ||
|
|
||
| expect(fCalls).toEqual(legacyCalls); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Assert the response status in every equivalence test.
These three tests compare fCalls to legacyCalls without checking that either request succeeded. If both requests fail early — for example a 422 from planFilter or a 500 from a throw — both call arrays stay empty and expect(fCalls).toEqual(legacyCalls) passes with no query ever built. The equivalence harness then reports green while proving nothing. The first test on Line 735 and Line 740 and the collaborators test on Line 797 already assert status === 200.
💚 Proposed fix for the tag test; apply the same change to the industry and assignees tests
const { GET } = await import("./route");
- await GET(fakeReq({ tag: "vip" }));
+ const legacyRes = await GET(fakeReq({ tag: "vip" }));
+ expect(legacyRes.status).toBe(200);
const fCalls: Call[] = [];
createServiceClientMock.mockResolvedValue(fakeDb({ leadsCalls: fCalls }));
- await GET(fakeReq({ [FILTER_PARAM]: encodedTreeFor({ tag: "vip" }) }));
+ const fRes = await GET(fakeReq({ [FILTER_PARAM]: encodedTreeFor({ tag: "vip" }) }));
+ expect(fRes.status).toBe(200);📝 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.
| it("?tag=vip and its equivalent ?f= tree produce byte-identical query calls (native .contains() path)", async () => { | |
| const legacyCalls: Call[] = []; | |
| createServiceClientMock.mockResolvedValue(fakeDb({ leadsCalls: legacyCalls })); | |
| const { GET } = await import("./route"); | |
| await GET(fakeReq({ tag: "vip" })); | |
| const fCalls: Call[] = []; | |
| createServiceClientMock.mockResolvedValue(fakeDb({ leadsCalls: fCalls })); | |
| await GET(fakeReq({ [FILTER_PARAM]: encodedTreeFor({ tag: "vip" }) })); | |
| expect(fCalls).toEqual(legacyCalls); | |
| expect(legacyCalls).toContainEqual(["contains", ["tags", ["vip"]]]); | |
| }); | |
| it("?industry=__none__ and its equivalent ?f= tree produce byte-identical query calls (native .is(null) path)", async () => { | |
| const legacyCalls: Call[] = []; | |
| createServiceClientMock.mockResolvedValue(fakeDb({ leadsCalls: legacyCalls })); | |
| const { GET } = await import("./route"); | |
| await GET(fakeReq({ industry: "__none__" })); | |
| const fCalls: Call[] = []; | |
| createServiceClientMock.mockResolvedValue(fakeDb({ leadsCalls: fCalls })); | |
| await GET(fakeReq({ [FILTER_PARAM]: encodedTreeFor({ industry: "__none__" }) })); | |
| expect(fCalls).toEqual(legacyCalls); | |
| expect(legacyCalls).toContainEqual(["is", ["prospect_industry", null]]); | |
| }); | |
| it("?assignees=unassigned,<uuid> and its equivalent ?f= tree produce byte-identical query calls", async () => { | |
| const uuid = "11111111-2222-4333-8444-555555555555"; | |
| const legacyCalls: Call[] = []; | |
| createServiceClientMock.mockResolvedValue(fakeDb({ leadsCalls: legacyCalls })); | |
| const { GET } = await import("./route"); | |
| await GET(fakeReq({ assignees: `unassigned,${uuid}` })); | |
| const fCalls: Call[] = []; | |
| createServiceClientMock.mockResolvedValue(fakeDb({ leadsCalls: fCalls })); | |
| await GET(fakeReq({ [FILTER_PARAM]: encodedTreeFor({ assignees: `unassigned,${uuid}` }) })); | |
| expect(fCalls).toEqual(legacyCalls); | |
| }); | |
| it("?tag=vip and its equivalent ?f= tree produce byte-identical query calls (native .contains() path)", async () => { | |
| const legacyCalls: Call[] = []; | |
| createServiceClientMock.mockResolvedValue(fakeDb({ leadsCalls: legacyCalls })); | |
| const { GET } = await import("./route"); | |
| const legacyRes = await GET(fakeReq({ tag: "vip" })); | |
| expect(legacyRes.status).toBe(200); | |
| const fCalls: Call[] = []; | |
| createServiceClientMock.mockResolvedValue(fakeDb({ leadsCalls: fCalls })); | |
| const fRes = await GET(fakeReq({ [FILTER_PARAM]: encodedTreeFor({ tag: "vip" }) })); | |
| expect(fRes.status).toBe(200); | |
| expect(fCalls).toEqual(legacyCalls); | |
| expect(legacyCalls).toContainEqual(["contains", ["tags", ["vip"]]]); | |
| }); | |
| it("?industry=__none__ and its equivalent ?f= tree produce byte-identical query calls (native .is(null) path)", async () => { | |
| const legacyCalls: Call[] = []; | |
| createServiceClientMock.mockResolvedValue(fakeDb({ leadsCalls: legacyCalls })); | |
| const { GET } = await import("./route"); | |
| const legacyRes = await GET(fakeReq({ industry: "__none__" })); | |
| expect(legacyRes.status).toBe(200); | |
| const fCalls: Call[] = []; | |
| createServiceClientMock.mockResolvedValue(fakeDb({ leadsCalls: fCalls })); | |
| const fRes = await GET(fakeReq({ [FILTER_PARAM]: encodedTreeFor({ industry: "__none__" }) })); | |
| expect(fRes.status).toBe(200); | |
| expect(fCalls).toEqual(legacyCalls); | |
| expect(legacyCalls).toContainEqual(["is", ["prospect_industry", null]]); | |
| }); | |
| it("?assignees=unassigned,<uuid> and its equivalent ?f= tree produce byte-identical query calls", async () => { | |
| const uuid = "11111111-2222-4333-8444-555555555555"; | |
| const legacyCalls: Call[] = []; | |
| createServiceClientMock.mockResolvedValue(fakeDb({ leadsCalls: legacyCalls })); | |
| const { GET } = await import("./route"); | |
| const legacyRes = await GET(fakeReq({ assignees: `unassigned,${uuid}` })); | |
| expect(legacyRes.status).toBe(200); | |
| const fCalls: Call[] = []; | |
| createServiceClientMock.mockResolvedValue(fakeDb({ leadsCalls: fCalls })); | |
| const fRes = await GET(fakeReq({ [FILTER_PARAM]: encodedTreeFor({ assignees: `unassigned,${uuid}` }) })); | |
| expect(fRes.status).toBe(200); | |
| expect(fCalls).toEqual(legacyCalls); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/`(main)/api/v1/leads/route.test.ts around lines 745 - 785, Update the
three equivalence tests for tag, industry, and assignees to capture both GET
responses and assert each response has status 200 before comparing query-call
arrays. Keep the existing byte-identical call assertions and native
query-operation checks unchanged.
| // ADVANCED-FILTERS-BRIEF Phase 2: every toolbar filter that used to be a | ||
| // hand-written .eq/.in/.or/.contains/.gte chain here (status, search, form, | ||
| // assignees, collaborators, source, tag, industry, created) now compiles | ||
| // through the SAME compileFilter() call whether it came from ?f= or from | ||
| // the legacy params via legacyLeadsParamsToTree — see filterTree/filterPlan | ||
| // above. compileFilter never touches .select()/.from()/.rpc() (see its | ||
| // module doc comment), so it's safe to call on `query` at this point | ||
| // regardless of which branch (visibleLeadsBase RPC vs plain service query) | ||
| // built it. `stage`/`list`/`funnel`/branch/pipeline/shared-pool SCOPE | ||
| // filters above and below this call are deliberately untouched. | ||
| query = compileFilter(query, filterTree, filterRegistry, compileCtx); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether the schema requires a value per operator.
fd -t f 'schema.ts' src/lib/filters --exec cat -n {}Repository: Zunkireelabs/edgexcrm
Length of output: 4735
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate route and filter files =="
fd -t f '^route\.ts$' src/app | rg '(^|/)src/app/\(main\)/api/v1/leads/route\.ts$' || true
fd -t f 'compile\.ts|registry|validation\.ts|index\.ts' src/lib filters | sort
echo "== inspect route relevant sections =="
sed -n '1,90p' src/app/\(main\)/api/v1/leads/route.ts
sed -n '380,460p' src/app/\(main\)/api/v1/leads/route.ts
echo "== filter compile registry snippets =="
fd -t f 'compile\.ts|leads\.ts|index\.ts' src/lib/filters --exec sh -c 'echo "--- $1"; wc -l "$1"; sed -n "1,140p" "$1" | cat -n' sh {}
fd -t f 'leads\.ts' src/lib/filters --exec sh -c 'echo "--- $1"; wc -l "$1"; sed -n "40,90p" "$1" | cat -n' sh {}
echo "== search planFilter and compileFilter definitions/usages =="
rg -n "function planFilter|const planFilter|planFilter|function compileFilter|const compileFilter|export.*compileFilter|FilterCompileError" src/lib/filters src/app/\(main\)/api/v1/leads/route.ts -SRepository: Zunkireelabs/edgexcrm
Length of output: 37486
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== route filter handling around planFilter/getLeads endpoint =="
sed -n '150,255p' src/app/\(main\)/api/v1/leads/route.ts
sed -n '555,680p' src/app/\(main\)/api/v1/leads/route.ts
echo "== compile.planFilter implementation =="
sed -n '500,640p' src/lib/filters/compile.ts
echo "== compiled date and source compiler values =="
sed -n '220,265p' src/lib/filters/compile.tsRepository: Zunkireelabs/edgexcrm
Length of output: 16406
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
# Read-only behavioral model of the route/schema path relevant to the concern.
from pathlib import Path
import re
route = Path("src/app/(main)/api/v1/leads/route.ts").read_text()
schema = Path("src/lib/filters/schema.ts").read_text()
plan = Path("src/lib/filters/compile.ts").read_text()
compile_line = re.search(r".*\bcompileFilter\s*\(\s*query\s*,", route, re.S)
plan_before_compile = route[:compile_line.start()] if compile_line else ""
compile_inside_try = "try {" not in plan_before_compile or plan_before_compile.count("{") == plan_before_compile.count("}")
schema_value_condition = re.search(r".*function\s+condition<[^>]+>\([^)]*\)\s*\{\s*return\s+z\.object\(\{[^}]+\}\);", schema, re.S)
schema_no_value_condition = re.search(r".*function\s+conditionNoValue<[^>]+>\([^)]*\)\s*\{\s*return\s+z\.object\(\{[^}]+\}\);", schema, re.S)
plan_empty_array = re.search(r"\(cond\.op === \"is_any_of\" \|\| cond\.op === \"is_none_of\" \|\| cond\.op === \"has_all\"\)\s*\&&\s*Array\.isArray\(cond\.value\)\s*\&&\s*cond\.value\.length === 0", plan)
print("compile_filter_call:")
print(f" exists={bool(compile_line)}")
if compile_line:
snippet = route[max(0, compile_line.start()-400):compile_line.end()+300]
print(" snippet_context_no_try:", "try {" not in snippet)
print("compile_filter_in_route_try:", compile_inside_try)
print("schema_exact:")
for m in [re.search(r'condition\("is_any_of",\s*listValue', schema), re.search(r"conditionNoValue\(\"(is_any_of|is_none_of|has_all)\"", schema)]:
print(f" operator_is_any_of_requires_value={bool(m)}")
print("plan_filter_empty_array_guard:")
print(f" checks_array_and_length_0={bool(plan_empty_array)}")
# Model exact route branches: decode failure returns validation; otherwise planFilter runs.
# PlanFilter only rejects empty arrays for set operators, not "before" string values.
# compileFilter then date compiles "before" via new Date(value), which should throw for "garbage".
try:
new_date = new Date = lambda v: Date(v)
new Date("garbage")
failed = False
except Exception:
failed = True
print("date_garbage_throws_native:", failed)
PYRepository: Zunkireelabs/edgexcrm
Length of output: 278
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
import datetime
route = Path("src/app/(main)/api/v1/leads/route.ts").read_text()
schema = Path("src/lib/filters/schema.ts").read_text()
plan = Path("src/lib/filters/compile.ts").read_text()
compile_line = re.search(r".*\bcompileFilter\s*\(\s*query\s*,", route, re.S)
compile_inside_try = "try {" not in route[:compile_line.start()] if compile_line else True
schema_value_condition = re.search(r".*function\s+condition<[^>]+>\([^)]*\)\s*\{\s*return\s+z\.object\(\{[^}]+\}\);", schema, re.S)
schema_no_value_condition = re.search(r".*function\s+conditionNoValue<[^>]+>\([^)]*\)\s*\{\s*return\s+z\.object\(\{[^}]+\}\);", schema, re.S)
plan_empty_array = re.search(r"\(cond\.op === \"is_any_of\" \|\| cond\.op === \"is_none_of\" \|\| cond\.op === \"has_all\"\)\s*\&&\s*Array\.isArray\(cond\.value\)\s*\&&\s*cond\.value\.length === 0", plan)
print("compile_filter_call:")
print(" exists={}".format(bool(compile_line)))
if compile_line:
snippet = route[max(0, compile_line.start()-400):compile_line.end()+300]
print(" snippet_context_no_try={}".format("try {" not in snippet))
print("compile_filter_in_route_try={}".format(compile_inside_try))
print(compile_line.group(0))
print("schema_exact:")
print(" operator_is_any_of_requires_list_value={}".format(bool(re.search(r'condition\("is_any_of",\s*listValue', schema))))
print(" no-value_grammar_applies_to_is_any_of={}".format(bool(re.search(r"conditionNoValue\(\"is_any_of\"", schema))))
print("plan_filter_empty_array_guard_checks_array_and_length_0={}".format(bool(plan_empty_array)))
print("date_invalid_native_throws={}".format(not bool(datetime.datetime.isoformat(datetime.datetime.fromisoformat("garbage")))))
PYRepository: Zunkireelabs/edgexcrm
Length of output: 23911
Handle FilterCompileError from compileFilter as a 422.
filterTreeSchema requires values for is_any_of, before, and other value-taking operators, and planFilter rejects empty allow-lists. Any remaining FilterCompileError still escapes the GET handler and becomes a 500. Map it the same way the route maps other validation failures.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/`(main)/api/v1/leads/route.ts around lines 418 - 428, Wrap the
compileFilter call in the GET handler with the route’s existing validation-error
handling, catch FilterCompileError, and return the same 422 response shape used
for other invalid filter inputs. Keep successful query compilation and unrelated
errors unchanged.
| case "before": | ||
| return `${col}.lt.${pgVal(new Date(String(value)).toISOString())}`; | ||
| case "after": | ||
| return `${col}.gt.${pgVal(new Date(String(value)).toISOString())}`; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the filter schema for date-value validation.
fd -t f 'schema.ts' src/lib/filters --exec cat -n {}
fd -t f 'operators.ts' src/lib/filters --exec cat -n {}Repository: Zunkireelabs/edgexcrm
Length of output: 7484
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== compile.ts outline =="
ast-grep outline src/lib/filters/compile.ts --view expanded | sed -n '1,220p'
echo "== compile.ts relevant sections =="
sed -n '1,280p' src/lib/filters/compile.ts | cat -n
echo "== route/filter integration snippets =="
rg -n "compileFilter|planFilter|filterTreeSchema|created_at|date_between|new Date\\(String\\(value\\)\\)" src -SRepository: Zunkireelabs/edgexcrm
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== compile.ts plan/filter call sites =="
sed -n '518,590p' src/lib/filters/compile.ts | cat -n
sed -n '280,390p' src/lib/filters/compile.ts | cat -n
echo "== compile tests for invalid date values =="
fd -t f 'compile.test.ts|filters.test.ts|validation.test.ts' . -x sh -c 'echo "--- $1"; rg -n "before|after|on|date_between|Invalid|compileFilter|planFilter|garbage|invalid date" "$1" || true' sh {}
echo "== behavioral probe: JS Date parsing semantics for invalid strings in relevant branches =="
node - <<'JS'
for (const value of ["garbage", "2026-01-01", "2026-13-00", "2026-13-40", ""]) {
try {
new Date(String(value)).toISOString();
console.log(value, JSON.stringify(String(value)), "ok");
} catch (err) {
console.log(value, JSON.stringify(String(value)), err.name + ": " + err.message);
}
}
JSRepository: Zunkireelabs/edgexcrm
Length of output: 28072
Reject invalid date filter values before parsing them.
filterTreeSchema.accepts "before"/"after"/"on" values as non-empty strings and planFilter accepts them too, but compileFilter then calls new Date(String(value)).toISOString() or dayBoundsInTz(String(value), ctx.tz). Invalid strings such as "garbage" throw RangeError during compilation instead of returning a 422. Catch these parsing failures and throw FilterCompileError, including bounds for date_between.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/filters/compile.ts` around lines 224 - 227, Update compileFilter’s
date handling for “before”, “after”, “on”, and date_between to validate parsed
dates before calling toISOString or dayBoundsInTz. Catch invalid-date parsing
failures and throw FilterCompileError, including the applicable bounds for
date_between, so invalid filter values produce the established validation
response instead of RangeError.
| function resolveAndValidate(registry: FieldRegistry, cond: FilterCondition): FieldDef { | ||
| const field = requireField(registry, cond.field); | ||
| requireOperator(field, cond); | ||
|
|
||
| if ((cond.op === "is_any_of" || cond.op === "is_none_of" || cond.op === "has_all") && Array.isArray(cond.value) && cond.value.length === 0) { | ||
| // Belt-and-suspenders: schema.ts already enforces .min(1), but a caller | ||
| // that hand-builds a tree (bypassing decodeFilterTree) must not get a | ||
| // silent no-op — the empty-pipeline-allow-list incident is exactly this | ||
| // shape of bug, just on a different filter axis. | ||
| throw new FilterCompileError(`${cond.op} requires at least one value`, "invalid_value"); | ||
| } | ||
|
|
||
| return field; | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
compileFilter does not enforce visibleTo, and no test covers that path. planFilter rejects a field whose visibleTo predicate returns false, but resolveAndValidate skips that check, so compileFilter fails open for any caller that does not run planFilter first. The module doc comment names several such intended call sites.
src/lib/filters/compile.ts#L379-L392: add thefield.visibleTo(ctx.permissions)check toresolveAndValidateand threadctxin fromapplyConditionToBuilderandapplyOrConditions.src/lib/filters/compile.test.ts#L605-L621: add acompileFiltercase beside the existingplanFiltercase that asserts aFilterCompileErrorfor thegatedfield.
📍 Affects 2 files
src/lib/filters/compile.ts#L379-L392(this comment)src/lib/filters/compile.test.ts#L605-L621
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/filters/compile.ts` around lines 379 - 392, Update
src/lib/filters/compile.ts lines 379-392: change resolveAndValidate to accept
the filter context, reject fields when visibleTo(ctx.permissions) is false, and
thread ctx through applyConditionToBuilder and applyOrConditions. Add a
neighboring compileFilter test in src/lib/filters/compile.test.ts lines 605-621
asserting that the gated field raises FilterCompileError, alongside the existing
planFilter coverage.
| export function pgLike(raw: string, mode: LikeMode): string { | ||
| // Escape the USER's own wildcard characters first, so a search for a literal | ||
| // "50%" or "a_b" doesn't turn into a wildcard match… | ||
| const lit = raw.replace(/([\\%_])/g, "\\$1"); | ||
| const pat = mode === "contains" ? `%${lit}%` : mode === "prefix" ? `${lit}%` : mode === "suffix" ? `%${lit}` : lit; | ||
| // …THEN add ours outside the escape, and quote the whole thing through pgVal | ||
| // so a value containing `,` `(` `)` etc. still can't break out of the value | ||
| // position. | ||
| return pgVal(pat); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
For the PostgREST version used by Supabase, does *inside a quotedlikeorilike filter value act as a wildcard? How should a literal asterisk be escaped?
💡 Result:
In PostgREST, the * character inside a like or ilike filter value acts as an alias for the SQL LIKE operator's percent sign % (the multi-character wildcard) [1][2][3]. This alias is provided specifically to simplify URL encoding, as the % character often requires special encoding in URLs [1][4][3]. Regarding literal matches: 1. Escaping a literal asterisk: Because PostgREST maps * to % before passing the pattern to the underlying PostgreSQL LIKE or ILIKE operator, you must ensure that any intended literal asterisk is treated as such by PostgreSQL. To match a literal character that acts as a wildcard, you must use the SQL ESCAPE mechanism [5]. 2. Practical Approach: Since PostgREST converts * to %, using * in your filter will generally be interpreted as a wildcard. To match a literal character, it is often safer and more standard to use the actual SQL percent sign % in your filter value and, if necessary, use the backslash \ as an escape character (e.g., \%) as per standard PostgreSQL pattern matching rules [5]. Note that if your filter value contains reserved characters (which include *, ,, ., :, (, )), PostgREST documentation recommends surrounding the value in double quotes [4]. If you are using the Supabase client libraries, these libraries typically handle the necessary encoding for you [6][7].
Citations:
- 1: https://docs.postgrest.org/en/latest/references/api/tables_views.html
- 2: https://docs.postgrest.org/en/v12/references/api/tables_views.html
- 3: https://github.com/PostgREST/postgrest/blob/b8ca1bb0/docs/references/api/tables_views.rst
- 4: https://docs.postgrest.org/en/latest/references/api/url_grammar.html
- 5: https://www.postgresql.org/docs/current/functions-matching.html
- 6: https://supabase.com/docs/reference/kotlin/ilike
- 7: https://github.com/supabase/postgrest-js/blob/master/src/PostgrestFilterBuilder.ts
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)pgrst\.ts$|package\.json|pnpm-lock\.yaml|bun\.lockb$|yarn\.lock$|postgrest|supabase' || true
echo "== pgrst.ts excerpt =="
fd -a 'pgrst\.ts$' . | while read -r f; do
echo "-- $f"
wc -l "$f"
cat -n "$f" | sed -n '1,160p'
done
echo "== references to pgLike/PostgREST/version =="
rg -n "pgLike|PostgREST|postgrest|supabase|filterString|pgVal|LikeMode|like|ilike" -S . --glob '!node_modules' --glob '!dist' --glob '!build' | head -200Repository: Zunkireelabs/edgexcrm
Length of output: 43922
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== deterministic AST/text verifier for pgLike escaping =="
python3 - <<'PY'
import re, pathlib
paths = [p for p in pathlib.Path('.').rglob('*') if p.name == 'pgrst.ts']
for path in paths:
text = path.read_text()
m = re.search(r'function pgLike\([^)]*\)[^{]*{(?P<body>(?:[^}]|\{[^}]*\})*)}', text, re.S)
print(path)
print(m.group('body') if m else 'pgLike not found')
escapes = re.findall(r'replace\(/\\(([^/`\\]|\\.)*?)\)/', m.group('body') if m else '')
if escapes:
esc_charset = set()
for part in escapes:
esc_charset.update(part)
print("pgLike escapes characters:", "".join(sorted(esc_charset)) or "none")
if m and '"' in m.group('body'):
print("pgLike uses quotes:", bool(re.search(r'pgVal\([^)]*"?[^"\\]*"', m.group('body'))))
PYRepository: Zunkireelabs/edgexcrm
Length of output: 497
🌐 Web query:
PostgREST 12 like filter asterisk percent URL grammar "*" acts as percent
💡 Result:
In PostgREST 12, the asterisk symbol (*) is supported as an alias for the percent sign (%) in the like and ilike filter operators [1][2]. This functionality allows you to avoid URL encoding the percent sign, which would otherwise be represented as %25 [1][3]. Because the asterisk is considered a reserved character in the PostgREST URL grammar, it must be handled carefully when it appears as part of the data you are searching for [4]. If your filter pattern includes a literal asterisk or other reserved characters (such as commas, dots, or parentheses), you must surround the value in double quotes and percent-encode those quotes as %22 [5][4]. For example, to search for a pattern using the asterisk alias:?column=like.pattern If you need to include a reserved character or a literal asterisk in your query value, the documentation recommends the following format [5][4]:?column=like."some%20pattern%20with%20reserved%20chars" Some HTTP libraries may automatically handle encoding, in which case you can use double quotes "" directly instead of %22 [5][4].
Citations:
- 1: https://postgrest.org/en/v12/references/api/tables%5Fviews.html
- 2: https://docs.postgrest.org/en/v12/references/api/tables_views.html
- 3: https://postgrest.org/en/stable/%5Fsources/references/api/tables%5Fviews.rst.txt
- 4: https://docs.postgrest.org/en/latest/references/api/url_grammar.html
- 5: https://github.com/postgrest/postgrest/blob/v12/docs/references/api/url_grammar.rst
🌐 Web query:
PostgREST 12 URL grammar reserved characters percent percent escaping quotes asterisk
💡 Result:
In PostgREST 12, certain characters are reserved for the API URL grammar and require special handling if they appear within filter values [1][2]. Reserved Characters and Escaping The characters reserved by PostgREST include the comma (,), period (.), colon (:), and parentheses ( ( and)) [1][2][3]. If your filter values contain any of these characters, you must surround the value with percent-encoded double quotes (%22) to ensure the API processes them correctly [1][2]. Example: To filter a name containing a comma, such as "Hebdon,John", use: curl "http://localhost:3000/employees?name=in.(%22Hebdon,John%22)" [1][2] Handling Double Quotes If the value itself contains a double quote character ("), you can escape it using a backslash (). Because the backslash is also a special character in this context, it must be percent-encoded as %5C [1][2]. Example: To include a double quote in a value, use %5C%22: curl "http://localhost:3000/marks?name=in.(%22Quote:%5C%22%22)" [1][2] Note on Asterisk () The asterisk () is used in PostgREST as an alias for the percent sign (%) in LIKE and ILIKE operators to avoid the need for URL encoding the percent sign [4]. While some versions of the documentation explicitly list it among reserved characters [5], standard filter values containing it should generally be handled using the same percent-encoded double-quote rule if they conflict with URL grammar interpretation [1][2]. Important Considerations - HTTP Libraries: Some HTTP libraries (such as axios) may automatically encode URLs. In these cases, you should use literal double quotes ("") in your code rather than the percent-encoded %22 [1][2]. - URL Encoding: General URL components that are not PostgREST-specific (such as spaces in table or column names) should be handled via standard percent-encoding (e.g., %20 for spaces) [6][7].
Citations:
- 1: https://docs.postgrest.org/en/v12/references/api/url_grammar.html
- 2: https://github.com/postgrest/postgrest/blob/v12/docs/references/api/url_grammar.rst
- 3: question about reserved characters PostgREST/postgrest#4254
- 4: https://postgrest.org/en/v12/references/api/tables_views.html
- 5: https://docs.postgrest.org/en/latest/references/api/url_grammar.html
- 6: https://docs.postgrest.org/en/stable/references/api/url_grammar.html
- 7: https://docs.postgrest.org/en/v14/references/api/url_grammar.html
🌐 Web query:
PostgREST source like filter quoteLike asterisk percent URL grammar parse quotes
💡 Result:
In PostgREST, when using the like or ilike operators, you can use the asterisk character * as an alias for the percent sign % in your filter patterns [1][2][3]. This substitution is provided specifically to avoid the need for URL encoding the percent character [1][4][5]. Regarding URL grammar and reserved characters, if your filter values or identifiers include any PostgREST reserved characters—defined as commas (,), periods (.), colons (:), asterisks (*), and parentheses ((, ))—you must surround them in percent-encoded double quotes (%22) to ensure correct parsing [6][7][8]. For example, to filter on a column name containing a reserved character like information.cpe, you would use: ?%22information.cpe%22=like.*MS* [9][7] Additionally, if the value being filtered with an operator like in contains a double quote character, it can be escaped using a backslash (\"), and a literal backslash can be escaped with a double backslash (\\) [9][7]. Note that in some contexts, such as the in, and, or or operators, double quoting is natively supported, while for other simple operators like eq, arbitrary double quoting may not behave as expected [10].
Citations:
- 1: https://docs.postgrest.org/en/latest/references/api/tables%5Fviews.html
- 2: https://docs.postgrest.org/en/v13/references/api/tables_views.html
- 3: https://postgrest.org/en/latest/references/api/tables%5Fviews.html
- 4: https://docs.postgrest.org/en/v12/references/api/tables_views.html
- 5: https://postgrest.org/en/v11/references/api/tables_views.html
- 6: https://docs.postgrest.org/en/latest/references/api/url_grammar.html
- 7: https://postgrest.org/en/latest/references/api/url_grammar.html
- 8: question about reserved characters PostgREST/postgrest#4254
- 9: https://docs.postgrest.org/en/stable/references/api/url_grammar.html
- 10: Normalize double quoting across all operators PostgREST/postgrest#1943
🌐 Web query:
PostgREST quoteLike function source
💡 Result:
There is no function named quoteLike in the PostgREST codebase. The confusion likely arises from a misunderstanding of PostgREST's implementation of pattern-matching filters (like like and ilike) or from external code. 1. PostgREST does not have a quoteLike function. The PostgREST codebase handles pattern-matching operators by mapping them to PostgreSQL equivalents (e.g., like maps to the like operator) and processing the user-provided filter value [1]. 2. Other technical projects do contain a QuoteLike function or class. Specifically: - A Perl module named PPIx::QuoteLike exists for parsing Perl string literals [2][3][4]. - A function named QuoteLike (written in Go) exists in the LUCI Go library (infra/luci/luci-go), which is designed to escape special characters (\, %, _) in a literal string so it can be used safely in a LIKE expression [5]. If you are seeing references to quoteLike in the context of a project interacting with PostgREST, it is likely custom application-level code written in a language like Go, rather than a built-in PostgREST feature [5][6].
Citations:
- 1: https://github.com/PostgREST/postgrest/blob/cfaeff8a5a76f541fc744ea9b97321ce65dc426e/src/PostgREST/Query/SqlFragment.hs
- 2: https://web.do.metacpan.org/release/WYANT/PPIx-QuoteLike-0.023/source/lib/PPIx/QuoteLike.pm
- 3: https://github.com/deepin-community/libppix-quotelike-perl
- 4: https://metacpan.org/pod/PPIx::QuoteLike
- 5: https://chromium.googlesource.com/infra/luci/luci-go/+/b1f798f8cec1ed40e43eb5003359e30742a0807d/analysis/internal/span/util.go
- 6: https://stackoverflow.com/questions/78627811/swift-golang-api-issue-fetching-from-db-upon-view-populating
🌐 Web query:
@supabase/postgrest-js 2.97.0 rawLike filter generation asterisk percent escaping
💡 Result:
In @supabase/postgrest-js, there is no specific method named rawLike. The library provides a.filter method intended as an escape hatch [1][2] and a dedicated.like method [3][4]. If you are using.like (or.ilike), the library handles the generation and encoding of the pattern [5]. PostgREST, which powers the Supabase backend, allows the use of an asterisk (*) as an alias for the percent sign (%) to avoid URL encoding issues [6]. For example, a pattern like word in the JavaScript client is translated into the appropriate PostgREST query parameter, and the backend interprets the asterisk correctly as a wildcard [6][7]. If you are using the.filter escape hatch, note that: 1. It is intended for raw PostgREST syntax and does not provide automatic escaping for filter values [1][2]. 2. You are responsible for ensuring the value is properly formatted for the PostgREST API [1][2]. 3. When using filters that require reserved characters (such as commas or parentheses) or special handling, values must often be wrapped in double quotes (%22) [7]. 4. Recent versions of the library (including 2.97.0 and later) include improvements for escaping backslashes and double quotes when using built-in filter methods (like.like or.in), ensuring that patterns containing these characters are handled correctly without breaking the query [5]. For manual construction of patterns where you need to match literal percent signs or asterisks, you should be aware that PostgreSQL uses the backslash () as the default escape character [8]. To match a literal character that is otherwise a wildcard, it must be escaped (e.g., % or *) [8]. However, because the Supabase client handles the transition from your JavaScript call to the URL parameter, it is highly recommended to use the standard.like method rather than attempting to manually construct raw filter strings unless absolutely necessary [1][3].
Citations:
- 1: https://supabase.github.io/postgrest-js/v2/classes/PostgrestFilterBuilder.html
- 2: https://cdn.jsdelivr.net/npm/@supabase/postgrest-js@2.110.8/src/PostgrestFilterBuilder.ts
- 3: https://supabase.com/docs/reference/javascript/using-filters-like
- 4: https://supabase.github.io/postgrest-js/v1/classes/_lib_postgrestfilterbuilder_.postgrestfilterbuilder.html
- 5: fix(postgrest): escape " and \ inside quoted filter values supabase/supabase-js#2529
- 6: https://postgrest.org/en/latest/references/api/tables%5Fviews.html
- 7: https://docs.postgrest.org/en/stable/references/api/url_grammar.html
- 8: Backslash literal in LIKE expression - Postgres (and H2, MySql) behaviour ebean-orm/ebean#192
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== lockfile postgrest-js exact =="
python3 - <<'PY'
import json
from pathlib import Path
p=Path('package-lock.json')
data=json.loads(p.read_text())
for key,val in data.get('packages',{}).items():
if key.endswith('node_modules/@supabase/postgrest-js'):
print(key,val)
PY
echo "== local node_modules if present =="
fd -a 'PostgrestFilterBuilder.ts$|PostgrestFilterBuilder.js$' node_modules 2>/dev/null | sed -n '1,20p' || true
echo "== pkg fetch for 2.97.0 if network available =="
python3 - <<'PY'
import json, urllib.request, tempfile, zipfile, pathlib
url='https://registry.npmjs.org/@supabase/postgrest-js/2.97.0'
data=json.load(urllib.request.urlopen(url, timeout=15))
print(data.get('version'), data.get('dist',{}).get('tarball'))
tarball=data['dist']['tarball']
with tempfile.NamedTemporaryFile(suffix='.tgz', delete=False) as f: f.write(urllib.request.urlopen(tarball, timeout=20).read())
tmp=f.name
with zipfile.ZipFile(tmp) as z:
names=[n for n in z.namelist() if n.endswith(('PostgrestFilterBuilder.ts','PostgrestFilterBuilder.js'))]
print(names)
for name in names[:3]:
txt=z.read(name).decode('utf-8', errors='replace')
print(f'--- {name}')
for i,line in enumerate(txt.splitlines(),1):
l=line.strip()
if any(s in l for s in ['like(value', 'ilike(value', 'addFilter', 'pg_filter' ,'filterString', 'escape']):
print(f'{i}: {l}')
PYRepository: Zunkireelabs/edgexcrm
Length of output: 2911
Escape literal PostgREST * in pgLike.
pgLike escapes only \, %, and _, but PostgREST treats * as the percent-sign wildcard for like/ilike. Include * in the escaped literal, and add regression coverage for each LikeMode.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/filters/pgrst.ts` around lines 29 - 37, Update pgLike to escape
PostgREST’s literal * wildcard alongside \, %, and _ before applying the
mode-specific pattern. Add regression coverage verifying literal * handling for
contains, prefix, suffix, and exact LikeMode values.
| function compileAssignees(cond: FilterCondition): string { | ||
| const values = asList(cond.value); | ||
| const wantsUnassigned = values.includes("unassigned"); | ||
| const ids = values.filter((v) => v !== "unassigned" && UUID_RE.test(v)); | ||
|
|
||
| if (wantsUnassigned && ids.length > 0) return or("assigned_to.is.null", `assigned_to.in.(${ids.map(pgVal).join(",")})`); | ||
| if (wantsUnassigned) return "assigned_to.is.null"; | ||
| if (ids.length > 0) return ids.length === 1 ? `assigned_to.eq.${pgVal(ids[0])}` : `assigned_to.in.(${ids.map(pgVal).join(",")})`; | ||
| return "id.not.is.null"; // no valid tokens — legacy applies no filter in this case | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
The id.not.is.null no-op becomes match-everything inside an OR clause.
When no token is a valid UUID and "unassigned" is absent, compileAssignees returns the always-true predicate id.not.is.null. In an AND tree this reproduces the legacy no-filter behavior, which is the stated intent. In an OR clause it does not: applyOrConditions in src/lib/filters/compile.ts joins every condition into a single or(...), so one always-true leg makes the whole clause true and the query returns the caller's entire visible row set. A ?f= tree with conjunction: "or" and { field: "assignees", op: "is_any_of", value: ["garbage"] } triggers this.
This also contradicts compileSource on Line 55, which throws FilterCompileError for an empty value list under the same fail-closed rule.
🛡️ Proposed fix: fail closed instead of emitting a tautology
function compileAssignees(cond: FilterCondition): string {
const values = asList(cond.value);
const wantsUnassigned = values.includes("unassigned");
const ids = values.filter((v) => v !== "unassigned" && UUID_RE.test(v));
if (wantsUnassigned && ids.length > 0) return or("assigned_to.is.null", `assigned_to.in.(${ids.map(pgVal).join(",")})`);
if (wantsUnassigned) return "assigned_to.is.null";
if (ids.length > 0) return ids.length === 1 ? `assigned_to.eq.${pgVal(ids[0])}` : `assigned_to.in.(${ids.map(pgVal).join(",")})`;
- return "id.not.is.null"; // no valid tokens — legacy applies no filter in this case
+ // No valid tokens. An always-true predicate is safe in an AND tree but turns an
+ // OR clause into match-everything, so fail closed like compileSource does.
+ throw new FilterCompileError("assignees requires at least one valid user id or \"unassigned\"", "invalid_value");
}If the legacy no-op must be preserved byte-for-byte, drop the condition in legacyLeadsParamsToTree when every token is invalid, rather than emitting a tautology from the compiler. That keeps the legacy path unchanged and removes the OR hazard. Note that src/lib/filters/registry/leads.test.ts Line 117-120 asserts the current tautology, so update that test with whichever option you choose.
📝 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.
| function compileAssignees(cond: FilterCondition): string { | |
| const values = asList(cond.value); | |
| const wantsUnassigned = values.includes("unassigned"); | |
| const ids = values.filter((v) => v !== "unassigned" && UUID_RE.test(v)); | |
| if (wantsUnassigned && ids.length > 0) return or("assigned_to.is.null", `assigned_to.in.(${ids.map(pgVal).join(",")})`); | |
| if (wantsUnassigned) return "assigned_to.is.null"; | |
| if (ids.length > 0) return ids.length === 1 ? `assigned_to.eq.${pgVal(ids[0])}` : `assigned_to.in.(${ids.map(pgVal).join(",")})`; | |
| return "id.not.is.null"; // no valid tokens — legacy applies no filter in this case | |
| } | |
| function compileAssignees(cond: FilterCondition): string { | |
| const values = asList(cond.value); | |
| const wantsUnassigned = values.includes("unassigned"); | |
| const ids = values.filter((v) => v !== "unassigned" && UUID_RE.test(v)); | |
| if (wantsUnassigned && ids.length > 0) return or("assigned_to.is.null", `assigned_to.in.(${ids.map(pgVal).join(",")})`); | |
| if (wantsUnassigned) return "assigned_to.is.null"; | |
| if (ids.length > 0) return ids.length === 1 ? `assigned_to.eq.${pgVal(ids[0])}` : `assigned_to.in.(${ids.map(pgVal).join(",")})`; | |
| // No valid tokens. An always-true predicate is safe in an AND tree but turns an | |
| // OR clause into match-everything, so fail closed like compileSource does. | |
| throw new FilterCompileError("assignees requires at least one valid user id or \"unassigned\"", "invalid_value"); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/filters/registry/leads.ts` around lines 70 - 79, Update
compileAssignees to fail closed when no valid UUID or "unassigned" token is
present, matching compileSource instead of returning the always-true
id.not.is.null predicate; preserve valid assigned and unassigned combinations.
Update the affected leads tests, including the assertion around the current
tautology, and if legacy no-op behavior must remain byte-for-byte, implement the
dropping behavior in legacyLeadsParamsToTree instead.
| -- Rollback: DROP INDEX CONCURRENTLY IF EXISTS idx_leads_tenant_created_active_nonother; | ||
| -- Applied: stage 2026-08-07 / prod HELD (promotion gate). | ||
| -- | ||
| -- NOT in a transaction: CREATE INDEX CONCURRENTLY cannot run inside BEGIN/COMMIT | ||
| -- (see supabase/migrations/085_unique_display_id.sql for the precedent). |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Expected: explicit transaction handling for this migration and a defined
# authoritative migration history table.
rg -n -C 5 \
-e 'CREATE INDEX CONCURRENTLY' \
-e 'DROP INDEX CONCURRENTLY' \
-e 'schema_migrations' \
-e '\bBEGIN\b|\bCOMMIT\b' \
-e 'supabase (db push|migration)' \
--glob '!node_modules/**' \
. || trueRepository: Zunkireelabs/edgexcrm
Length of output: 50380
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '--- target migration ---\n'
cat -n supabase/migrations/201_leads_tags_other_partial_index.sql
printf '\n--- migration script outline ---\n'
ls -1supabase/migrations/*.sql 2>/dev/null | wc -l
printf 'concurrent index migrations:\n'
rg -n '^BEGIN;$|CREATE INDEX CONCURRENTLY|DROP INDEX CONCURRENTLY|schema_migrations|Migration template|NO BEGIN/COMMIT|CREATE INDEX IF NOT EXISTS' supabase/migrations/*.sql docs scripts 2>/dev/null | head -n 200
printf '\n--- schema_migrations table definition ---\n'
rg -n -C 4 'CREATE TABLE.*schema_migrations|public.schema_migrations|pgmigrations' supabase/migrations scripts docs 2>/dev/null | head -n 240Repository: Zunkireelabs/edgexcrm
Length of output: 250
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- target migration ---'
cat -n supabase/migrations/201_leads_tags_other_partial_index.sql
printf '%s\n' ''
printf '%s\n' '--- relevant pattern summary ---'
rg -n -C 3 '^BEGIN;$|CREATE INDEX CONCURRENTLY|DROP INDEX CONCURRENTLY|schema_migrations|Migration template|NO BEGIN/COMMIT|CREATE INDEX IF NOT EXISTS' supabase/migrations scripts docs 2>/dev/null | head -n 240
printf '%s\n' ''
printf '%s\n' '--- schema_migrations table definition refs ---'
rg -n -C 4 'CREATE TABLE.*schema_migrations|pgmigrations|supabase/migrations' supabase/migrations scripts docs 2>/dev/null | head -n 240Repository: Zunkireelabs/edgexcrm
Length of output: 41703
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- migrate-apply.sh command assembly and transaction options ---'
sed -n '1,150p' scripts/migrate-apply.sh
printf '%s\n' ''
printf '%s\n' '--- migration guard and CONCURRENTLY handling ---'
sed -n '60,115p' scripts/check-migrations.sh
printf '%s\n' ''
printf '%s\n' '--- concurrent migrations without BEGIN/COMMIT and self-record ---'
python3 - <<'PY'
from pathlib import Path
import re
for p in sorted(Path('supabase/migrations').glob('[0-9][0-9][0-9]_*.sql')):
s = p.read_text(errors='replace')
cc = bool(re.search(r'CREATE\s+INDEX\s+(UNIQUE\s+)?CONCURRENTLY', s, re.I))
begin = bool(re.search(r'^BEGIN;$', s, re.M))
commit = bool(re.search(r'^COMMIT;$', s, re.M))
self = bool(re.search(r"INSERT\s+INTO\s+public\.schema_migrations\s*\(\s*version\s*\)\s*VALUES\s*\(\s*'{}'".format(p.name), s))
if cc:
print(p.as_posix(), 'concurrent=True', 'begin=', bool(begin), 'commit=', bool(commit), 'self_record=', bool(self))
PYRepository: Zunkireelabs/edgexcrm
Length of output: 7456
🌐 Web query:
PostgreSQL CREATE INDEX CONCURRENTLY transaction block error
💡 Result:
The PostgreSQL error "CREATE INDEX CONCURRENTLY cannot run inside a transaction block" occurs because the CONCURRENTLY option requires the index build process to perform multiple table scans and manage its own internal transaction boundaries [1][2][3]. Because the operation needs to wait for existing transactions to complete to ensure data consistency, it cannot be executed within a user-defined transaction block (BEGIN/COMMIT) [1][3][4]. If you attempt to run this command inside an explicit transaction block, PostgreSQL will raise SQLSTATE 25001 [3]. To resolve this issue, you must ensure the command is executed outside of any transaction block. Common solutions include: 1. Migration Tools: Many ORM or migration frameworks (such as Rails, TypeORM, or Flyway) wrap migrations in a transaction by default [2][3]. You must configure these tools to disable the transaction wrapper specifically for migrations that include a CONCURRENTLY index build [2][3]. 2. Direct Execution: Run the command as a standalone SQL statement, outside of any BEGIN...COMMIT block [2][3]. Note that this same restriction applies to the DROP INDEX CONCURRENTLY command, as it shares the same multi-transactional requirement [4].
Citations:
- 1: https://www.postgresql.org/docs/current/sql-createindex.html
- 2: https://pgfence.com/blog/concurrently-in-a-transaction-silent-footgun/
- 3: https://thesev1database.com/errors/msg-create-index-concurrently-cannot-run-inside-a-transaction-block/
- 4: https://postgrespro.com/list/thread-id/2322335
Align the concurrent index migration with the repo transaction policy.
CREATE INDEX CONCURRENTLY cannot run inside BEGIN/COMMIT, but post-ledger migrations currently wrap their index DDL and public.schema_migrations self-record in one transaction. If this file is run through the normal transactional migration path, the index creation fails before self-recording. Choose one migration strategy before merge: use ordinary CREATE INDEX inside the transaction, or update scripts/check-migrations.sh / the deployment SOP to allow the non-transactional CONCURRENTLY pattern and document the recovery path for invalid indexes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@supabase/migrations/201_leads_tags_other_partial_index.sql` around lines 6 -
10, Choose and implement one supported migration strategy for this file: replace
the concurrent index creation with ordinary CREATE INDEX so the DDL and
schema_migrations self-record remain transactional, or update
scripts/check-migrations.sh and the deployment SOP to explicitly support
non-transactional CONCURRENTLY execution, including invalid-index recovery.
Ensure the rollback statement and migration comments match the selected policy.
Sources: Coding guidelines, Linters/SAST tools
2 participants
Summary
Migrations
Both applied to prod DB (pirhnklvtjjpuvbvibxf) and verified before this PR was opened:
200_class_managers.sql— new class_managers table + RLS + backfill (3 marker rows + 13 position-holder rows → 13 deduped rows)201_leads_tags_other_partial_index.sql— partial index (CONCURRENTLY, no data change)Test plan
Summary by CodeRabbit
New Features
Bug Fixes