Skip to content

Conversation

@Pbonmars-20031006
Copy link
Contributor

@Pbonmars-20031006 Pbonmars-20031006 commented Dec 17, 2025

Summary

Adding more KB tag types + filters.

Type of Change

  • New feature

Testing

Tested Manually.

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)

priyanshu.solanki added 9 commits December 17, 2025 12:33
- Update base-tags-modal with field type selector dropdown
- Update document-tags-modal with different input types per fieldType
- Update knowledge-tag-filters with operator dropdown and type-specific inputs
- Update search routes to support all tag slot types
- Update hook to use AllTagSlot type
- Add dropdown with all field types (Text, Number, Date, Boolean)
- Render different value inputs based on field type
- Update slot counting to include all field types (28 total)
- Replace MAX_TAG_SLOTS with totalSlots in document-tag-entry
- Add z-index to SelectContent in base-tags-modal for proper layering
- Only apply empty string check for text columns (tag1-tag7)
- Numeric/date/boolean columns only check IS NOT NULL
- Cast values to text for consistent output
- Replace @/components/ui imports with @/components/emcn
- Use Combobox instead of Select for dropdowns
- Use EMCN Switch, Button, Input, Label components
- Remove unsupported 'size' prop from EMCN Button
- Change delete button from absolute to inline positioning
- Add proper column width (w-10) for delete button
- Add empty header cell for delete column
- Apply fix to both document-tag-entry and knowledge-tag-filters
- Reset value to empty when changing type (e.g., boolean to text)
- Reset value when tag name changes and type differs
- Prevents 'true'/'false' from sticking in text inputs
…arch

- Copy all tag types (number, date, boolean) from document to embedding records
- Update processDocumentTags to handle all field types with proper type conversion
- Add number/date/boolean columns to document queries in checkDocumentWriteAccess
- Update chunk creation to inherit all tag types from parent document
- Add getSearchResultFields helper for consistent query result selection
- Support structured filters with operators (eq, gt, lt, between, etc.)
- Fix search queries to include all 28 tag fields in results
@vercel
Copy link

vercel bot commented Dec 17, 2025

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

1 Skipped Deployment
Project Deployment Review Updated (UTC)
docs Skipped Skipped Dec 20, 2025 4:57am

@Pbonmars-20031006 Pbonmars-20031006 changed the base branch from main to staging December 17, 2025 23:01
@icecrasher321
Copy link
Collaborator

@Pbonmars-20031006 also don't forget to generate the migration using bunx drizzle-kit generate to the sql, snapshot, etc shows up [you can do that before me merge]

@icecrasher321
Copy link
Collaborator

@Pbonmars-20031006 build failing here

Also, we realized that the write speed would become really slow if we have this many indices. Can we have 3 for number and 2 for date.

And for text have more filters like "contains" and "starts with" if they have equivalent sql ops -- e.g. https://stackoverflow.com/questions/1069302/using-the-star-sign-in-grep

@icecrasher321 icecrasher321 marked this pull request as ready for review December 18, 2025 21:18
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 18, 2025

Greptile Summary

  • Extends knowledge base tagging system to support typed tags (number, date, boolean) beyond text tags with structured filtering capabilities including operators like equals, greater than, between, and contains
  • Adds comprehensive database schema changes with typed columns (number1-5, date1-2, boolean1-3) and corresponding indexes to both document and embedding tables
  • Implements new UI components including DatePicker, structured filter interfaces, and type-specific input controls for managing the enhanced tagging system

Important Files Changed

Filename Overview
packages/db/migrations/0126_dapper_midnight.sql Database migration adding typed tag columns (number, date, boolean) to document/embedding tables with indexes for filtering
apps/sim/lib/knowledge/constants.ts Schema mismatch - hardcodes 5 number/2 date/3 boolean slots but database migration creates7 of each type
apps/sim/lib/knowledge/filters/query-builder.ts Comprehensive query builder for typed tag filtering with SQL generation, but slot validation conflicts with database schema
apps/sim/lib/knowledge/documents/service.ts Breaking changes to processDocumentTags function signature and return type, requires existing tag definitions

Confidence score: 2/5

  • This PR requires careful review due to critical schema inconsistencies that will cause runtime failures
  • Score lowered due to hardcoded slot count mismatches between constants (5,2,3) and database migration (7,7,7 for each type), breaking changes in service functions, and potential type safety issues where validation logic conflicts with database schema
  • Pay close attention to schema files, constants definitions, and query builder validation logic to resolve slot count inconsistencies

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

24 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

@icecrasher321
Copy link
Collaborator

@greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Additional Comments (3)

  1. apps/sim/lib/knowledge/chunks/service.ts, line 181-187 (link)

    logic: Return object missing typed tags - only includes text tags (tag1-tag7) but should also include number, date, and boolean tags that were assigned during creation.

  2. apps/sim/app/api/knowledge/[id]/documents/route.test.ts, line 342-348 (link)

    logic: Missing new typed tag fields (number1-5, date1-2, boolean1-3) that were added to the main mockDocument. This inconsistency could cause test failures if the actual service returns these fields.

  3. apps/sim/tools/knowledge/search.ts, line 33-41 (link)

    logic: Parameter schema doesn't reflect new structured format - missing fieldType, operator, tagSlot, valueTo fields that are used in the implementation

34 files reviewed, 12 comments

Edit Code Review Agent Settings | Greptile

@icecrasher321
Copy link
Collaborator

@greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

34 files reviewed, 15 comments

Edit Code Review Agent Settings | Greptile

@icecrasher321 icecrasher321 merged commit 4f69b17 into staging Dec 20, 2025
10 checks passed
@waleedlatif1 waleedlatif1 deleted the feat/filter-tags-update branch December 21, 2025 01:12
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.

3 participants