feat(documents): fuzzy match document search and folder team management#766
Merged
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
larryro
force-pushed
the
feat/document-list-fuzzy-match
branch
from
64714f2 to
a6e7c7c
Compare
📝 WalkthroughWalkthroughThis pull request refactors document listing to "document find" terminology, introduces team-scoped folder creation and management, and implements fuzzy folder path resolution. Key changes include: renaming the agent tool from Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Replace exact query parameter with fileName-based fuzzy matching in the document list tool, improving how agents search and filter documents.
…pan improvements - Add team assignment to folders with cascade to child folders and documents - Replace multi-team checkbox UI with single-team select dropdown - Support folder entity type in team tags dialog - Implement recursive folder deletion (replace empty-folder guard) - Inherit parent folder team on subfolder/document creation - Block document team changes when inherited from parent folder - Add folder team access filtering in agent document list fuzzy matching - Enhance zoom-pan viewer with keyboard shortcuts, drag/pan, focus states - Add aria-label for chat image attachment button - Add agent:document-list rate limit
…ully handle invalid tool names Rename the document_list agent tool to document_find across the codebase to better reflect its fuzzy-matching search capabilities. Also change validateToolNames to filterValidToolNames so that stale tool references in custom agents are silently dropped instead of throwing errors.
Team filter was only filtering documents/files but not folders, so folders from other teams remained visible when a team was selected. Extract filtering logic into a testable utility and apply both team filter types (context and dropdown) to folders as well.
larryro
force-pushed
the
feat/document-list-fuzzy-match
branch
from
a6e7c7c to
44e3900
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
services/platform/app/features/chat/components/chat-input.tsx (1)
171-186:⚠️ Potential issue | 🟠 MajorInclude the attachment name in the preview button label.
aria-labelreplaces the image alt text here, so every preview button is now announced as the same generic “View image”. With multiple attachments, screen-reader users can’t tell which file each button opens.Suggested fix
<button type="button" - aria-label={tChat('viewImage')} + aria-label={`${tChat('viewImage')}: ${attachment.fileName}`} onClick={() => attachment.previewUrl && setPreviewImage({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/app/features/chat/components/chat-input.tsx` around lines 171 - 186, The preview button's aria-label is too generic (tChat('viewImage')) so screen readers can't distinguish attachments; update the aria-label to include the attachment name (e.g., combine tChat('viewImage') with attachment.fileName) in the button inside chat-input.tsx so each button announces which file it opens; ensure the change is applied where the onClick uses setPreviewImage and when rendering the <img> so the accessible name reflects attachment.fileName and still falls back safely if fileName is missing.services/platform/convex/folders/mutations.ts (1)
146-196:⚠️ Potential issue | 🟠 MajorPersist the full inherited team scope when creating a child folder.
A multi-team parent can admit the caller via
teamTags, buteffectiveTeamId = parent.teamIdcollapses that inheritance to one team and the later membership check can reject the same user. The insert also never writesteamTags, so child folders lose the parent’s full team scope on create.♻️ Proposed fix
- let effectiveTeamId = args.teamId; + let effectiveTeamIds = args.teamId ? [args.teamId] : undefined; @@ - if (parent.teamId) { - effectiveTeamId = parent.teamId; - } + if (parent.teamTags?.length) { + effectiveTeamIds = parent.teamTags; + } else if (parent.teamId) { + effectiveTeamIds = [parent.teamId]; + } @@ - if (effectiveTeamId) { + if (effectiveTeamIds?.length) { const userTeamIds = await getUserTeamIds(ctx, String(authUser._id)); - if (!userTeamIds.includes(effectiveTeamId)) { + if (effectiveTeamIds.some((teamId) => !userTeamIds.includes(teamId))) { throw new Error('Cannot create folder in a team you do not belong to'); } } + + const { teamId, teamTags } = teamIdsToFields(effectiveTeamIds); @@ - teamId: effectiveTeamId, + teamId, + teamTags, createdBy: String(authUser._id),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/convex/folders/mutations.ts` around lines 146 - 196, The code collapses multi-team inheritance by setting effectiveTeamId = parent.teamId and never persisting teamTags; fix by preserving the parent's full team scope: when parent.teamId or parent.teamTags exist, compute childTeamId (single-team inheritance) and childTeamTags (union of parent.teamTags plus parent.teamId if present) and use those for both access checks and the insert. Replace the later membership check that does userTeamIds.includes(effectiveTeamId) with logic that checks access against the full inherited scope (e.g., use hasTeamAccess-like logic or check intersection between userTeamIds and childTeamTags/childTeamId), and include teamTags: childTeamTags in the ctx.db.insert call (alongside teamId if still relevant) so child folders retain the parent's multi-team scope; update references to effectiveTeamId to use the new childTeamId/childTeamTags values and keep checkDuplicateName and createdBy behavior unchanged.services/platform/convex/documents/update_document.ts (1)
36-46:⚠️ Potential issue | 🟠 MajorBlock the empty-array team edit path too.
Because this new guard still sits under
args.teamIds.length > 0, a caller can sendteamIds: []and bypass both the inherited-folder check and theuserIdrequirement while still clearingteamId/teamTags/sharedWithTeamIdsbelow on Lines 88-95. That leaves an easy bypass for the new “inherited from parent folder” restriction.🔒 Proposed fix
- if (args.teamIds !== undefined && args.teamIds.length > 0) { + if (args.teamIds !== undefined) { if (!args.userId) { throw new Error('userId is required when updating teamIds'); } if (document.folderId) { const folder = await ctx.db.get(document.folderId); if (folder?.teamId) { throw new Error('Cannot change team: inherited from parent folder'); } } - const userTeamIds = await getUserTeamIds(ctx, args.userId); - const userTeamSet = new Set(userTeamIds); - for (const id of args.teamIds) { - if (!userTeamSet.has(id)) { - throw new Error( - 'Cannot assign document to a team you do not belong to', - ); + if (args.teamIds.length > 0) { + const userTeamIds = await getUserTeamIds(ctx, args.userId); + const userTeamSet = new Set(userTeamIds); + for (const id of args.teamIds) { + if (!userTeamSet.has(id)) { + throw new Error( + 'Cannot assign document to a team you do not belong to', + ); + } } - } + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/convex/documents/update_document.ts` around lines 36 - 46, The current guard only runs when args.teamIds.length > 0, letting callers pass teamIds: [] to bypass the checks; change the condition to run whenever teamIds is provided (args.teamIds !== undefined) so you enforce the userId requirement and the parent-folder inheritance check for empty arrays too. Specifically, update the block that references args.teamIds, args.userId, document.folderId and ctx.db.get so it triggers on args.teamIds !== undefined (not args.teamIds.length > 0), throw if !args.userId, and still fetch the parent folder and throw if folder?.teamId to block clearing inherited team fields.services/platform/app/features/documents/components/document-row-actions.tsx (1)
140-145:⚠️ Potential issue | 🟠 MajorDon't hide folder team management inside inherited folders.
visible: !parentFolderTeamIdsuppresses this action for folders as well as files. Once you're browsing inside a team-owned folder, child folders lose their only entry point for team assignment even though Lines 197-198 now passentityType={itemType}specifically to support folder handling. The inherited-team lock should apply to files, not to folders that still need their own team-management affordance.🩹 Minimal fix
{ key: 'teamTags', label: tDocuments('actions.manageTeams'), icon: Users, onClick: dialogs.open.teamTags, - visible: !parentFolderTeamId, + visible: itemType === 'folder' || !parentFolderTeamId, },Also applies to: 197-198
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/app/features/documents/components/document-row-actions.tsx` around lines 140 - 145, The team-management action is incorrectly hidden for folders due to the condition visible: !parentFolderTeamId; change this to only suppress the action for files when the parent folder has a team (e.g., visible: !(parentFolderTeamId && itemType === 'file') or equivalent) so folders still show the teamTags action; update the same logic where entityType={itemType} is passed (see the component that renders the action around the entityType/itemType usage) to ensure folder items are allowed to open dialogs.open.teamTags while files inherit the lock.services/platform/convex/documents/list_documents_for_agent.ts (1)
137-139:⚠️ Potential issue | 🟠 MajorInclude secondary team assignments in the
teamIdfilter.Lines 137-139 only compare
doc.teamId, but documents can still carry the full assignment inteamTags. Filtering byteamId: "team-b"will therefore miss documents shared with team B whenever B is not the primaryteamId.🛠️ Suggested fix
// Team filter if (args.teamId) { - if (doc.teamId !== args.teamId) continue; + const docTeamIds = doc.teamTags ?? (doc.teamId ? [doc.teamId] : []); + if (!docTeamIds.includes(args.teamId)) continue; } else if (!hasTeamAccess(doc, teamIdSet)) { continue; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/convex/documents/list_documents_for_agent.ts` around lines 137 - 139, The filter only checks doc.teamId against args.teamId and thus misses documents where the team is listed in secondary assignments (doc.teamTags); update the conditional in list_documents_for_agent (around the block using args.teamId and hasTeamAccess) to allow the document when doc.teamId === args.teamId OR (doc.teamTags && doc.teamTags.includes(args.teamId)), otherwise continue, while keeping the existing fallback that uses hasTeamAccess(teamIdSet) when args.teamId is not provided.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@services/platform/app/components/ui/data-display/zoom-pan-viewer.tsx`:
- Around line 141-146: The container currently uses tabIndex={isZoomed ? 0 :
-1}, which removes it from the tab order until zoomed; change this to always be
tabbable (e.g., tabIndex={0}) so keyboard shortcuts (+/-/0) are reachable at
100% load; keep the existing focus-visible:ring classes for discoverability and
retain the isZoomed-based cursor logic in className (do not modify isDragging or
cursor-grab/grabbing handling).
In
`@services/platform/app/features/documents/components/create-folder-dialog.tsx`:
- Line 45: The form currently defaults to '__org_wide__' before useTeams()
finishes, causing org-wide folders to be created during the teams loading
window; change the form so it does not set a default team value until useTeams()
settles: read isLoading and teams from useTeams(), initialize the team field to
null/undefined (not '__org_wide__') while isLoading is true, and disable the
submit button (or return a loading state) until isLoading is false and teams are
known; additionally, guard the onSubmit/createFolder handler to reject
submission if useTeams().isLoading is true (or if the selected team is still
undefined) so you cannot submit during the loading window and only allow
selecting '__org_wide__' once teams have loaded.
In
`@services/platform/app/features/documents/components/document-team-tags-dialog.tsx`:
- Around line 52-53: The dialog currently assumes a single team by using
currentTeamIds?.[0] to set currentTeamId and always submits either [] or one ID
via selectedTeamId/setSelectedTeamId, which will drop extra team assignments for
legacy multi-team documents; update the component to detect when
currentTeamIds?.length > 1 (use the currentTeamIds array) and prevent the
single-select flow by blocking the dialog (e.g., show a non-editable warning
state or disable save) and avoid submitting changes in that case, or
alternatively trigger a migration flow before allowing edits; reference
currentTeamIds, currentTeamId, selectedTeamId, and setSelectedTeamId to
implement the guard and adjust the submit handler to no-op when multiple teams
exist.
In `@services/platform/app/features/documents/utils/filter-documents.ts`:
- Around line 22-29: The team filtering currently only checks
doc.teamIds/folder.teamIds and treats items with a singular teamId as org-wide;
update both the selectedTeamId and selectedTeamIds branches to also honor the
singular doc.teamId/folder.teamId when teamIds is not present. Replace
predicates like "!doc.teamIds?.length || doc.teamIds.includes(teamId)" with a
check that keeps the item if neither teamIds nor teamId exist (org-wide) OR
teamIds includes the selected id OR the singular teamId equals the selected id;
do the same for folder.teamId and for the selectedTeamIds branch (which should
check membership against an array of selected IDs using the same combined
logic).
In `@services/platform/app/hooks/use-zoom-pan.ts`:
- Around line 72-84: The ResizeObserver effect and the zoom-change handlers need
to revalidate the current pan after bounds change: when updating
containerDimsRef in the ResizeObserver callback (useEffect around containerRef),
call clampPan (or the same logic used to constrain pan) and apply the clamped
result to panRef / setPan so the image is immediately snapped into bounds;
likewise, in the zoom-out path where you compute a `next` pan (the handler that
calculates next after zoom), run that `next` through clampPan before setting it.
Update the useEffect (observer), the zoom compute block that produces `next`,
and the other similar effects (the blocks around lines 112-124 and 179-191) to
always clamp and set the pan after changing container/zoom bounds.
- Around line 192-223: The setZoom updater (setZoom((currentZoom) => {...})) is
impure: it calls e.preventDefault() and calls setPan inside the updater; move
all side-effects out so the updater only computes zoom. In the Arrow key handler
call e.preventDefault() immediately, read the current zoom from the stable
source (use the existing zoom state value or create/read a zoomRef), compute
dx/dy using PAN_KEY_STEP and then call setPan(...) directly (using
containerDimsRef.current to clamp pan as the current code does) and finally call
setZoom with a pure updater or value if zoom must change; ensure setZoom's
callback no longer invokes preventDefault or setPan and that pan clamping logic
still uses containerDimsRef and the current zoom value you read outside the
updater.
In
`@services/platform/convex/agent_tools/documents/__tests__/document_find_tool.test.ts`:
- Around line 287-290: The test and schema allow whitespace-only fileName;
update the documentFindArgs schema in
services/platform/convex/agent_tools/documents/document_find_tool.ts (symbol:
documentFindArgs) to trim and require non-empty values (e.g.,
.trim().min(1).optional() or convert blanks to undefined) so
blank/whitespace-only fileName is rejected or normalized, and update this test
(document_find_tool.test.ts) to assert documentFindArgs.parse({ fileName: ' '
}) throws (or results in undefined) to lock the behavior at the tool boundary.
In `@services/platform/convex/custom_agents/mutations.ts`:
- Around line 71-74: Existing persisted agents may still contain stale
document_list values; don't rely solely on write-time filtering. Add read-side
sanitization by applying filterValidToolNames (and checking/removing the
document_list field) wherever agents are loaded at runtime (e.g., agent
fetch/get functions or any deserialize routine) so invalid tool names are
dropped before use; additionally add a one-time backfill/migration that scans
stored agent records, sanitizes their tool name arrays using
filterValidToolNames and removes document_list for affected records, and
persists the cleaned records to eliminate stale data permanently.
In
`@services/platform/convex/documents/__tests__/list_documents_for_agent.test.ts`:
- Around line 82-103: The folders branch of getIterator currently returns every
folder in the org and ignores parent-scoped index constraints; update the
folders filter to mirror the documents logic by checking indexUsed for the
parent-id index (e.g., 'by_organizationId_and_parentId' or whatever name is used
in tests) and filter folders by a parentFilter (compare folder.parentId ===
parentFilter) in addition to organizationId, ensuring you reference getIterator,
table === 'folders', indexUsed and the parentFilter symbol so the mock enforces
the same parent-scoped search semantics as production.
In `@services/platform/convex/documents/list_documents_for_agent.ts`:
- Around line 390-405: The matchesPathSuffix function uses a too-permissive
threshold allowing up to 2 edits even for tiny segment names; tighten the
fuzzy-match rule by making the allowed edit distance depend on segment length
(e.g., require exact match for very short segments and allow at most 1 small
edit for short segments, otherwise use a small fraction like
Math.floor(seg.length * 0.15) with a minimum of 1). Update the threshold
calculation in matchesPathSuffix (where threshold is computed) so segments of
length <=2 are exact (threshold = 0), length 3–4 allow at most 1 edit, and
longer segments use the small-fraction rule, then use that threshold in the
existing levenshteinDistance check.
- Around line 345-349: The current block returns fuzzy leaf matches (leafIds →
resolvedPaths via buildFullPath(folderById)) even when the ancestor chain check
failed for a multi-segment request, which widens a specific path like
"contracts/2024" to unrelated leaf matches; change this branch so it only
returns leafIds when the original request was a single-segment lookup (i.e.,
detect the parsed path has only one segment or no separator), otherwise do not
return leafIds and instead return a not-found/suggestions response (or an empty
folderIds with resolved suggestion data); update the logic around leafIds,
buildFullPath, and folderById in this block to gate returning leaf matches by
the original request's segment count.
In `@services/platform/convex/folders/__tests__/mutations.test.ts`:
- Around line 467-815: Tests are not exercising the real mutation logic — they
call helpers like hasTeamAccess, teamIdsToFields, or compute effectiveTeamId
locally instead of invoking the production mutations; replace or supplement
these specs to call the real mutation functions (e.g., updateFolderTeams,
createFolder, and the document update/upload handlers) using the same mock
context (createMockMutationCtx) so the tests assert the actual behavior
(inheritance, cascade, and enforcement) rather than just helper return values;
locate usages in this file of hasTeamAccess/teamIdsToFields and update those
test cases to call the corresponding mutation entry points and assert DB
patches/gets or thrown authorization errors as appropriate.
- Around line 817-982: Tests currently only query for children but never
exercise the cascade deletion logic; update the specs to invoke the actual
deletion functions (call deleteFolder or deleteFolderContents from the module
under test using the test context created by createMockMutationCtx) for the
parent folder IDs you set up, then assert the folders and documents maps no
longer contain the deleted IDs and any scheduled-deletion records are
created/cleared as expected; reference the test utilities and symbols like
createMockMutationCtx, deleteFolder, deleteFolderContents, ctx.db.query, folders
and documents maps when locating where to add the delete invocation and
subsequent expect(...) assertions.
In `@services/platform/convex/folders/mutations.ts`:
- Around line 20-45: The recursive helpers (cascadeTeamToDescendants and the
similar helper at lines 48-75) are mutating every descendant without rechecking
permissions, allowing callers who can modify a parent to change descendants they
shouldn't; update both helpers to call the permission check hasTeamAccess (or
the equivalent access function used elsewhere) for each child folder and each
child document before calling ctx.db.patch or recursing — skip any descendant
for which hasTeamAccess returns false (or throw/return an error if that matches
your security policy), and ensure createFolder continues to enforce team-scoped
constraints by relying on these guarded helpers.
- Around line 177-182: The current check uses getUserTeamIds() to authorize
effectiveTeamId but does not verify the target team's organization; load the
team record (query the team collection by the target team _id) and compare its
organizationId to the folder's/operation's organization before accepting the
team, throwing an error if they differ; apply this same pattern for both the
effectiveTeamId branch (symbol: effectiveTeamId) and the other occurrence around
lines 337-341, mirroring the pattern used in addMember/removeMember (query
'team' by _id and compare organizationId) to ensure teams from other
organizations are rejected.
In `@services/platform/convex/lib/fuzzy_match.ts`:
- Around line 55-57: The levenshteinThreshold function currently returns
Math.max(2, Math.floor(len * 0.3)) which lets very short tokens (len 1-2) match
with up to 2 edits; change the logic in levenshteinThreshold to tighten
thresholds for short lengths—for example, return 0 for len === 1, 1 for len ===
2, 1 for len === 3-4, and otherwise use Math.max(2, Math.floor(len * 0.3))—so
that 1–2 character tokens cannot match with multiple edits while preserving
looser tolerance for longer tokens.
In `@services/platform/convex/lib/rate_limiter/index.ts`:
- Around line 169-173: Rename the rate-limit entry key 'agent:document-list' to
'agent:document-find' to match the renamed tool and avoid confusion, and make
the rate-limit algorithm consistent with TIER 5 by changing kind from 'fixed
window' to 'token bucket' (or, if you intentionally want fixed-window semantics,
move this rule under the TIER 3 block or add a one-line comment above the rule
explaining why 'fixed window' is preferred); update the entry where the object
keyed by 'agent:document-list' is defined and ensure any lookups referencing
that key are updated to 'agent:document-find'.
In `@services/platform/messages/en.json`:
- Around line 2348-2350: Update the localization entries for the team update
messages to use singular phrasing: modify the "updated" key value from "Teams
updated" to "Team updated" and the "updateFailed" key value from "Failed to
update teams" to "Failed to update team" (keys: "updated", "updateFailed" in
services/platform/messages/en.json) so messages match the single-team select UI.
---
Outside diff comments:
In `@services/platform/app/features/chat/components/chat-input.tsx`:
- Around line 171-186: The preview button's aria-label is too generic
(tChat('viewImage')) so screen readers can't distinguish attachments; update the
aria-label to include the attachment name (e.g., combine tChat('viewImage') with
attachment.fileName) in the button inside chat-input.tsx so each button
announces which file it opens; ensure the change is applied where the onClick
uses setPreviewImage and when rendering the <img> so the accessible name
reflects attachment.fileName and still falls back safely if fileName is missing.
In
`@services/platform/app/features/documents/components/document-row-actions.tsx`:
- Around line 140-145: The team-management action is incorrectly hidden for
folders due to the condition visible: !parentFolderTeamId; change this to only
suppress the action for files when the parent folder has a team (e.g., visible:
!(parentFolderTeamId && itemType === 'file') or equivalent) so folders still
show the teamTags action; update the same logic where entityType={itemType} is
passed (see the component that renders the action around the entityType/itemType
usage) to ensure folder items are allowed to open dialogs.open.teamTags while
files inherit the lock.
In `@services/platform/convex/documents/list_documents_for_agent.ts`:
- Around line 137-139: The filter only checks doc.teamId against args.teamId and
thus misses documents where the team is listed in secondary assignments
(doc.teamTags); update the conditional in list_documents_for_agent (around the
block using args.teamId and hasTeamAccess) to allow the document when doc.teamId
=== args.teamId OR (doc.teamTags && doc.teamTags.includes(args.teamId)),
otherwise continue, while keeping the existing fallback that uses
hasTeamAccess(teamIdSet) when args.teamId is not provided.
In `@services/platform/convex/documents/update_document.ts`:
- Around line 36-46: The current guard only runs when args.teamIds.length > 0,
letting callers pass teamIds: [] to bypass the checks; change the condition to
run whenever teamIds is provided (args.teamIds !== undefined) so you enforce the
userId requirement and the parent-folder inheritance check for empty arrays too.
Specifically, update the block that references args.teamIds, args.userId,
document.folderId and ctx.db.get so it triggers on args.teamIds !== undefined
(not args.teamIds.length > 0), throw if !args.userId, and still fetch the parent
folder and throw if folder?.teamId to block clearing inherited team fields.
In `@services/platform/convex/folders/mutations.ts`:
- Around line 146-196: The code collapses multi-team inheritance by setting
effectiveTeamId = parent.teamId and never persisting teamTags; fix by preserving
the parent's full team scope: when parent.teamId or parent.teamTags exist,
compute childTeamId (single-team inheritance) and childTeamTags (union of
parent.teamTags plus parent.teamId if present) and use those for both access
checks and the insert. Replace the later membership check that does
userTeamIds.includes(effectiveTeamId) with logic that checks access against the
full inherited scope (e.g., use hasTeamAccess-like logic or check intersection
between userTeamIds and childTeamTags/childTeamId), and include teamTags:
childTeamTags in the ctx.db.insert call (alongside teamId if still relevant) so
child folders retain the parent's multi-team scope; update references to
effectiveTeamId to use the new childTeamId/childTeamTags values and keep
checkDuplicateName and createdBy behavior unchanged.
🪄 Autofix (Beta)
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: fbd7931a-ecbf-4986-bd75-2e9352f38a9b
⛔ Files ignored due to path filters (1)
services/platform/convex/_generated/api.d.tsis excluded by!**/_generated/**
📒 Files selected for processing (41)
services/platform/app/components/ui/data-display/zoom-pan-viewer.test.tsxservices/platform/app/components/ui/data-display/zoom-pan-viewer.tsxservices/platform/app/features/chat/components/chat-input.tsxservices/platform/app/features/custom-agents/components/tool-selector.tsxservices/platform/app/features/documents/components/__tests__/document-team-tags-dialog.test.tsxservices/platform/app/features/documents/components/create-folder-dialog.tsxservices/platform/app/features/documents/components/document-row-actions.tsxservices/platform/app/features/documents/components/document-team-tags-dialog.tsxservices/platform/app/features/documents/components/documents-action-menu.tsxservices/platform/app/features/documents/components/documents-table.tsxservices/platform/app/features/documents/hooks/mutations.tsservices/platform/app/features/documents/hooks/queries.tsservices/platform/app/features/documents/hooks/use-documents-table-config.tsxservices/platform/app/features/documents/utils/__tests__/filter-documents.test.tsservices/platform/app/features/documents/utils/filter-documents.tsservices/platform/app/hooks/use-zoom-pan.tsservices/platform/convex/agent_tools/documents/__tests__/document_find_tool.test.tsservices/platform/convex/agent_tools/documents/document_find_tool.tsservices/platform/convex/agent_tools/documents/document_retrieve_tool.tsservices/platform/convex/agent_tools/documents/helpers/list_documents.tsservices/platform/convex/agent_tools/tool_names.tsservices/platform/convex/agent_tools/tool_registry.tsservices/platform/convex/custom_agents/config.tsservices/platform/convex/custom_agents/mutations.tsservices/platform/convex/documents/__tests__/list_documents_for_agent.test.tsservices/platform/convex/documents/get_documents.tsservices/platform/convex/documents/helpers.tsservices/platform/convex/documents/internal_queries.tsservices/platform/convex/documents/list_documents_for_agent.tsservices/platform/convex/documents/mutations.tsservices/platform/convex/documents/types.tsservices/platform/convex/documents/update_document.tsservices/platform/convex/documents/validators.tsservices/platform/convex/folders/__tests__/mutations.test.tsservices/platform/convex/folders/mutations.tsservices/platform/convex/folders/queries.tsservices/platform/convex/folders/schema.tsservices/platform/convex/lib/fuzzy_match.test.tsservices/platform/convex/lib/fuzzy_match.tsservices/platform/convex/lib/rate_limiter/index.tsservices/platform/messages/en.json
Comment on lines
+141
to
+146
| tabIndex={isZoomed ? 0 : -1} | ||
| className={cn( | ||
| 'flex flex-1 items-center justify-center overflow-hidden', | ||
| 'flex flex-1 items-center justify-center overflow-hidden outline-none', | ||
| isZoomed ? 'cursor-grab' : 'cursor-default', | ||
| isDragging && 'cursor-grabbing', | ||
| 'focus-visible:ring-ring focus-visible:ring-2 focus-visible:ring-offset-2', |
There was a problem hiding this comment.
Keep the viewer in the tab order if the keyboard shortcuts are part of the UX.
tabIndex={isZoomed ? 0 : -1} makes the shortcut surface unreachable from the keyboard at the initial 100% state, so +/-/0 only become available after some other interaction path is used first. If those shortcuts are intended as a first-class control, the container should stay tabbable and rely on the focus-visible ring for discoverability.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/components/ui/data-display/zoom-pan-viewer.tsx` around
lines 141 - 146, The container currently uses tabIndex={isZoomed ? 0 : -1},
which removes it from the tab order until zoomed; change this to always be
tabbable (e.g., tabIndex={0}) so keyboard shortcuts (+/-/0) are reachable at
100% load; keep the existing focus-visible:ring classes for discoverability and
retain the isZoomed-based cursor logic in className (do not modify isDragging or
cursor-grab/grabbing handling).
| const { t: tDocuments } = useT('documents'); | ||
| const { toast } = useToast(); | ||
| const { mutateAsync: createFolder } = useCreateFolder(); | ||
| const { teams } = useTeams(); |
There was a problem hiding this comment.
Don’t let the form default to org-wide before useTeams() finishes loading.
Line 45 drops isLoading, Line 61 initializes the form to __org_wide__, and Lines 124-136 hide the selector until the teams query resolves. That means a user can submit this dialog during the loading window and Lines 69-75 will create an org-wide folder even in an org that actually has team options. Please block submission or show a loading state until useTeams() settles.
Also applies to: 61-61, 69-75, 124-136
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/features/documents/components/create-folder-dialog.tsx`
at line 45, The form currently defaults to '__org_wide__' before useTeams()
finishes, causing org-wide folders to be created during the teams loading
window; change the form so it does not set a default team value until useTeams()
settles: read isLoading and teams from useTeams(), initialize the team field to
null/undefined (not '__org_wide__') while isLoading is true, and disable the
submit button (or return a loading state) until isLoading is false and teams are
known; additionally, guard the onSubmit/createFolder handler to reject
submission if useTeams().isLoading is true (or if the selected team is still
undefined) so you cannot submit during the loading window and only allow
selecting '__org_wide__' once teams have loaded.
Comment on lines
+52
to
+53
| const currentTeamId = currentTeamIds?.[0] ?? ORG_WIDE_VALUE; | ||
| const [selectedTeamId, setSelectedTeamId] = useState(currentTeamId); |
There was a problem hiding this comment.
Guard the single-select path against existing multi-team assignments.
Line 52 only keeps currentTeamIds[0], and Lines 72-75 always submit either [] or a single team ID. The document update path still supports multi-team state, so if any legacy document is shared with multiple teams, editing it here will silently drop the extra assignments. Please block this dialog when currentTeamIds.length > 1 or migrate those records before shipping the single-select UI.
Also applies to: 72-75
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/app/features/documents/components/document-team-tags-dialog.tsx`
around lines 52 - 53, The dialog currently assumes a single team by using
currentTeamIds?.[0] to set currentTeamId and always submits either [] or one ID
via selectedTeamId/setSelectedTeamId, which will drop extra team assignments for
legacy multi-team documents; update the component to detect when
currentTeamIds?.length > 1 (use the currentTeamIds array) and prevent the
single-select flow by blocking the dialog (e.g., show a non-editable warning
state or disable save) and avoid submitting changes in that case, or
alternatively trigger a migration flow before allowing edits; reference
currentTeamIds, currentTeamId, selectedTeamId, and setSelectedTeamId to
implement the guard and adjust the submit handler to no-op when multiple teams
exist.
Comment on lines
+22
to
+29
| if (options.selectedTeamId) { | ||
| const teamId = options.selectedTeamId; | ||
| filtered = filtered.filter( | ||
| (doc) => !doc.teamIds?.length || doc.teamIds.includes(teamId), | ||
| ); | ||
| filteredFolders = filteredFolders.filter( | ||
| (folder) => !folder.teamIds?.length || folder.teamIds.includes(teamId), | ||
| ); |
There was a problem hiding this comment.
Honor teamId when teamIds is not populated.
DocumentItem still carries both shapes, but both team filters read only teamIds. Any item coming through with just teamId is treated as org-wide in the selectedTeamId branch and filtered out entirely in the selectedTeamIds branch.
♻️ Proposed fix
+function getItemTeamIds(item: DocumentItem): string[] {
+ return item.teamIds?.length ? item.teamIds : item.teamId ? [item.teamId] : [];
+}
+
export function filterDocumentResults(
documents: DocumentItem[],
folders: DocumentItem[],
options: DocumentFilterOptions,
): DocumentItem[] {
@@
if (options.selectedTeamId) {
const teamId = options.selectedTeamId;
filtered = filtered.filter(
- (doc) => !doc.teamIds?.length || doc.teamIds.includes(teamId),
+ (doc) => {
+ const teamIds = getItemTeamIds(doc);
+ return teamIds.length === 0 || teamIds.includes(teamId);
+ },
);
filteredFolders = filteredFolders.filter(
- (folder) => !folder.teamIds?.length || folder.teamIds.includes(teamId),
+ (folder) => {
+ const teamIds = getItemTeamIds(folder);
+ return teamIds.length === 0 || teamIds.includes(teamId);
+ },
);
}
@@
if (options.selectedTeamIds.length > 0) {
const teamIdSet = new Set(options.selectedTeamIds);
- filtered = filtered.filter((doc) =>
- doc.teamIds?.some((id) => teamIdSet.has(id)),
- );
- filteredFolders = filteredFolders.filter((folder) =>
- folder.teamIds?.some((id) => teamIdSet.has(id)),
- );
+ filtered = filtered.filter((doc) =>
+ getItemTeamIds(doc).some((id) => teamIdSet.has(id)),
+ );
+ filteredFolders = filteredFolders.filter((folder) =>
+ getItemTeamIds(folder).some((id) => teamIdSet.has(id)),
+ );
}Also applies to: 51-58
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/features/documents/utils/filter-documents.ts` around
lines 22 - 29, The team filtering currently only checks
doc.teamIds/folder.teamIds and treats items with a singular teamId as org-wide;
update both the selectedTeamId and selectedTeamIds branches to also honor the
singular doc.teamId/folder.teamId when teamIds is not present. Replace
predicates like "!doc.teamIds?.length || doc.teamIds.includes(teamId)" with a
check that keeps the item if neither teamIds nor teamId exist (org-wide) OR
teamIds includes the selected id OR the singular teamId equals the selected id;
do the same for folder.teamId and for the selectedTeamIds branch (which should
check membership against an array of selected IDs using the same combined
logic).
Comment on lines
+72
to
+84
| // Cache container dimensions via ResizeObserver for pan clamping | ||
| useEffect(() => { | ||
| const el = containerRef.current; | ||
| if (!el) return; | ||
| const observer = new ResizeObserver(([entry]) => { | ||
| containerDimsRef.current = { | ||
| width: entry.contentRect.width, | ||
| height: entry.contentRect.height, | ||
| }; | ||
| }); | ||
| observer.observe(el); | ||
| return () => observer.disconnect(); | ||
| }, []); |
There was a problem hiding this comment.
Clamp the existing pan after bounds shrink.
Line 76 updates the stored container bounds, but nothing re-validates the already-stored pan. After panning to an edge, a resize or the new keyboard zoom-out path can leave the image outside the reduced limits until the next drag/key move snaps it back. Reapply clampPan when the observer fires and when a zoom-out computes next.
Also applies to: 112-124, 179-191
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/hooks/use-zoom-pan.ts` around lines 72 - 84, The
ResizeObserver effect and the zoom-change handlers need to revalidate the
current pan after bounds change: when updating containerDimsRef in the
ResizeObserver callback (useEffect around containerRef), call clampPan (or the
same logic used to constrain pan) and apply the clamped result to panRef /
setPan so the image is immediately snapped into bounds; likewise, in the
zoom-out path where you compute a `next` pan (the handler that calculates next
after zoom), run that `next` through clampPan before setting it. Update the
useEffect (observer), the zoom compute block that produces `next`, and the other
similar effects (the blocks around lines 112-124 and 179-191) to always clamp
and set the pan after changing container/zoom bounds.
Comment on lines
+20
to
+45
| async function cascadeTeamToDescendants( | ||
| ctx: MutationCtx, | ||
| folderId: Id<'folders'>, | ||
| organizationId: string, | ||
| fields: TeamFields, | ||
| ) { | ||
| const childFolders = ctx.db | ||
| .query('folders') | ||
| .withIndex('by_org_parent_name', (q) => | ||
| q.eq('organizationId', organizationId).eq('parentId', folderId), | ||
| ); | ||
|
|
||
| for await (const child of childFolders) { | ||
| await ctx.db.patch(child._id, fields); | ||
| await cascadeTeamToDescendants(ctx, child._id, organizationId, fields); | ||
| } | ||
|
|
||
| const childDocs = ctx.db | ||
| .query('documents') | ||
| .withIndex('by_organizationId_and_folderId', (q) => | ||
| q.eq('organizationId', organizationId).eq('folderId', folderId), | ||
| ); | ||
|
|
||
| for await (const doc of childDocs) { | ||
| await ctx.db.patch(doc._id, fields); | ||
| } |
There was a problem hiding this comment.
These recursive helpers bypass descendant team checks.
createFolder() still allows team-scoped children under an org-wide parent, but both helpers mutate every descendant without rechecking hasTeamAccess. A caller who can touch the parent can therefore retag or delete descendants they cannot access directly.
Also applies to: 48-75
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/convex/folders/mutations.ts` around lines 20 - 45, The
recursive helpers (cascadeTeamToDescendants and the similar helper at lines
48-75) are mutating every descendant without rechecking permissions, allowing
callers who can modify a parent to change descendants they shouldn't; update
both helpers to call the permission check hasTeamAccess (or the equivalent
access function used elsewhere) for each child folder and each child document
before calling ctx.db.patch or recursing — skip any descendant for which
hasTeamAccess returns false (or throw/return an error if that matches your
security policy), and ensure createFolder continues to enforce team-scoped
constraints by relying on these guarded helpers.
Comment on lines
+177
to
182
| if (effectiveTeamId) { | ||
| const userTeamIds = await getUserTeamIds(ctx, String(authUser._id)); | ||
| if (!userTeamIds.includes(args.teamId)) { | ||
| if (!userTeamIds.includes(effectiveTeamId)) { | ||
| throw new Error('Cannot create folder in a team you do not belong to'); | ||
| } | ||
| } |
There was a problem hiding this comment.
Reject team IDs that belong to a different organization.
getUserTeamIds() returns all of the caller’s team memberships, so these checks allow a user in multiple orgs to attach a folder in org A to a team from org B. Please load each target team and verify its organizationId before accepting it in either flow.
Based on learnings: In services/platform/convex/team_members/mutations.ts, addMember validates the target team belongs to the provided organization (findOne on 'team' by _id, compare organizationId) before checking/creating membership, matching the pattern already enforced in removeMember.
Also applies to: 337-341
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/convex/folders/mutations.ts` around lines 177 - 182, The
current check uses getUserTeamIds() to authorize effectiveTeamId but does not
verify the target team's organization; load the team record (query the team
collection by the target team _id) and compare its organizationId to the
folder's/operation's organization before accepting the team, throwing an error
if they differ; apply this same pattern for both the effectiveTeamId branch
(symbol: effectiveTeamId) and the other occurrence around lines 337-341,
mirroring the pattern used in addMember/removeMember (query 'team' by _id and
compare organizationId) to ensure teams from other organizations are rejected.
Comment on lines
+55
to
+57
| function levenshteinThreshold(len: number): number { | ||
| return Math.max(2, Math.floor(len * 0.3)); | ||
| } |
There was a problem hiding this comment.
Tighten the edit-distance threshold for short tokens.
Line 56 allows up to 2 edits even for 1-2 character inputs, so short queries like hr, it, or q1 can fuzzy-match unrelated names. That makes both title search and leaf-folder matching much noisier than intended.
🛠️ Suggested fix
function levenshteinThreshold(len: number): number {
- return Math.max(2, Math.floor(len * 0.3));
+ if (len <= 2) return 0;
+ if (len <= 4) return 1;
+ return Math.max(1, Math.floor(len * 0.3));
}📝 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.
Suggested change
| function levenshteinThreshold(len: number): number { | |
| return Math.max(2, Math.floor(len * 0.3)); | |
| } | |
| function levenshteinThreshold(len: number): number { | |
| if (len <= 2) return 0; | |
| if (len <= 4) return 1; | |
| return Math.max(1, Math.floor(len * 0.3)); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/convex/lib/fuzzy_match.ts` around lines 55 - 57, The
levenshteinThreshold function currently returns Math.max(2, Math.floor(len *
0.3)) which lets very short tokens (len 1-2) match with up to 2 edits; change
the logic in levenshteinThreshold to tighten thresholds for short lengths—for
example, return 0 for len === 1, 1 for len === 2, 1 for len === 3-4, and
otherwise use Math.max(2, Math.floor(len * 0.3))—so that 1–2 character tokens
cannot match with multiple edits while preserving looser tolerance for longer
tokens.
Comment on lines
+169
to
+173
| 'agent:document-list': { | ||
| kind: 'fixed window', | ||
| rate: 30, | ||
| period: MINUTE, | ||
| }, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Rate limit key name inconsistent with renamed tool.
The PR renamed the tool from document_list to document_find, but the rate limit key uses agent:document-list. Consider renaming to agent:document-find for consistency with the tool name, which will reduce confusion during future maintenance.
Additionally, this rule uses fixed window while placed under TIER 5 which is labeled "Workflow Operations (Token Bucket)". If fixed window is intentional for this operation, consider either:
- Moving to TIER 3 (File & Folder Operations) which already uses fixed window, or
- Adding a brief comment explaining why fixed window is preferred here
Suggested rename for consistency
- 'agent:document-list': {
+ 'agent:document-find': {
kind: 'fixed window',
rate: 30,
period: MINUTE,
},📝 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.
Suggested change
| 'agent:document-list': { | |
| kind: 'fixed window', | |
| rate: 30, | |
| period: MINUTE, | |
| }, | |
| 'agent:document-find': { | |
| kind: 'fixed window', | |
| rate: 30, | |
| period: MINUTE, | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/convex/lib/rate_limiter/index.ts` around lines 169 - 173,
Rename the rate-limit entry key 'agent:document-list' to 'agent:document-find'
to match the renamed tool and avoid confusion, and make the rate-limit algorithm
consistent with TIER 5 by changing kind from 'fixed window' to 'token bucket'
(or, if you intentionally want fixed-window semantics, move this rule under the
TIER 3 block or add a one-line comment above the rule explaining why 'fixed
window' is preferred); update the entry where the object keyed by
'agent:document-list' is defined and ensure any lookups referencing that key are
updated to 'agent:document-find'.
Comment on lines
+2348
to
+2350
| "team": "Team", | ||
| "updated": "Teams updated", | ||
| "updateFailed": "Failed to update teams", |
There was a problem hiding this comment.
Inconsistent pluralization in team update messages.
The PR replaces the multi-team checkbox with a single-team select dropdown, yet the success/error messages use plural forms ("Teams updated", "Failed to update teams"). Since the user is now selecting a single team, these should be singular:
✏️ Suggested fix
- "updated": "Teams updated",
- "updateFailed": "Failed to update teams",
+ "updated": "Team updated",
+ "updateFailed": "Failed to update team",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/messages/en.json` around lines 2348 - 2350, Update the
localization entries for the team update messages to use singular phrasing:
modify the "updated" key value from "Teams updated" to "Team updated" and the
"updateFailed" key value from "Failed to update teams" to "Failed to update
team" (keys: "updated", "updateFailed" in services/platform/messages/en.json) so
messages match the single-team select UI.
yannickmonney
pushed a commit
that referenced
this pull request
Apr 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
document_findagent tool (renamed fromdocument_list) so agents can search documents by partial/approximate file namesChanges
fuzzy_matchutility with scoring algorithm, integrated intolist_documents_for_agentdocument_list→document_findacross agent tools, registry, and custom agent configsTest plan
fuzzy_match.test.ts)document_find_tool.test.ts)list_documents_for_agentwith fuzzy search scenariosfilter-documents.test.ts)🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Accessibility