From 784a7759f57e055cc124e184ebe9fe7428d14f97 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 7 Jun 2026 19:50:48 +0200 Subject: [PATCH] fix(review): resolve all review blockers and concerns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Delete frontend/e2e/nl-search.spec.ts (was left alive; would have crashed CI when Playwright couldn't find the deleted SmartModeToggle) - Fix docs/DEPLOYMENT.md: remove NLP service arrow + key-facts bullet that were accidentally added instead of removed in the prior commit - Clean docs/GLOSSARY.md: remove keyword→tag resolution, PersonHint, TagHint, theme chip entries; trim NameMatches to drop the NlQueryParserService reference - Remove @ConfigurationPropertiesScan from FamilienarchivApplication (all remaining @ConfigurationProperties beans carry @Component) - Remove 12 orphaned i18n keys from de/en/es message files (search_loading_nl, search_chip_*, search_disambiguation_*, etc.) - Fix SearchFilterBar.svelte input padding: pr-20 → pr-4 (SmartModeToggle that justified the right padding is gone) - Delete docs/superpowers/plans/2026-06-07-remove-nlp-search.md (scaffolding artefact; plan files belong in Gitea issues, not the repo) - Add docs/adr/034-remove-nl-search.md documenting the removal decision (supersedes deleted ADR-028 ×2, ADR-034-ollama, ADR-035) Co-Authored-By: Claude Sonnet 4.6 --- .../FamilienarchivApplication.java | 2 - docs/DEPLOYMENT.md | 3 +- docs/GLOSSARY.md | 11 +- docs/adr/034-remove-nl-search.md | 53 ++ .../plans/2026-06-07-remove-nlp-search.md | 768 ------------------ frontend/e2e/nl-search.spec.ts | 115 --- frontend/messages/de.json | 12 - frontend/messages/en.json | 12 - frontend/messages/es.json | 12 - frontend/src/routes/SearchFilterBar.svelte | 2 +- 10 files changed, 56 insertions(+), 934 deletions(-) create mode 100644 docs/adr/034-remove-nl-search.md delete mode 100644 docs/superpowers/plans/2026-06-07-remove-nlp-search.md delete mode 100644 frontend/e2e/nl-search.spec.ts diff --git a/backend/src/main/java/org/raddatz/familienarchiv/FamilienarchivApplication.java b/backend/src/main/java/org/raddatz/familienarchiv/FamilienarchivApplication.java index 0b358e80..4fef338f 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/FamilienarchivApplication.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/FamilienarchivApplication.java @@ -2,10 +2,8 @@ package org.raddatz.familienarchiv; import org.springframework.boot.SpringApplication; import org.springframework.boot.autoconfigure.SpringBootApplication; -import org.springframework.boot.context.properties.ConfigurationPropertiesScan; @SpringBootApplication -@ConfigurationPropertiesScan public class FamilienarchivApplication { public static void main(String[] args) { diff --git a/docs/DEPLOYMENT.md b/docs/DEPLOYMENT.md index 4692964a..9368d782 100644 --- a/docs/DEPLOYMENT.md +++ b/docs/DEPLOYMENT.md @@ -33,7 +33,6 @@ graph TD Backend -->|JDBC :5432| DB[(PostgreSQL 16)] Backend -->|S3 API :9000| MinIO[(MinIO)] Backend -->|HTTP :8000 internal| OCR["OCR Service\nPython FastAPI"] - Backend -->|HTTP :8001 internal| NLP["NLP Service\nPython FastAPI"] OCR -->|presigned URL| MinIO Caddy -->|SSE proxy_pass| Backend ``` @@ -41,7 +40,7 @@ graph TD **Key facts:** - Caddy terminates TLS and reverse-proxies to frontend (`:3000`) and backend (`:8080`). The Caddyfile is committed at [`infra/caddy/Caddyfile`](../infra/caddy/Caddyfile) and is installed on the host as `/etc/caddy/Caddyfile` (symlink). - The host binds all docker-published ports to `127.0.0.1` only; Caddy is the sole external entry point. -- The OCR service and NLP service have **no published ports** — reachable only on the internal Docker network from the backend. +- The OCR service has **no published port** — reachable only on the internal Docker network from the backend. - SSE notifications transit Caddy (browser → Caddy → backend); the backend is never reachable directly from the public internet. The SvelteKit SSR layer is bypassed for SSE, but Caddy is not. - The Caddyfile responds `404` on `/actuator/*` (defense in depth). Internal monitoring scrapes the backend on the docker network, not through Caddy. - Production and staging cohabit on the same host via docker compose project names: `archiv-production` (ports 8080/3000) and `archiv-staging` (ports 8081/3001). diff --git a/docs/GLOSSARY.md b/docs/GLOSSARY.md index 0af61be9..d7f05bb7 100644 --- a/docs/GLOSSARY.md +++ b/docs/GLOSSARY.md @@ -165,16 +165,7 @@ _See also [Chronik](#chronik-internal)._ **Domain** — a Tier-1 bounded context with its own entities, controller, service, repository, and DTOs. Backend domains: `document`, `person`, `tag`, `user`, `geschichte`, `notification`, `ocr`, `audit`, `dashboard`. Frontend domains mirror this structure under `src/lib/`. - -**keyword→tag resolution** — the post-Ollama step in `NlQueryParserService` where each LLM-extracted keyword is substring-matched against the tag taxonomy via `TagService.findByNameContaining()`. Keywords that hit one or more tags are removed from the FTS text list and become an OR-union tag filter; keywords with no match remain as FTS text. Matching is case-insensitive and traverses the tag hierarchy via the recursive CTE `findDescendantIdsByName`. See ADR-033. - -**PersonHint** — a lightweight `{id, displayName}` pair used in `NlQueryInterpretation` to describe a resolved or ambiguous person without exposing the full `Person` entity to the frontend. - -**NameMatches** — the Person-domain result of `PersonService.resolveByName(name)`: candidate persons split by name-match strength into `direct` and `partial`. A match is **direct** when every query token is a whole-token match (order-independent, alias/maiden-name aware) across all of a person's name components (`firstName`, `lastName`, `alias`, each `PersonNameAlias` first+last, `title`); a **partial** matched the substring fetch but is not direct (e.g. "Cram" → "Clara Cramer"). The vocabulary is deliberately match strength, not the search layer's resolved/ambiguous buckets — `NlQueryParserService` maps one direct → resolved (auto-select), ≥2 direct → ambiguous, partial-only → ambiguous suggestions ("Meintest du …?"), and no candidates → folded into full-text search. - -**TagHint** — a lightweight `{id, name, color?}` triple used in `NlQueryInterpretation.resolvedTags` to describe a tag matched by keyword→tag resolution. `color` is the tag's effective color (one-level inheritance from parent when the tag has no own color), or null if neither tag nor parent has a color. - -**theme chip** `[frontend]` — a removable chip rendered in `InterpretationChipRow` for each entry in `NlQueryInterpretation.resolvedTags` when `tagsApplied` is `true`. Displays "Thema: {tag.name}" (prefix varies by locale). Clicking × removes the tag from the OR-union filter and navigates to `/documents?tag=…&tagOp=OR` with remaining tag and person parameters preserved. +**NameMatches** — the Person-domain result of `PersonService.resolveByName(name)`: candidate persons split by name-match strength into `direct` and `partial`. A match is **direct** when every query token is a whole-token match (order-independent, alias/maiden-name aware) across all of a person's name components (`firstName`, `lastName`, `alias`, each `PersonNameAlias` first+last, `title`); a **partial** matched the substring fetch but is not direct (e.g. "Cram" → "Clara Cramer"). --- diff --git a/docs/adr/034-remove-nl-search.md b/docs/adr/034-remove-nl-search.md new file mode 100644 index 00000000..6f9e1ca6 --- /dev/null +++ b/docs/adr/034-remove-nl-search.md @@ -0,0 +1,53 @@ +# ADR-034 — Remove NL/smart-search (supersedes ADR-028 ×2, ADR-034-ollama, ADR-035) + +**Date:** 2026-06-07 +**Status:** Accepted +**Issue:** #772 +**Supersedes:** ADR-028 (nl-search-ollama), ADR-028 (ollama-docker-compose-service), ADR-034 (ollama-production-deployment-and-keep-alive), ADR-035 (rule-based-nlp-service) + +--- + +## Context + +The natural-language search feature ("KI-Suche" / smart search) allowed users to enter +free-form queries like *"Was hat Walter an Emma im Krieg geschrieben?"* and have them +interpreted by an LLM into structured filters (persons, tags, date range, keywords). + +The feature went through two major iterations: +1. **Ollama integration** (ADR-028): an `ollama` Docker service running a local LLM + (llama3.2/gemma3) parsed queries via a JSON-mode prompt. +2. **Rule-based NLP service** (ADR-035): after Ollama proved too slow and unreliable on + CPU-only hardware, a Python FastAPI microservice (`nlp-service`, port 8001) replaced + it with deterministic regex + spaCy parsing plus a lightweight LLM call. + +Both approaches shared the same fundamental problem: inference on the production server +(Hetzner Serverbörse, no GPU, 64 GB RAM, i7-6700) was too slow to be useful, with +typical query latencies of 10–30 seconds. Users got better and faster results from +the existing keyword search with date/person/tag filters. + +## Decision + +**Remove the NL search feature entirely.** The Python `nlp-service` microservice, the +Spring Boot `search/` package (`NlSearchController`, `NlQueryParserService`, +`RestClientNlpClient`, `NlSearchRateLimiter`, and all supporting classes), the frontend +NL search components (`SmartModeToggle`, `SmartSearchStatus`, `InterpretationChipRow`, +`DisambiguationPicker`), the related Docker Compose services, Prometheus scrape job, +Grafana dashboard, and all i18n keys are removed. + +The existing structured search (FTS keyword + person/tag/date/directional filters) is +sufficient for the archive's current audience and search workload. + +## Consequences + +- **Capability removed:** users can no longer enter free-form natural-language queries. + They must use the structured filter bar (keyword text box + person/tag/date/directional + dropdowns). For documents where these filters are sufficient, there is no regression. +- **Operational simplification:** the Docker Compose stack loses two services + (`nlp-service` and previously `ollama`/`ollama-model-init`). Memory budget on the + production host is freed. No external model weights need to be kept warm. +- **Future reinstatement:** if a GPU-capable host becomes available, re-implementing + server-side LLM inference would be straightforward given the clean separation of the + `NlSearchController` entry point. However, this ADR deliberately avoids leaving dead + infrastructure or stub code in place — start clean if and when that becomes viable. +- **No data or schema change:** only query/endpoint code and Docker services are removed. + The `documents`, `persons`, and `tags` tables and their FTS indexes are untouched. diff --git a/docs/superpowers/plans/2026-06-07-remove-nlp-search.md b/docs/superpowers/plans/2026-06-07-remove-nlp-search.md deleted file mode 100644 index 635a4b66..00000000 --- a/docs/superpowers/plans/2026-06-07-remove-nlp-search.md +++ /dev/null @@ -1,768 +0,0 @@ -# Remove NLP/Smart Search Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Remove the NLP/smart-search feature entirely from the codebase — backend search package, frontend components, i18n keys, infrastructure config, and the nlp-service microservice. - -**Architecture:** Pure deletion + targeted edits. No new code. Each task deletes a self-contained layer, then verifies compilation passes before committing. Order: backend first (most isolated), then frontend, then infrastructure, then docs. - -**Tech Stack:** Spring Boot 4 (Java 21, Maven), SvelteKit 2 / Svelte 5, Docker Compose, Paraglide i18n. - ---- - -## File Map - -### Delete entirely -- `backend/src/main/java/org/raddatz/familienarchiv/search/` — 14 Java source files -- `backend/src/test/java/org/raddatz/familienarchiv/search/` — 6 Java test files -- `frontend/src/routes/search/SmartModeToggle.svelte` + `.spec.ts` -- `frontend/src/routes/search/SmartSearchStatus.svelte` + `.spec.ts` -- `frontend/src/routes/search/InterpretationChipRow.svelte` + `.spec.ts` -- `frontend/src/routes/search/DisambiguationPicker.svelte` + `.spec.ts` -- `frontend/src/routes/search/chip-types.ts` -- `frontend/src/routes/documents/theme-chip-removal.ts` + `.spec.ts` -- `infra/observability/grafana/provisioning/dashboards/ollama.json` -- `nlp-service/` (entire directory) -- `docs/adr/028-nl-search-ollama.md` -- `docs/adr/028-ollama-docker-compose-service.md` -- `docs/adr/034-ollama-production-deployment-and-keep-alive.md` -- `docs/adr/035-rule-based-nlp-service.md` - -### Modify -- `backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java` — remove 2 enum values -- `backend/src/main/resources/application.yaml` — remove `nlp` + `nl-search` config blocks -- `backend/src/main/resources/application-dev.yaml` — remove `nlp` config block -- `frontend/src/routes/SearchFilterBar.svelte` — remove SmartModeToggle, smartMode prop, smart callbacks -- `frontend/src/routes/SearchFilterBar.svelte.spec.ts` — remove smart-mode describe block -- `frontend/src/routes/documents/+page.svelte` — remove all NL state, functions, template block -- `frontend/src/lib/shared/errors.ts` — remove 2 error codes + their switch cases -- `frontend/messages/de.json` — remove 8 smart-search keys -- `frontend/messages/en.json` — remove 8 smart-search keys -- `frontend/messages/es.json` — remove 8 smart-search keys -- `docker-compose.yml` — remove nlp-service block + backend depends_on + env var -- `docker-compose.prod.yml` — remove nlp-service block + backend depends_on + env var -- `infra/observability/prometheus/prometheus.yml` — remove ollama scrape job -- `CLAUDE.md` — remove search package reference + error code entries -- `backend/CLAUDE.md` — no change needed (search package already absent from structure) -- `frontend/CLAUDE.md` — update routes/search/ description - ---- - -### Task 1: Delete backend search package - -**Files:** -- Delete: `backend/src/main/java/org/raddatz/familienarchiv/search/` (14 files) -- Delete: `backend/src/test/java/org/raddatz/familienarchiv/search/` (6 files) - -- [ ] **Step 1: Delete all source files** - -```bash -rm -rf backend/src/main/java/org/raddatz/familienarchiv/search -rm -rf backend/src/test/java/org/raddatz/familienarchiv/search -``` - -- [ ] **Step 2: Verify backend compiles** - -```bash -cd backend && . ~/.sdkman/candidates/java/current/bin/../.. && source ~/.sdkman/bin/sdkman-init.sh && ./mvnw compile -q -``` - -Expected: BUILD SUCCESS with no errors. - -- [ ] **Step 3: Commit** - -```bash -git add -A -git commit -m "refactor(search): delete backend NLP search package" -``` - ---- - -### Task 2: Remove ErrorCode entries and backend config - -**Files:** -- Modify: `backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java:138-142` -- Modify: `backend/src/main/resources/application.yaml:133-138` -- Modify: `backend/src/main/resources/application-dev.yaml:15-17` - -- [ ] **Step 1: Remove NL Search enum values from ErrorCode.java** - -Remove these lines (138–142): -```java - // --- NL Search --- - /** Ollama is unreachable or timed out. 503 */ - SMART_SEARCH_UNAVAILABLE, - /** NL search rate limit exceeded (5 requests per user per minute). 429 */ - SMART_SEARCH_RATE_LIMITED, -``` - -The block between `TAG_MERGE_INVALID_TARGET,` and `// --- Generic ---` becomes empty. - -- [ ] **Step 2: Remove nlp and nl-search config from application.yaml** - -Remove these lines (133–138): -```yaml - nlp: - base-url: http://nlp-service:8001 - - nl-search: - rate-limit: - max-requests-per-minute: 20 -``` - -- [ ] **Step 3: Remove nlp config from application-dev.yaml** - -Remove these lines (15–17): -```yaml -app: - nlp: - base-url: http://localhost:8001 -``` - -Note: only remove the `nlp:` sub-key under `app:`, preserving any other `app:` config above it. - -- [ ] **Step 4: Verify backend still compiles** - -```bash -cd backend && source ~/.sdkman/bin/sdkman-init.sh && ./mvnw compile -q -``` - -Expected: BUILD SUCCESS. - -- [ ] **Step 5: Commit** - -```bash -git add backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java \ - backend/src/main/resources/application.yaml \ - backend/src/main/resources/application-dev.yaml -git commit -m "refactor(search): remove NLP error codes and application config" -``` - ---- - -### Task 3: Delete frontend NL search components and utilities - -**Files:** -- Delete: `frontend/src/routes/search/SmartModeToggle.svelte` + `.spec.ts` -- Delete: `frontend/src/routes/search/SmartSearchStatus.svelte` + `.spec.ts` -- Delete: `frontend/src/routes/search/InterpretationChipRow.svelte` + `.spec.ts` -- Delete: `frontend/src/routes/search/DisambiguationPicker.svelte` + `.spec.ts` -- Delete: `frontend/src/routes/search/chip-types.ts` -- Delete: `frontend/src/routes/documents/theme-chip-removal.ts` + `.spec.ts` - -- [ ] **Step 1: Delete all NL search components, specs, and utilities** - -```bash -rm frontend/src/routes/search/SmartModeToggle.svelte \ - frontend/src/routes/search/SmartModeToggle.svelte.spec.ts \ - frontend/src/routes/search/SmartSearchStatus.svelte \ - frontend/src/routes/search/SmartSearchStatus.svelte.spec.ts \ - frontend/src/routes/search/InterpretationChipRow.svelte \ - frontend/src/routes/search/InterpretationChipRow.svelte.spec.ts \ - frontend/src/routes/search/DisambiguationPicker.svelte \ - frontend/src/routes/search/DisambiguationPicker.svelte.spec.ts \ - frontend/src/routes/search/chip-types.ts \ - frontend/src/routes/documents/theme-chip-removal.ts \ - frontend/src/routes/documents/theme-chip-removal.spec.ts -``` - -- [ ] **Step 2: Commit** - -```bash -git add -A -git commit -m "refactor(search): delete frontend NLP search components and utilities" -``` - ---- - -### Task 4: Remove NL search from SearchFilterBar - -**Files:** -- Modify: `frontend/src/routes/SearchFilterBar.svelte` -- Modify: `frontend/src/routes/SearchFilterBar.svelte.spec.ts:199-233` - -- [ ] **Step 1: Rewrite SearchFilterBar.svelte** - -Replace the entire ` -``` - -- [ ] **Step 2: Update the search input element in the template** - -Replace the `` element (lines 92–105) with: -```svelte - -``` - -- [ ] **Step 3: Remove the SmartModeToggle component from the template** - -Delete this line (135): -```svelte - -``` - -- [ ] **Step 4: Remove smart-mode describe block from SearchFilterBar.svelte.spec.ts** - -Delete lines 199–233 (the entire final `describe` block): -```typescript -describe('SearchFilterBar – smart-mode chip lifecycle hooks', () => { - // ... -}); -``` - -- [ ] **Step 5: Run the SearchFilterBar tests to verify they pass** - -```bash -cd frontend && source ~/.nvm/nvm.sh && npm run test -- --project=client src/routes/SearchFilterBar.svelte.spec.ts -``` - -Expected: all tests pass, no failures. - -- [ ] **Step 6: Commit** - -```bash -git add frontend/src/routes/SearchFilterBar.svelte \ - frontend/src/routes/SearchFilterBar.svelte.spec.ts -git commit -m "refactor(search): remove smart mode from SearchFilterBar" -``` - ---- - -### Task 5: Remove NL search from documents/+page.svelte - -**Files:** -- Modify: `frontend/src/routes/documents/+page.svelte` - -This is the largest edit. Remove all NL search state, derived values, functions, and the NL results template block. - -- [ ] **Step 1: Remove NL search imports (lines 11–16, 23–27)** - -Remove these import lines: -```typescript -import SmartSearchStatus from '../search/SmartSearchStatus.svelte'; -import InterpretationChipRow from '../search/InterpretationChipRow.svelte'; -import type { ChipType } from '../search/chip-types.js'; -import { buildThemeRemovalUrl } from './theme-chip-removal.js'; -import DisambiguationPicker from '../search/DisambiguationPicker.svelte'; -``` - -Remove these type aliases: -```typescript -type NlQueryInterpretation = components['schemas']['NlQueryInterpretation']; -type NlSearchResponse = components['schemas']['NlSearchResponse']; -type DocumentSearchResult = components['schemas']['DocumentSearchResult']; -type PersonHint = components['schemas']['PersonHint']; -type SmartSearchErrorCode = 'SMART_SEARCH_UNAVAILABLE' | 'SMART_SEARCH_RATE_LIMITED'; -``` - -Also remove the `import { csrfFetch } from '$lib/shared/cookies';` line — it is only used by `runSmartSearch`. - -- [ ] **Step 2: Remove all NL state and derived values (lines 51–70)** - -Remove these declarations: -```typescript -// Smart (NL) search — UI-local state, resets on real page navigation (away + back). -let smartMode = $state(false); -let nlSubmitted = $state(false); -let nlLoading = $state(false); -let nlError = $state(null); -let nlInterpretation = $state(null); -let nlResult = $state(null); - -const showNlView = $derived(smartMode && nlSubmitted); -const nlHasResults = $derived((nlResult?.items.length ?? 0) > 0); -const ambiguousPersons = $derived(nlInterpretation?.ambiguousPersons ?? []); -const nlIsAmbiguous = $derived(ambiguousPersons.length > 0); -const disambiguationHeading = $derived( - ambiguousPersons.length === 1 - ? m.search_disambiguation_did_you_mean({ name: ambiguousPersons[0].displayName }) - : m.search_disambiguation_heading() -); -const showDisambiguationCue = $derived(ambiguousPersons.length >= 2); -``` - -- [ ] **Step 3: Remove all NL search functions (lines 202–318)** - -Remove these functions entirely: -- `resetNlState()` -- `onModeToggle()` -- `runSmartSearch()` -- `switchToKeywordMode()` -- `applyResolvedAndSearch()` -- `paramsFromInterpretation()` -- `removeChip()` -- `selectDisambiguated()` - -- [ ] **Step 4: Update SearchFilterBar usage in the template** - -Replace the SearchFilterBar call with (removing `bind:smartMode`, `onSmartSearch`, `onModeToggle`): -```svelte - (qFocused = true)} - onblur={() => (qFocused = false)} - /> -``` - -- [ ] **Step 5: Remove the NL results template block** - -Replace the entire `{#if showNlView}...{:else}...{/if}` block with just the content of the `{:else}` branch — the `