Skip to content

feat(ingestion): paginated, faceted, sortable artifacts with date/language filters, per-id delete outcomes and AI status proxy - #258

Open
daniilperkin wants to merge 9 commits into
devfrom
feature/KB-updates-final
Open

daniilperkin wants to merge 9 commits into
devfrom
feature/KB-updates-final

Conversation

@daniilperkin

@daniilperkin daniilperkin commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Related issue

SprintStartProject/Wiki#303 — [Epic]: Knowledge Base Server-Side Pagination & Faceted Search

Short summary

Artifact reads for a project now page and count in the database instead of in the client: a 10k-artifact project no longer hydrates 10k entities — each carrying the eagerly fetched content TEXT column — to answer one screen.

  • Index: @CollectionTable(indexes = [...]) on Artifact.projectIdsInternal declares idx_artifact_projects_project (project_id, artifact_id) — the join the page query actually makes. Hibernate's ddl-auto: update creates it on boot; V20__add_artifact_project_index.sql is the reference-only equivalent for a production database, to be run by hand with CONCURRENTLY.
  • Projection queries: ArtifactFacetRepositoryImpl builds both the page read and the single-artifact read as a Criteria projection into ArtifactResponse (id, title, sourceSystem, sourceId, sourceUrl, artifactType, ingestedAt, lastChangedAt, metadata, sourceVersion), so content is never selected. Ordering is ingestedAt DESC, id ASC — the unique tiebreaker without which a page boundary can duplicate or drop rows.
  • Facet counts: ArtifactFacetRepositoryImpl.findFacets keeps the client's "count each would add" model — each facet is counted with its own selection excluded and the others applied, the upload-format and repository facets stay scoped to the rows they describe, the org profile is counted for repositories owned by its org (case-folded), and a selected value whose count fell to zero stays listed. Upload formats are classified by one rule, shared with the filter.
  • REST:
    • GET /api/v1/projects/{projectId}/artifacts — page, size, search (title, sourceId, sourceUrl), types, sources, repositories, format; filter is kept as a deprecated alias of search.
    • GET /api/v1/projects/{projectId}/artifacts/facets — { types, sources, formats, repositories }, each a list of { value, count }.
    • GET /api/v1/projects/{projectId}/artifacts/{artifactId} — one artifact's metadata for deep links, projected like the list.
  • Tests: ArtifactControllerTest (list filters, facets, single artifact, 403/404), ArtifactQueryServiceTest, ArtifactFacetRepositoryImplTest.

Checks

  • I verified the code makes sense intuitively
  • The PR changes affect only this issue, no unrelated/unwanted code changes to other modules/code segments
  • CI runs (./gradlew ktlintCheck detekt test)
  • New business logic is unit tested (ArtifactControllerTest, ArtifactQueryServiceTest, ArtifactFacetRepositoryImplTest)
  • The new functionality is tested manually against local PostgreSQL container

Review round (upgrade)

Ships in upgrade(ingestion): finish the artifact pagination upgrade:

  • the upload-format filter and the facet counts beside it disagreed: the predicate compared nullable columns directly, so NOT(OR(...)) — the OTHER bucket — evaluated to NULL instead of TRUE, and an upload with no mime and a .txt title was counted as OTHER but returned nothing when that filter was picked. The nullable columns now fold to "" exactly as the classifier does;
  • the single-artifact read no longer hydrates the entity (and its content column); both reads share one projection;
  • removed the caller-less legacy getProjectArtifacts(page, size, filter, ...) overload and ArtifactRepository.findByIdAndProjectId;
  • filter documented as an alias of search; Index imported instead of written out fully qualified; the V20 script's header no longer contradicts the statement below it; the .gitignore note is tool-neutral;
  • ArtifactFacetServiceTest renamed to ArtifactFacetRepositoryImplTest — it tests the repository, not a service — and moved next to it, with a classifier case for the row above.

Knowledge Base UX & filtering upgrade (v1, phases 4-8, backend half)

Builds on the pagination work above; one commit per phase, each green on the full ./gradlew build (tests, detekt, ktlint, jacoco) before commit. Final head 3512a0ae: 3465 tests, 0 failures.

Phase Commit Change
4 69bf7b29 sort on the list
5 464f18b6, 04e68596 from / to activity-date window on list + facets
6 dde464e4 languages filter + languages facet + language on ArtifactResponse
7 455f1e21 Bulk delete answers per id instead of a blind 204
8 1a0e8437 GET .../artifacts/ai-status proxy to the AI service
decision 2 3512a0ae AI status timeout via sync(timeout), no lint suppression

Frontend counterpart: SprintStartProject/sprintstart-frontend#264. AI counterpart: SprintStartProject/sprintstart-ai#204.

Phase 4: sort

  • sort = ADDED_DESC (default, exactly today's ingestedAt DESC, id ASC), CHANGED_DESC (COALESCE(lastChangedAt, ingestedAt) DESC, id ASC), TITLE_ASC (lower(title) ASC NULLS LAST, id ASC, JPA 3.2 Nulls.LAST). Unknown value is a 400; facets ignore sort. id stays the unique tie-breaker so page boundaries never duplicate or drop rows.
  • The sort now travels in ArtifactFilterCriteria; the unused Sort the service used to build (the repository ignored it) is gone.

Phase 5: activity-date window

  • from / to (yyyy-MM-dd, both inclusive, either optional) on list AND facets, filtering last activity COALESCE(lastChangedAt, ingestedAt) (was ingestedAt until 04e6859) with UTC day boundaries (from 00:00Z inclusive, to next day 00:00Z exclusive). from > to is a 400.

Phase 6: language

  • languages (repeatable) matched case-insensitively on list and facets; ArtifactResponse.language is now sent (the frontend type already declared it).
  • languages facet: grouped by lower(language) with least(language) as the display spelling, so kotlin/Kotlin never split. Document kinds (Markdown, Plain Text) are excluded because they are not programming languages, but a selected one stays listed so it can be unticked. Same own-dimension rule as the existing facets.

Phase 7: bulk delete outcome

  • DELETE /api/v1/uploads now returns 200 { deletedIds, failed: [{ artifactId, error }] } instead of 204. The service already computed per-id outcomes and threw them away; the frontend can now say exactly what was and was not deleted.
  • A storage failure is reported with a generic "Artifact could not be deleted."; the raw exception text is logged at WARN and kept in the deletion event, never sent to the browser.

Phase 8: AI index status proxy

  • GET /api/v1/projects/{projectId}/artifacts/ai-status?ids=... returns 200 { aiAvailable, items: [{ artifactId, status, updatedAt, chunkCount }] } with status one of INDEXED | PROCESSING | FAILED | DEINDEXED | UNKNOWN.
  • Same USER role + project-access checks as the list. Ids outside the project are silently left out; more than 100 ids is a 400; no ids answers { aiAvailable: true, items: [] } without calling the AI.
  • Calls the AI's GET /api/v1/ingest/status?artifact_ids=... with a 3 s per-request timeout (optional sync(timeout) parameter on the shared RequestBuilder; existing sync() callers unchanged). Any AI failure (unreachable, timeout, non-2xx, bad body) gives aiAvailable: false and UNKNOWN items, never a 5xx; unrecognised AI statuses map to UNKNOWN.
  • Tests: ArtifactAiStatusServiceTest, ArtifactAiStatusControllerTest (imports SecurityConfig so @PreAuthorize is actually enforced in the slice), client tests on MockWebServer.

Tests added across phases 4-8

  • ArtifactFacetRepositoryQueryTest (new, H2): real queries for ordering, date boundaries, activity vs import date, language facet and list/facet parity; ArtifactControllerTest and ArtifactQueryServiceTest extended; UploadServiceTest / UploadControllerTest for per-id outcomes.

Checks for this upgrade

  • ./gradlew build green on 3512a0ae (3465 tests, 0 failures)
  • Contract cross-checked against the frontend and AI PRs
  • End-to-end against a running sprintstart-ai (in progress; docker stack logs clean)

Decisions (resolved 2026-09-24)

  1. Date filter = last activity. from/to now match COALESCE(lastChangedAt, ingestedAt), the same key as the CHANGED_DESC sort; UI label "Updated". Backend 04e68596, frontend 0f44bb76.
  2. No suppression. The 3 s AI status timeout moved from a RequestBuilder.timeout() method to sync(timeout), so @Suppress("TooManyFunctions") is gone. Backend 3512a0ae.
  3. Commit style kept. Frontend/backend match their existing type(scope): subject history; recorded as the SprintStart exception in the workspace rules. No history rewrite.
  4. Stale "Indexed" after a failed deindex: accepted for v1, tracked in Ingest status reports 'indexed' while a failed deindex is pending sprintstart-ai#205.
  5. Phase 9 (facet caching): not started. Measure facet latency on the largest project first; only act above ~300 ms.

E2E checklist for manual verification is being run before this leaves draft.

Follows the server-side pagination and faceted search work with the findings
from a full review of the change.

Correctness:
* the upload-format filter disagreed with the facet counts it sits next to. The
  predicate compared nullable columns directly, so `NOT(OR(...))` — the OTHER
  bucket — evaluated to NULL instead of TRUE: an upload with no mime and a .txt
  title was counted as OTHER but returned nothing when that filter was picked.
  The nullable columns now fold to "" exactly as `classifyUploadFormat` does,
  so the counts and the filter can no longer describe different rows.
* `GET .../artifacts/{artifactId}` hydrated the entity, dragging the eagerly
  fetched `content` TEXT column along to answer a metadata question. Both reads
  now share one projection, so the deep-link path cannot drift from the list.

Cleanup:
* removed the legacy `getProjectArtifacts(page, size, filter, ...)` overload and
  `ArtifactRepository.findByIdAndProjectId`; neither had a caller left.
* `filter` is documented as what it now is — an alias of `search` — in OpenAPI
  and in code, instead of quietly changing meaning under its old description.
* `Index` is imported rather than written out fully qualified; the reference-only
  V20 script no longer documents CONCURRENTLY and then ships the blocking
  statement; the `.gitignore` note no longer names the tool that wrote it.
* `ArtifactFacetServiceTest` tested `ArtifactFacetRepositoryImpl` — renamed and
  moved to the repository package, plus a classifier case for the row that
  exposed the OTHER bug above.

Gates: ./gradlew build (ktlint, detekt, tests).
@daniilperkin
daniilperkin marked this pull request as draft September 23, 2026 20:14
…d or title

The project artifact list could only be read in one order (newest import
first). Knowledge-base users asked for "recently changed" and an
alphabetical view, so GET /api/v1/projects/{projectId}/artifacts now takes
a `sort` query parameter:

  ADDED_DESC   (default) ingestedAt DESC, id ASC   -- exactly today's order
  CHANGED_DESC coalesce(lastChangedAt, ingestedAt) DESC, id ASC
  TITLE_ASC    lower(title) ASC NULLS LAST, id ASC

The values and their SQL come from the pinned cross-repo contract
(kb-contract.md) that the frontend and the AI service are built against
in parallel, so the names are uppercase enum constants and are not
negotiable here.

Why a new enum (ArtifactSort) instead of Spring's `sort=field,dir`:
  - The contract offers three named orderings, not arbitrary columns.
    Accepting free-form Pageable sort strings would let a client order by
    any entity property (including the heavyweight `content` column, or
    properties the projection does not even select) and would make the
    set of supported orderings an accident of the entity shape.
  - Binding to an enum gives the "unknown value -> 400" rule for free:
    Spring's String->Enum conversion fails with a type mismatch, which the
    default handler resolver answers with 400 before the service runs.
    No hand-written validation, no chance of a silent fallback.
  - `defaultValue = "ADDED_DESC"` keeps every existing caller (which sends
    no sort) on the exact order it had before, so this is backwards
    compatible for the current frontend.

Why the order is carried as its own argument and not through Pageable:
  - ArtifactQueryService built a PageRequest with
    Sort(ingestedAt DESC, id ASC), but ArtifactFacetRepositoryImpl never
    read pageable.sort; it hard-coded the same ORDER BY itself. The Sort
    was dead code that only looked like it controlled ordering. It is
    removed; the service now passes an unsorted PageRequest (page window
    only) plus the ArtifactSort, and the repository KDoc says so.
  - It is a separate parameter rather than a field on
    ArtifactFilterCriteria because the criteria object is also what the
    facets endpoint receives and what the list/facet parity guarantee is
    defined over. Facets ignore sort (contract), so putting sort into the
    criteria would make two criteria that select the same rows compare
    unequal and would invite a future facet query to depend on it.

Why every ordering ends in `id ASC`:
  - The list is offset-paginated. Without a unique tie-break, rows that
    share the leading key (same ingest instant from a bulk sync, same
    title, or no title at all) can be returned in a different order by
    each query, so a row can appear on two pages or on none. id is the
    primary key, so the full ORDER BY is a total order.

Why TITLE_ASC uses lower(title) and the JPA 3.2 Nulls API:
  - lower() makes "alpha" and "Alpha" sort together, which is what a
    person scanning an alphabetical list expects; the id tie-break then
    keeps the two deterministic.
  - Untitled artifacts must not float to the top. PostgreSQL sorts NULLs
    last for ASC but H2 and other databases differ, so the null placement
    is stated explicitly with cb.asc(expr, jakarta.persistence.criteria
    .Nulls.LAST) -- the portable JPA 3.2 API that Hibernate 7 renders as
    NULLS LAST -- instead of Hibernate's NullPrecedence extension, which
    would tie the repository to a provider-specific API for no gain.

Why ArtifactFacetRepositoryImpl now carries @Suppress("TooManyFunctions"):
  - The ordering builder is its 12th function and detekt caps classes at
    11. The class is one function per facet dimension plus the shared
    predicate/order builders; splitting it would scatter the own-dimension
    exclusion rule that has to stay identical between list and facets.
    The suppression follows the 30+ existing ones in the codebase
    (ArtifactRepository included) rather than inventing a new split.

Why CHANGED_DESC coalesces with ingestedAt:
  - lastChangedAt is null until ingestion sees the content change for the
    first time. Treating "never changed" as "changed when it was
    imported" gives those rows a real position instead of lumping them at
    one end, and ingestedAt is non-null so the expression never is.

Tests:
  - New ArtifactFacetRepositoryQueryTest, a @DataJpaTest against H2 (same
    setup as ArtifactProjectRepositoryTest). The existing
    ArtifactFacetRepositoryImplTest only unit-tests helpers and never
    executes a query, so nothing pinned the SQL the criteria API renders.
    Covers: each ordering, the id tie-break for equal keys, NULLS LAST and
    case folding for titles, the coalesce fallback, stable page boundaries
    for rows sharing a key, and that sorting does not change totalElements.
    Ids are UUID(0, n) so their order is the same on every database.
  - ArtifactControllerTest: explicit sort binds to the enum and reaches the
    service; the default is ADDED_DESC; an unknown value is rejected with
    400 and never reaches the service.
  - ArtifactQueryServiceTest: the sort is forwarded to the repository and
    the Pageable is unsorted (page window only), replacing the assertions
    on the dead Sort.

Gate: ./gradlew build (compile, detekt, ktlint, full test suite, jacoco)
green: 3426 tests, 0 failures (baseline before this change: 3419).
…indow

Adds optional `from` / `to` query parameters (ISO yyyy-MM-dd) to
GET /api/v1/projects/{projectId}/artifacts and to its /facets sibling, so
the Knowledge Base can answer "what was imported last week" without paging
through everything.

Contract (pinned with the frontend and AI-service work on this branch):
  - from, to = ISO calendar dates, both optional, both inclusive.
  - They bound `ingestedAt` (first import), read as UTC calendar days:
    from -> ingestedAt >= from 00:00Z; to -> ingestedAt < (to + 1) 00:00Z.
  - from > to -> 400. A malformed date -> 400 (Spring's type conversion).
  - The facets endpoint applies the exact same predicate (parity).

Why the window is on ingestedAt and not coalesce(lastChangedAt, ingestedAt):
  - The original plan suggested filtering on the "last changed" instant;
    the contract agreed afterwards pins ingestedAt. It is the column the
    default ADDED_DESC order uses, so "added from 1 March" and "sorted by
    added" talk about the same instant.
    It is also NOT NULL, so no row silently escapes an open-ended window.

Why UTC calendar days with a half-open upper bound:
  - The server has no user time zone to work with, and ingestedAt is an
    Instant; pinning UTC makes the same URL return the same rows for every
    caller and on every server, whatever its default zone.
  - `to` becomes `< start of the next day` rather than `<= 23:59:59.999`,
    so the last microsecond of that day still matches regardless of the
    precision the timestamp column keeps (H2 and PostgreSQL both keep
    microseconds today; the bound does not depend on it).

Why the predicate lives in buildPredicates and is never "excluded":
  - Every list, count and facet query already goes through buildPredicates;
    no facet counts the import date, so no FacetKind ever skips the window.
    That single code path is what guarantees facet counts equal the list's
    totalElements under the same filter -- the parity the contract requires.
    It sits in its own buildIngestedWindowPredicates helper so the already
    long buildPredicates stays under detekt's complexity threshold.

Why from > to is a 400 and why the check is in ArtifactQueryService:
  - An inverted window can never match; answering it with an empty page
    would disguise a client bug (swapped bounds) as "no results".
  - AGENTS.md puts validation in services, not controllers. One private
    requireValidDateWindow is called by both getProjectArtifacts and
    getProjectArtifactFacets, so the two endpoints cannot disagree on what
    a valid window is. It runs before the access check and the repository,
    like Spring's own binding errors, which also precede the access check.
  - from == to is valid and means "that one day".

Tests:
  - ArtifactFacetRepositoryQueryTest (real H2 via @DataJpaTest):
    boundary instants -- 23:59:59.999999 the day before (out), 00:00:00 on
    `from` (in), 23:59:59.999999 on `to` (in), 00:00:00 the day after
    (out); each open bound on its own; and facet parity: types and sources
    facet counts sum to the list's totalElements under the same window.
  - ArtifactQueryServiceTest: from > to is a 400 from both list and facets
    and reaches neither the access check nor the repository; from == to is
    passed through.
  - ArtifactControllerTest: from/to bind as ISO dates on list and facets
    into the criteria; a non-ISO date is a 400 that never reaches the
    service.

Gate: ./gradlew build (compile, detekt, ktlint, full test suite, jacoco)
green: 3435 tests, 0 failures (previous commit: 3426).
…uages

Artifacts already carry a `language` column, filled at ingestion by
FileMetaDataResolver's extension map ("kt" -> "Kotlin", "md" -> "Markdown",
"txt" -> "Plain Text", ...), but the API neither exposed it nor let
users narrow by it. This commit adds, per the pinned contract:

  - `languages` query parameter (repeatable, case-insensitive) on
    GET /api/v1/projects/{projectId}/artifacts and on /artifacts/facets.
  - `language: String?` on every ArtifactResponse (list and detail).
  - `languages: [{value, count}]` on ArtifactFacetsResponse.

Why the match is lower(language) IN (lowercased selection):
  - The stored value is a display name ("Kotlin", "YAML"), and a client
    may send it from a chip, a URL someone typed, or an older build. Case
    must not decide whether "kotlin" finds Kotlin files. Folding both
    sides with lower() keeps the predicate a plain IN list, which every
    database plans well, instead of per-row equalsIgnoreCase logic.
  - Selected values are trimmed, blanks dropped and deduplicated ignoring
    case before use (selectedLanguages()), so `languages=` or
    `languages=Kotlin&languages=kotlin` behave like the obvious intent.

Why the language filter narrows every source (unlike format/repository):
  - The existing format and repository filters are written as
    "not UPLOAD or matches format" / "not GITHUB or matches repo", because
    they are sub-filters of one source. Language is a property any artifact
    may have, so it is a plain AND like `types`: while it is set,
    artifacts without a language (issues, PRs, pages) drop out. That is
    what a user choosing "Kotlin" expects, and it is stated in the KDoc
    and the Swagger parameter description.

Why the language facet counts the way it does:
  - Contract part 2 says: copy whatever own-dimension rule the existing
    facets use. In buildPredicates each facet drops only its own filter
    (types drops types; sources additionally drops format/repository
    because those are sub-filters of a source). Language is an
    independent dimension like types, so the language facet passes
    FacetKind.LANGUAGES and drops only the language filter; every other
    facet keeps applying it. Result: each chip shows how many rows the
    list would have if that language were (also) picked.
  - Grouping is by lower(language), the same folding as the filter, so a
    stray "kotlin" next to "Kotlin" is one group whose count equals the
    list total for that filter (list/facet parity). least(language) picks
    one stored spelling to display; which one is collation-dependent, and
    the test deliberately only checks it case-insensitively.
  - Null languages are excluded (isNotNull), and "Markdown"/"Plain Text"
    are hidden because they describe documents, which the format facet
    already covers; offering both would duplicate the same choice.
    Exception: if the client did select one of them, it is still shown
    with its real count so a selected chip never vanishes.
  - A selected language with no match is returned with count 0, the same
    way the types and repositories facets echo their selection, so the
    UI can render the active chip and let the user remove it.
  - Order is count descending then value ascending, matching the other
    facets and the contract, and it is deterministic across databases.

Why the shaping lives in a top-level internal languageFacetOptions():
  - Hiding documents, echoing selections and ordering are pure list
    operations. Keeping them out of the JPA query makes them unit-testable
    without a database and keeps the SQL a simple GROUP BY; the class
    already carries @Suppress("TooManyFunctions") and this does not add
    to it.

Why `language` is appended last to the ArtifactResponse projection:
  - The list query builds DTOs with cb.construct(), which binds by
    constructor position. Appending the new nullable, defaulted property
    at the end keeps every existing positional call and named call site
    valid and makes the projection change a one-line addition.

Tests:
  - ArtifactFacetRepositoryQueryTest (real H2 via @DataJpaTest): filter
    ignores case and drops null-language rows; the language facet drops
    its own filter but applies types; documents and nulls are hidden and
    an unmatched selection comes back at 0; "Kotlin"+"kotlin" is one
    group whose count equals the list totalElements for that filter.
  - ArtifactFacetRepositoryImplTest: a selected document language is kept
    with its real count.
  - ArtifactControllerTest: repeated `languages` bind into the criteria on
    list and facets; `language` and `languages` appear in the JSON.
  - Existing facet fixtures gained the new required `languages` list.

Gate: ./gradlew build (compile, detekt, ktlint, full test suite, jacoco)
green: 3441 tests, 0 failures (previous commit: 3435).
DELETE /api/v1/uploads used to answer 204 whatever happened: ids that
were missing, belonged to another project or failed in storage were
swallowed, so the Knowledge Base could only assume every row was gone
and silently drifted from the server. It now answers 200 with

  { "deletedIds": [uuid], "failed": [{ "artifactId": uuid, "error": str }] }

Contract (pinned with the frontend work on this branch):
  - deletedIds keeps request order, so the client can drop exactly those
    rows and leave the rest selected.
  - Every requested id lands in exactly one of the two lists; a partial
    failure never fails the whole request (still 200).
  - The frontend also accepts an empty 204 from an older backend and
    then treats every requested id as deleted, so rollout order is free.

Why the client-facing error is never the raw exception message:
  - Storage exceptions can carry file-system paths, bucket names or
    driver internals. Those belong in operator logs, not in a JSON body
    any PM can read. Not-found keeps "Artifact with id <id> not found."
    (it only echoes the caller's own id); every storage failure reports
    the fixed "Artifact could not be deleted.".
  - The raw message is not lost: it stays in the
    UploadBatchDeletionFinishedEvent outcome, exactly as before, and is
    written to a warn log with the artifact id, so ingestion bookkeeping
    and debugging see the real cause.
  - The response failures are therefore collected in their own list next
    to the event outcomes instead of being mapped from them afterwards;
    mapping would have leaked the raw text or needed a second lookup.

Why the event and the per-item behaviour are otherwise unchanged:
  - Listeners of UploadStartedEvent / UploadBatchDeletionFinishedEvent
    (the upload ingestion-run lifecycle) keep receiving the same
    payload, raw error text included, so this is purely an HTTP-surface
    change.
  - Foreign-project ids are still reported as "not found" rather than
    "forbidden", so the endpoint does not confirm that an id exists in a
    project the caller cannot see.

Tests:
  - UploadServiceTest: deleted ids in request order, not-found reason,
    and a storage failure that carries a path-like message: the response
    shows only the generic reason while the event outcome keeps the raw
    message.
  - UploadControllerTest: 200 with the deletedIds/failed body for PM and
    admin; the existing role / validation guards are unchanged.

Gate: ./gradlew build green (3442 tests, 0 failures; detekt 0 issues;
ktlint clean).
Adds GET /api/v1/projects/{projectId}/artifacts/ai-status?ids=a&ids=b
so the Knowledge Base can show, per row, whether the AI assistant can
actually find an artifact (Indexed / Indexing / Failed / Not indexed).

Contract (kb-contract part 3, pinned with the frontend on this branch):
  200 { "aiAvailable": bool,
        "items": [{ "artifactId", "status",
                    "updatedAt", "chunkCount" }] }
  - status = INDEXED | PROCESSING | FAILED | DEINDEXED | UNKNOWN.
  - aiAvailable=true: the AI answered; UNKNOWN means it holds no record.
  - aiAvailable=false: AI unreachable, timed out, non-2xx or unreadable
    body; every visible id is UNKNOWN with null fields; warn log.
  - Ids outside the project are omitted; more than 100 ids -> 400; no
    ids -> 200 { aiAvailable: true, items: [] } without an AI call.
  - Same USER role and project-access check as the artifact list.

Why a separate endpoint instead of a field on the list:
  - The list is served from our own database and must stay fast and
    available. Folding an AI call into it would make every page wait
    for, or fail with, the AI service. The frontend asks for chips after
    the page rendered, for exactly the ids it shows.

Why aiAvailable exists and why AI trouble is never a 5xx:
  - "The AI has no record" (show "Not indexed") and "we could not ask"
    (show nothing) look the same as bare UNKNOWN items. The flag lets
    the frontend hide chips during an outage instead of claiming that
    every artifact is missing from the index.
  - Status chips are decoration; a broken AI must not turn a working
    Knowledge Base page into an error. Every failure, including an
    unexpected one, becomes aiAvailable=false plus a one-line warn log
    (no stack trace: an outage would otherwise flood the log on every
    page view). Coroutine cancellation is still propagated.

Why foreign ids are dropped before the AI is asked:
  - ArtifactRepository.findIdsInProject selects ids only (no content
    column is loaded) through the same project join the list uses. Only
    ids linked to the project reach the AI, so the endpoint can neither
    leak another project's index state nor confirm that an id exists
    elsewhere; unknown ids are omitted the same way. Duplicates collapse
    and the answer keeps request order. If nothing visible remains, the
    AI is not called at all.

Why the 100-id cap is checked in the controller:
  - It is request-shape validation, like the page-size @max, and it
    matches the AI endpoint's own limit, so an oversized request fails
    with our 400 before any database or AI work instead of an AI 422. It
    counts raw ids, the same way the AI counts them.

Why RequestBuilder gains an optional per-request timeout:
  - The shared HttpClient only has a 10 s connect timeout and no request
    timeout, so a hung AI (accepting connections, never answering) would
    hold a Knowledge Base request forever. A coroutine withTimeout
    cannot help: SyncExecution runs the blocking HttpClient.send on
    Dispatchers.IO, and withContext waits for it to return.
  - RequestBuilder.timeout(Duration) sets HttpRequest.timeout for that
    one request and is carried through copy(). It is additive: unset
    (every existing caller) builds exactly the request it built before.
    The extra method tips detekt's TooManyFunctions on this fluent
    builder; it is suppressed there with the reason in the KDoc.
  - fetchIngestStatus uses 3 s: the AI side is a keyed metadata read,
    and the frontend polls this per page, so a stuck AI costs a short
    wait and then degrades to aiAvailable=false.

Why the backend owns the status enum and maps it leniently:
  - ArtifactIngestStatusAiItem keeps the AI wire format as raw strings;
    ArtifactAiIndexStatus.fromAi maps case-insensitively and turns null,
    "unknown" and anything unrecognised into UNKNOWN. A status the AI
    adds later therefore degrades to "Not indexed" instead of failing
    deserialisation and hiding every chip.
  - UNKNOWN always carries null updatedAt/chunkCount, whatever the AI
    sent, so it has one meaning downstream. An id the AI omits is
    UNKNOWN.
  - updatedAt is passed through as the AI's ISO string rather than
    parsed into an Instant: the backend is a proxy here, and re-parsing
    would turn an offset-less but valid timestamp into a failure.

Why the endpoint is a suspend function in its own service:
  - ArtifactIngestionClient is suspend-based. ArtifactAiStatusService is
    deliberately not @transactional (the annotation does not apply to
    suspend functions, and the one id lookup needs none), the same
    choice ArtifactProjectService documents.

Tests:
  - ArtifactAiStatusControllerTest (imports SecurityConfig so the
    @PreAuthorize guard is really exercised): contract JSON shape incl.
    explicit nulls, missing ids -> empty, 101 ids -> 400 without calling
    the service, exactly 100 accepted, service 403 passed through, no
    USER role -> 403 without calling the service, malformed id -> 400.
  - ArtifactAiStatusServiceTest: access denied before any lookup, empty
    request and all-foreign request make no AI call, foreign ids dropped
    and duplicates collapsed in request order, connect error / timeout /
    non-2xx / bad body each give aiAvailable=false with UNKNOWN nulls,
    and status mapping incl. uppercase, "unknown", unrecognised and
    omitted ids.
  - ArtifactIngestionClientTest (MockWebServer): GET path with repeated
    artifact_ids, snake_case parsing with unknown fields ignored, 503 ->
    IngestionResponseException, wrong body -> SerializationException,
    closed port -> IOException, hung server -> HttpTimeoutException.
  - RequestBuilderTest: no timeout by default; timeout() survives
    chaining. ArtifactRepositoryIdsInProjectTest: the project join on
    H2.

Gate: ./gradlew build green (3463 tests, 0 failures, 0 skipped; detekt
0 issues; ktlint clean; jacoco verification passed).
@daniilperkin daniilperkin changed the title feat(ingestion): server-side pagination and faceted search for artifacts feat(ingestion): paginated, faceted, sortable artifacts with date/language filters, per-id delete outcomes and AI status proxy Sep 24, 2026
`from` / `to` now match an artifact's last activity,
COALESCE(lastChangedAt, ingestedAt), instead of its first import.

Why: the original plan decided on activity semantics ("added or changed
in this window"). The pinned API contract narrowed it to ingestedAt by
mistake. For onboarding, "what changed recently that I should re-read"
is the more useful question: a README imported months ago but edited
yesterday should show up under "Last 7 days".

- The window uses the same key as the CHANGED_DESC sort, so "updated in
  the last 7 days" and "most recently changed first" always agree.
- An artifact imported inside the window but changed after it no longer
  matches, because its latest activity lies outside the window.
- The UTC whole-day bounds and the `from > to` 400 are unchanged; the
  window still applies to list and facets alike (count parity).
- New repository test pins all three shapes: old but edited inside,
  imported inside but changed after, never changed and imported inside.
- KDoc and OpenAPI descriptions say "activity" instead of "import".
… the builder

Drops the `@Suppress("TooManyFunctions")` that the ai-status proxy added
to the shared RequestBuilder.

The 3 s bound on the AI status call stays, because a hung AI service
must cost the Knowledge Base a short wait, not a request that never
returns. It now travels as an optional parameter:
`sync(timeout: Duration? = null)`, not a separate `timeout()` builder
method.

- That keeps RequestBuilder at detekt's function limit, so no new
  suppression lands (sprintstart-helper rule: don't commit
  suppressions).
- It fits the design: the timeout says how to run the request, not what
  to send, and `sync()` is already the execution-context step.
- Source-compatible: every existing `.sync()` call keeps its behaviour,
  with no timeout unless asked for.
- RequestBuilderTest now covers `sync(timeout)` landing on the built
  request, and `sync()` leaving it unbounded.
@daniilperkin
daniilperkin marked this pull request as ready for review September 24, 2026 11:01
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