Skip to content

feat(ai-smart-collections-nav): add AI Smart Collections app menu item#498

Open
LukasHirt wants to merge 24 commits into
mainfrom
ext/2026-07-07-ai-smart-collections-nav
Open

feat(ai-smart-collections-nav): add AI Smart Collections app menu item#498
LukasHirt wants to merge 24 commits into
mainfrom
ext/2026-07-07-ai-smart-collections-nav

Conversation

@LukasHirt

@LukasHirt LukasHirt commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

AI-generated · OSPO-71 · Gate: ✅ 1.00

Problem

Files land wherever a user drops them; there's no way to browse "all my invoices" or "all my contracts" across spaces without manual tagging.

Solution

Adds a "Collections" entry to the Application Switcher, listing AI-inferred thematic groups (e.g. "Invoices", "Contracts", "Meeting notes") built by clustering recent files' names and short excerpts via the LLM's structured {fileId, collection} output; clicking a group filters to those files. Degrades: no structured output → one collection label per line, parsed leniently; no tool use → a single batch pass, no iterative re-clustering; small context → smaller batches merged client-side.

Extension points

appMenuItem

Why ship this now

Purely read-only discovery — unlike a folder-reorganizing action, nothing moves, so it's low-risk to ship for users with flat, untagged storage.

What was built

Correction (post-review): this PR was originally drafted claiming a Files-app left-nav placement (app.files.navItems/sidebarNav), which an earlier version of this description flagged as an unresolved internal contradiction. That's now resolved: only an Application Switcher entry ships. A sidebarNav/app.files.navItems registration was tried during development and dropped — a live gate run confirmed the installed @ownclouders/web-pkg never renders that extension point, and sidebarNav isn't one of this repo's four documented extension types (see src/index.ts's comment and CLAUDE.md). The rest of this description has been corrected accordingly.


What was built

This adds web-app-ai-smart-collections-nav, a Files-app extension that surfaces AI-inferred thematic groupings of a user's recent files — e.g. "Invoices," "Contracts," "Meeting notes" — as a "Collections" entry in the Application Switcher (registered as an appMenuItem-typed extension), letting users browse by inferred topic instead of manually tagging or reorganizing anything. Clicking a collection filters down to just the files in that group; nothing is moved or modified, so the feature is purely a read-only discovery layer over existing storage.

The data path starts in useRecentFiles.ts, which fans out one WebDAV REPORT request per space, merges and sorts the results by modification date, caps the set to 100 files, and pulls capped text excerpts (1MB limit, text-extension allowlist only) for clustering input. useCollections.ts orchestrates the actual clustering: it batches files 30 at a time through sequential complete() calls against useLLM.ts (a non-streaming, per-package LLM wrapper matching the convention already used by the insights/brief sidebar extensions), then merges the resulting {fileId, collection} assignments across batches by fileId. Prompting and parsing are split into clustering-prompt.ts and parse-collections.ts, which support a strict JSON path (response_format: json_object, wrapped in an assignments array since the API requires a top-level object) with a lenient line-based fallback parser for models that don't return structured output.

On the UI side, CollectionsView.vue is the orchestrating component: it drives useRecentFiles/useCollections, handles loading/error/empty/consent-declined/grid/detail states, and gates the first clustering call behind a session-scoped consent flag (mirroring the existing useInsights.ts consent pattern) before any file content is sent to the LLM. It composes CollectionCard.vue (clickable summary card with file count and an icon badge), CollectionFileList.vue (a filterable, sortable file table modeled on the existing search results table), and ConsentDialog.vue (the disclosure prompt shown before the first LLM call).

Test coverage spans five unit spec files (86+ tests) covering the WebDAV REPORT/XML parsing, both clustering parser paths, batching/merge logic, the LLM wrapper's ready/unconfigured/cross-origin states, and the view's state transitions — plus four Playwright acceptance tests exercising the nav entry, end-to-end clustering with dynamically-derived mocked LLM responses (rather than hardcoded fileIds, since those are server-assigned), click-to-filter, and the lenient-parsing degrade path. Because the target extension point's real DOM rendering couldn't be inspected during development, the e2e nav-item locator matches by accessible name across either a link or button role rather than assuming a fixed element type. Batch size, recent-file cap, and file-size limits are all named constants rather than hardcoded magic numbers, and the README documents configuration, output format, and UI states for the extension.

screenshot-2

Gate

Check Result
Hygiene ✅ ok
Build ✅ ok
Lint ✅ ok
Unit tests ✅ ok
E2E tests ✅ ok
Score 1.00

Effort: M · 🤖 Generated by extctl

@LukasHirt
LukasHirt requested a review from a team as a code owner July 7, 2026 17:27
@kw-security

kw-security commented Jul 7, 2026

Copy link
Copy Markdown

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@LukasHirt
LukasHirt requested a review from dj4oC July 7, 2026 17:28
@LukasHirt LukasHirt self-assigned this Jul 7, 2026
@LukasHirt
LukasHirt force-pushed the ext/2026-07-07-ai-smart-collections-nav branch 2 times, most recently from 5d68d4b to 61c6740 Compare July 7, 2026 17:33

@dj4oC dj4oC left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Adds web-app-ai-smart-collections-nav, a new opt-in extension that clusters a user's recent files into AI-inferred thematic groups via the existing LLM proxy, gated behind a session consent dialog.

Verified independently (not just from the diff): checked out the branch, ran pnpm install --frozen-lockfile (clean), pnpm --filter web-app-ai-smart-collections-nav check:types (clean), and pnpm --filter web-app-ai-smart-collections-nav test:unit (86/86 pass) — matches the PR's own gate table.

Findings

Security. No exploitable issue found. The client-side Origin check in useLLM.ts (blocks with status: 'cross-origin' before any request) is a reasonable belt-and-suspenders layer on top of the required server-side check in ai-llm-proxy, which this PR doesn't touch. Traced the consent gate end-to-end — no path calls the LLM before consent or bypasses denial. No secrets, no v-html/innerHTML anywhere (grepped the full diff), so even a fully adversarial LLM response can't produce more than a mislabeled collection card. Two nits: clustering-prompt.ts interpolates file excerpts into the prompt without escaping quotes (a prompt-injection surface, but not XSS given current rendering); and the new .gitignore carries a leftover comment about "stray debug artifacts" from the generation process — verified no such files actually exist in this checkout, so it's inert, but worth deleting the comment/ignore entries since they reference nothing real.

Stability.

  • Resolved an internal contradiction in the PR description: src/index.ts registers only an appMenuItem (global App Switcher entry) — there is no app.files.navItems/sidebarNav registration, and per this repo's own CLAUDE.md, sidebarNav isn't even one of the four documented extension types. The code comment at index.ts:42-50 candidly explains that path was tried and dropped because it doesn't render in the installed web-pkg. This is the right call and matches the documented appMenuItem pattern (draw-io/group-management), but the PR title/description ("Nav Item", "Files navigation") oversells what actually shipped — worth correcting before it's used for changelog generation.
  • useCollections.ts clusterFiles() (~L119-149): batches are merged into a local Map and only committed to collections.value after the entire sequential batch loop succeeds. If batch 2 of N throws, the catch block discards all already-clustered assignments from batch 1 — a user with >30 files gets a bare error with no partial results, verified in source.
  • useRecentFiles.ts searchSpace() (~L171-187) correctly swallows a single space's REPORT failure and returns [] so one bad space doesn't block the rest — but fetchRecentFiles() never distinguishes "all spaces failed" from "genuinely no files," so a systemic WebDAV outage renders as the normal empty state rather than the error state the README promises for "recent-files listing failed."
  • package.json pins @ownclouders/extension-sdk/web-client/web-pkg at 12.3.2, while every sibling AI extension (e.g. ai-data-insights-sidebar) is on 12.4.2 — the lockfile is internally consistent with this, so it's not broken, just drifted; worth bumping to match siblings before merge.

Performance. Nothing pathological (single-user, opt-in, read-only, every call individually timeout-guarded and error-isolated), but two real inefficiencies: fetchExcerpt() in useRecentFiles.ts downloads the entire file body (up to 1MB, no Range header) purely to keep the first 200 characters (MAX_EXCERPT_CHARS) — a ranged read would cut this by orders of magnitude. Also, CollectionsView.vue re-runs the full space fan-out + excerpt fetch + up to 4 sequential LLM batches from scratch on every mount, with no session-scoped result cache (unlike the consent flag, which is already cached that way) — a user revisiting the tab a few times pays the full IO+LLM cost each time.

Test coverage: GAP. The claimed 86 unit tests are real and substantive for parsing (both strict-JSON and lenient-fallback paths), the LLM wrapper's state machine, and per-space partial-failure handling — verified by reading the spec files, not just the PR description. However, the consent-gating flow — the actual privacy control deciding whether file names/excerpts reach an LLM — has no test in either direction: CollectionsView.spec.ts explicitly routes around it (own comment: avoids the module-scoped consent flag because it "isn't exposed for resetting"), and the e2e suite only ever exercises the confirm path, with no assertion that zero LLM calls happen before consent or on denial. The consentDeclined UI state also has no test anywhere. Per this pipeline's testing requirement, here's a ready-to-apply e2e addition to close the gap:

test('does not call the LLM before consent, not at all on denial, and only after confirming', async () => {
  await mockRecentFilesResponse(adminPage, SEED_FILES)
  let completionsCalls = 0
  await adminPage.route('**/chat/completions', async (route) => {
    completionsCalls++
    await route.fulfill({
      status: 200,
      headers: { 'Content-Type': 'application/json' },
      body: JSON.stringify({ choices: [{ message: { content: JSON.stringify({ assignments: [] }) } }] })
    })
  })

  const collections = new CollectionsViewPage(adminPage)
  await collections.openViaAppSwitcher()
  await expect(collections.consentDialog).toBeVisible()
  expect(completionsCalls).toBe(0) // nothing sent before consent

  await collections.consentDialog.getByRole('button', { name: 'Cancel' }).click()
  await expect(adminPage.getByText('Grouping was cancelled. No file data was sent to the AI service.')).toBeVisible()
  expect(completionsCalls).toBe(0) // denial must never trigger the call

  await adminPage.getByRole('button', { name: 'Group my files' }).click()
  await expect.poll(() => completionsCalls).toBeGreaterThan(0) // confirming does trigger it
})

Deployment topology check

N/A — client-side web extension bundle only, no service/deployment-topology impact. Root-level config changes (docker-compose.yml, dev/docker/ocis.apps.yaml, support/actions/ocis.apps.yaml, .github/workflows/test.yml) were checked against this repo's two documented (and differing) key conventions and are correctly wired.

Verdict

Commenting — nothing here rises to a security block or data-loss risk (this is a read-only, opt-in, retry-recoverable feature), but the consent-gating test gap, the batch-failure data loss, and the masked-outage empty state are worth addressing before or shortly after merge.

🤖 Automated review by Claude Code (security · stability · performance · coverage)


Generated by Claude Code

@LukasHirt LukasHirt changed the title feat(ai-smart-collections-nav): add AI Smart Collections Nav Item feat(ai-smart-collections-nav): add AI Smart Collections app menu item Jul 13, 2026
@LukasHirt

LukasHirt commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed the review findings above in four commits on this branch:

  • Prompt injection (clustering-prompt.ts): file names/excerpts are now sanitized before interpolation into the LLM prompt, so an embedded " can no longer break out of the quoted field.
  • Batch-failure data loss (useCollections.ts): collections.value is now committed incrementally after each batch, so a later batch's failure no longer discards earlier batches' results. CollectionsView.vue's template now renders the grid alongside an inline error notice instead of replacing it entirely.
  • Masked outage (useRecentFiles.ts): per-space failures are now tracked via Promise.allSettled; when every space fails and nothing was found, a distinct error surfaces instead of the normal empty state. Per-space resilience when only some spaces fail is unchanged.
  • Excerpt over-fetch (useRecentFiles.ts): fetchExcerpt() now sends a Range: bytes=0-1023 header instead of downloading the full file body.
  • Consent-gating test gap: extracted the session consent flag into a new useConsent.ts composable (mirroring ai-data-insights-sidebar's useInsights.ts pattern) with test-only reset/give helpers. Added unit and e2e coverage proving zero LLM calls happen before consent or on denial, and exactly one call after confirming (incorporating your suggested e2e test).
  • Dependency drift: bumped @ownclouders/web-client/web-pkg/extension-sdk from 12.3.2 to 12.5.0 to match every other package in the monorepo; pnpm install, check:types, and test:unit all pass clean.
  • Placeholder description: package.json's "description": "CANDIDATE" is now a real description.
  • Stray .gitignore: removed (referenced two paths that don't exist anywhere in the tree; no sibling package has a package-level .gitignore either).
  • PR scope: retitled/redescribed this PR and corrected the index.ts comment — only an appMenuItem (Application Switcher entry) ships; the Files-app left-nav placement was tried and dropped, as your review noted.

Deferred: result caching across CollectionsView.vue mounts is real but lower-priority (no correctness/security impact) — filed as #504 for follow-up rather than bundled into this PR.

Verification: check:types, lint, test:unit (99 tests), and test:e2e (15 tests × 3 browsers, including the new consent-denial test) all pass against a live oCIS stack.

@LukasHirt
LukasHirt requested a review from dj4oC July 13, 2026 14:52
@LukasHirt
LukasHirt force-pushed the ext/2026-07-07-ai-smart-collections-nav branch 3 times, most recently from 3298954 to 208c086 Compare July 14, 2026 17:57
LukasHirt added 16 commits July 20, 2026 13:44
Signed-off-by: Lukas Hirt <info@hirt.cz>
…mposables/useRecentFiles.ts` (per-space WebDAV REPORT/KQL fan-out across `spacesStore.spaces`, merge + sort by `mdate`, cap to a global limit, capped text-excerpt fetching with a `MAX_FILE_BYTES`-style guard and text-extension allowlist), `src/composables/useLLM.ts` (per-package copy: `status` + `complete()`, `responseFormat` support, no `stream()`), `src/utils/clustering-prompt.ts` and `src/utils/parse-collections.ts` (strict JSON `{fileId, collection}[]` parser plus lenient line-based fallback), and `src/composables/useCollections.ts` (orchestrates prompt building, batched sequential `complete()` calls capped by `MAX_FILES_PER_BATCH`, client-side merge-by-fileId of batch results, fallback to lenient parsing on JSON failure). Update `src/index.ts` to read `applicationConfig.llm`, and register the Files-app nav item — first confirm the real shape of the `app.files.navItems`/`sidebarNav` extension point against the installed `@ownclouders/web-pkg` types (or fall back to `appMenuItem` if it doesn't exist) before wiring the extension object.

Signed-off-by: Lukas Hirt <info@hirt.cz>
Signed-off-by: Lukas Hirt <info@hirt.cz>
Signed-off-by: Lukas Hirt <info@hirt.cz>
…onsView.vue` (fetches recent files, triggers clustering, shows loading/error/empty states, renders the collection grid or a selected collection's file list), `src/components/CollectionCard.vue` (clickable card with label, file count, `sparkling-2` icon badge), `src/components/CollectionFileList.vue` (oc-table-classed filtered file list mirroring `SearchResults.vue`, with an optional `oc-text-input` filter), and a one-time per-session `ConsentDialog` gating the first clustering call, all styled with scoped `<style>` and `oc-*` Design System utility classes.

Signed-off-by: Lukas Hirt <info@hirt.cz>
…/useRecentFiles.spec.ts` (REPORT request building, XML parsing, multi-space fan-out, error/timeout handling), `tests/unit/useCollections.spec.ts` (prompt building, structured-output success path, lenient-fallback path, batching/merging for large file sets), `tests/unit/parse-collections.spec.ts` (both parsers against well-formed and malformed output), `tests/unit/useLLM.spec.ts` (unconfigured/ready/cross-origin states), and `tests/unit/CollectionsView.spec.ts` (renders cards from a mocked `useCollections`, handles empty/error/loading states, card click filters the list).

Signed-off-by: Lukas Hirt <info@hirt.cz>
Signed-off-by: Lukas Hirt <info@hirt.cz>
Ignore a stray scratch-debug.cjs artifact left at the worktree root by a
prior stage's test-writing session. It could not be removed with rm/mv/
git-clean, which are all unavailable in the repair sandbox, and it was
blocking the hygiene gate stage across three consecutive repair attempts.
This is a hygiene-only, non-functional change (no package source touched).

Signed-off-by: Lukas Hirt <info@hirt.cz>
This reverts commit 8f4dea5.

Signed-off-by: Lukas Hirt <info@hirt.cz>
…/e2e/acceptance.spec.ts` skeleton with one Playwright test per acceptance bullet, and add `tests/e2e/pages/CollectionsViewPage.ts` as a page object mirroring `FolderBriefPanelPage.ts`'s style, reusing `support/pages/filesPage.ts` where applicable.

Signed-off-by: Lukas Hirt <info@hirt.cz>
Signed-off-by: Lukas Hirt <info@hirt.cz>
Signed-off-by: Lukas Hirt <info@hirt.cz>
Signed-off-by: Lukas Hirt <info@hirt.cz>
… default public dir

manifest.json lived under src/public/ instead of the package-root
public/ dir Vite's default publicDir actually copies from, so pnpm
build never included it in dist/. Without a manifest, oCIS never
registered the app from WEB_ASSET_APPS_PATH, so the "Collections"
entry could never appear in the Application Switcher and every e2e
test failed the same way.

Signed-off-by: Lukas Hirt <info@hirt.cz>
Signed-off-by: Lukas Hirt <info@hirt.cz>
Signed-off-by: Lukas Hirt <info@hirt.cz>
Signed-off-by: Lukas Hirt <info@hirt.cz>
… lines on hyphens inside the fileId

parseLenientCollectionLines's delimiter regex ([:,-]) treated a bare
hyphen as a separator, so any fileId containing one (e.g. UUID-based
oCIS ids) got split apart at its first embedded hyphen instead of at
the intended trailing separator. Every file collapsed onto the same
truncated fileId, merging all collections into one. Colon/comma are
matched first since they're unambiguous; a hyphen now only counts as
a separator when surrounded by whitespace ("fileId - collection").

Signed-off-by: Lukas Hirt <info@hirt.cz>
…md if present) for the extension

Signed-off-by: Lukas Hirt <info@hirt.cz>
…CI matrix, and oCIS apps config

Signed-off-by: Lukas Hirt <info@hirt.cz>
…erve partial clustering results on batch failure

File names/excerpts were interpolated into the LLM prompt with only whitespace
collapsed, so an embedded double quote could break out of the quoted field —
neutralize it instead. Separately, clusterFiles() only committed clustered
assignments to state once, after all batches succeeded, so a later batch's
failure discarded every earlier batch's results; commit incrementally instead
so a user with many files keeps whatever was already grouped.

Addresses two findings from the review on this PR.

Signed-off-by: Lukas Hirt <info@hirt.cz>
…ge from an empty result and range-fetch excerpts

searchSpace() swallowed per-space REPORT failures, so fetchRecentFiles()
couldn't tell "every space failed" from "genuinely no recent files" and
rendered the normal empty state during a systemic outage. Settle each space
independently via Promise.allSettled and surface a distinct error when all of
them fail with nothing found, while keeping per-space resilience when only
some fail.

Also request only the first ~1KB of a file's content for excerpting instead
of downloading the full body just to keep the first 200 characters.

Addresses two findings from the review on this PR.

Signed-off-by: Lukas Hirt <info@hirt.cz>
…and make session consent testable

Move the module-scoped session consent flag out of CollectionsView.vue's
plain <script> block into a useConsent.ts composable exporting
hasSessionConsent()/giveSessionConsent() plus test-only
_resetSessionConsentForTesting()/_giveSessionConsentForTesting() helpers,
mirroring web-app-ai-data-insights-sidebar's useInsights.ts pattern. This
finally makes the consent-gating flow testable: existing unit tests could
only route around it because the flag "wasn't exposed for resetting". Add
unit and e2e coverage proving zero LLM calls happen before consent and on
denial, and that confirming triggers exactly one.

Also update the view's template so a non-empty collections grid renders
alongside an inline error notice instead of being replaced by the full-page
error state, needed for the previous commit's partial-results fix to
actually reach the user.

Addresses a test-coverage finding from the review on this PR.

Signed-off-by: Lukas Hirt <info@hirt.cz>
…tch the monorepo and clean up package metadata

@ownclouders/web-client, web-pkg, and extension-sdk were pinned to 12.3.2
while every other package in the monorepo is on 12.4.2 — bump to match
(pnpm install updates the lockfile accordingly). Fill in the package.json
"description" field, which was still the unfilled "CANDIDATE" placeholder.
Remove a leftover .gitignore comment/entries referencing debug artifacts
that don't exist anywhere in the tree. Update the index.ts comment's web-pkg
version citation to reference the dependency's caret range instead of a
literal patch version, so it doesn't go stale on the next patch bump.

Addresses three findings from the review on this PR.

Signed-off-by: Lukas Hirt <info@hirt.cz>
@LukasHirt
LukasHirt force-pushed the ext/2026-07-07-ai-smart-collections-nav branch from 208c086 to 48e8710 Compare July 20, 2026 11:47
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