Skip to content

fix(knowledge): send tag filters as a JSON string so the document filter works#5259

Open
waleedlatif1 wants to merge 6 commits into
stagingfrom
kb-tag-filter-recheck
Open

fix(knowledge): send tag filters as a JSON string so the document filter works#5259
waleedlatif1 wants to merge 6 commits into
stagingfrom
kb-tag-filter-recheck

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

The Knowledge Base document-list tag filter never reached the database — it returned empty for every filter, regardless of operator or value. (Earlier #5221 fixed the backend matching, but the request never got there.)

Root cause: the tagFilters query field was a Zod .transform() that JSON.parsed the string into an array of objects. The client's requestJson parses the query schema before serializing, so appendQuery received the array and emitted tagFilters=[object Object] into the URL. The route's JSON.parse then threw → 400 → the list came back empty (or stale via keepPreviousData, which is why the document still appeared in the reporter's screenshots even though the filter "did nothing").

Changes

  • Contract: tagFilters is now modeled as the wire string it actually is. Decoding into typed filters happens server-side via a new exported parseDocumentTagFiltersParam helper; the route maps a malformed value to a 400.
  • Client hardening: appendQuery now throws on an array-of-objects query param instead of silently serializing [object Object] — this whole bug class now fails loudly at the boundary.
  • UX: the text tag-filter operator now defaults to contains (a partial value matches), instead of exact equals.
  • Tests: requestJson serializes a JSON-string query param verbatim and the guard throws on array-of-objects; the query schema keeps tagFilters a string (guards against reintroducing the transform); the decode helper round-trips and rejects malformed input.

Scope / "is this anywhere else?"

A full sweep of every GET/DELETE contract query field across lib/api/contracts/** confirmed tagFilters was the only field of this class. Explicitly verified safe: the logs filters (level/workflowIds/folderIds/triggers are plain comma-joined strings), table filter/sort (non-array objects, round-tripped via JSON.stringify), and the JSON.parse cases in tables/credentials/mcp (all request bodies, not query).

Type of Change

  • Bug fix

Testing

  • New unit tests (requestJson serialization + guard, contract schema, decode helper) pass; existing documents route tests (16) pass; typecheck + biome + check:api-validation clean.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

…ter works

The document-list tag filter never reached the database. The `tagFilters`
query field was a Zod `.transform()` that decoded the JSON string into an
array of objects; the client's `requestJson` parses the query before
serializing, so `appendQuery` received the array and emitted
`tagFilters=[object Object]` into the URL. The route then failed to
`JSON.parse` it and returned 400, so the list came back empty (or stale via
keepPreviousData) regardless of operator or value.

- Model `tagFilters` as the wire string it actually is; decode it server-side
  via a new `parseDocumentTagFiltersParam` helper (route maps a bad value to 400).
- Harden `appendQuery`: throw on an array-of-objects query param instead of
  silently serializing `[object Object]`, so this whole class fails loudly.
- Default the text tag-filter operator to `contains` so a partial value matches.
- Tests: requestJson serializes the JSON param verbatim + the guard throws; the
  query schema keeps tagFilters a string; the decode helper round-trips.

A full sweep of every GET/DELETE contract query field confirmed this was the
only field of this class — logs filters and table filter/sort are unaffected.
@vercel

vercel Bot commented Jun 29, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Jun 29, 2026 8:24am

Request Review

@cursor

cursor Bot commented Jun 29, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Targeted bug fix on knowledge document listing and query serialization, with stricter validation that should reduce server errors; no auth or payment changes.

Overview
Fixes Knowledge Base document list tag filtering, which was broken because tagFilters was modeled as a Zod transform that turned the wire string into an array before requestJson serialized the query, producing tagFilters=[object Object] and 400 responses.

Contract & API: tagFilters stays a string on the query schema; the documents route decodes it with parseDocumentTagFiltersParam and returns 400 on bad JSON. documentTagFilterSchema now validates slots, operators, and values at the boundary (via isValidFilterValue, including real calendar dates) so bad input fails with 400 instead of 500.

Client: appendQuery throws if a query param is an array of objects, preventing silent [object Object] corruption.

UI: Text tag filters default to contains; incomplete between filters are not sent until both bounds are set.

Reviewed by Cursor Bugbot for commit de73d43. Configure here.

@greptile-apps

greptile-apps Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes Knowledge Base document tag filters so they reach the API in the expected wire format. The main changes are:

  • Keeps tagFilters as a JSON string in the route contract.
  • Decodes and validates tag filters on the server before querying documents.
  • Rejects unsupported operators, slot/type mismatches, bad values, and invalid dates.
  • Throws on unsafe array-of-object query serialization in the client helper.
  • Updates the filter UI defaults and incomplete between handling.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
apps/sim/lib/api/contracts/knowledge/documents.ts Defines the raw tagFilters query string and validates decoded filters for slot, type, operator, and value correctness.
apps/sim/lib/knowledge/filters/types.ts Adds shared filter value checks, including real calendar-date validation.
apps/sim/app/api/knowledge/[id]/documents/route.ts Parses tag filters after query validation and returns a bad-request response for malformed filter input.
apps/sim/lib/api/client/request.ts Prevents array-of-object query params from being silently serialized as invalid strings.
apps/sim/app/workspace/[workspaceId]/knowledge/[id]/base.tsx Builds active tag filters with safer defaults and skips incomplete between filters while the user is editing.

Reviews (6): Last reviewed commit: "fix(knowledge): reject impossible calend..." | Re-trigger Greptile

Comment thread apps/sim/lib/api/contracts/knowledge/documents.ts
Greptile P2: documentTagFilterSchema accepted any non-empty operator string, so
an unsupported operator was silently dropped by the query builder instead of
returning 400. Validate the operator against the field type's allowed set
(single source of truth in filters/types) via superRefine.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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 00d4872. Configure here.

Comment thread apps/sim/lib/api/contracts/knowledge/documents.ts
…ient claim

Greptile P1: operator validation trusted the client-supplied fieldType, so a
numeric slot could be sent with fieldType 'text' + 'contains' and slip through
to build a text LIKE on a numeric column. Validate against the slot's inherent
type via getFieldTypeForSlot (the source of truth): reject unknown slots and
fieldType/slot mismatches at the boundary before checking the operator.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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 7a68802. Configure here.

Comment thread apps/sim/lib/api/contracts/knowledge/documents.ts
Greptile P1: value/valueTo were z.unknown(), so a number filter accepted 'abc',
a date filter 'not-a-date', etc. — unusable values the query builder then
silently dropped. Add a shared isValidFilterValue (single source of truth in
filters/types) and reject unusable value/valueTo at the boundary, including the
between upper bound.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/lib/api/contracts/knowledge/documents.ts
Cursor Bugbot: the strict valueTo validation made a partially-entered between
filter (lower bound only) 400 and break the whole document list mid-entry.
activeTagFilters now withholds a between row until both bounds are filled —
consistent with already requiring the lower bound before sending any filter — so
the list keeps loading while the range is being entered.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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 5f4fe13. Configure here.

Comment thread apps/sim/lib/knowledge/filters/types.ts Outdated
Greptile P1: the date value check was format-only, so 2026-02-30 / 2026-99-99
passed the boundary and then made the document query's ::date cast throw a 500.
Validate real calendar dates by round-tripping the parsed parts.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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 de73d43. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant