mirror of
https://github.com/Crosstalk-Solutions/project-nomad.git
synced 2026-05-25 22:05:07 +02:00
fix(KB): surface file-warning compute failures instead of masking as healthy (PR #895 review)
`computeFileWarnings()` previously caught all errors and returned an empty
map, which the frontend rendered as "every file is healthy" — reintroducing
exactly the silent-failure mode this surface exists to expose.
Return `{ ok, warnings }`; flip `ok: false` from the catch. KB modal renders
an inline amber notice under the Stored Files header when `ok === false`,
leaving per-row warning rendering untouched. Transient failures self-heal on
the next 30s poll; no toast spam.
This commit is contained in:
parent
cbd86b7af9
commit
cbae48a3c8
|
|
@ -69,8 +69,8 @@ export default class RagController {
|
||||||
}
|
}
|
||||||
|
|
||||||
public async getFileWarnings({ response }: HttpContext) {
|
public async getFileWarnings({ response }: HttpContext) {
|
||||||
const warnings = await this.ragService.computeFileWarnings()
|
const result = await this.ragService.computeFileWarnings()
|
||||||
return response.status(200).json({ warnings })
|
return response.status(200).json(result)
|
||||||
}
|
}
|
||||||
|
|
||||||
public async deleteFile({ request, response }: HttpContext) {
|
public async deleteFile({ request, response }: HttpContext) {
|
||||||
|
|
|
||||||
|
|
@ -19,7 +19,8 @@ import KVStore from '#models/kv_store'
|
||||||
import KbIngestState from '#models/kb_ingest_state'
|
import KbIngestState from '#models/kb_ingest_state'
|
||||||
import { decideScanAction, type IngestPolicy } from '../utils/kb_ingest_decision.js'
|
import { decideScanAction, type IngestPolicy } from '../utils/kb_ingest_decision.js'
|
||||||
import KbRatioRegistry from '#models/kb_ratio_registry'
|
import KbRatioRegistry from '#models/kb_ratio_registry'
|
||||||
import { decideWarnings, type FileWarning } from '../utils/kb_warning_decision.js'
|
import { decideWarnings } from '../utils/kb_warning_decision.js'
|
||||||
|
import type { FileWarning, FileWarningsResult } from '../../types/rag.js'
|
||||||
import { ZIMExtractionService } from './zim_extraction_service.js'
|
import { ZIMExtractionService } from './zim_extraction_service.js'
|
||||||
import { ZIM_BATCH_SIZE } from '../../constants/zim_extraction.js'
|
import { ZIM_BATCH_SIZE } from '../../constants/zim_extraction.js'
|
||||||
import { ProcessAndEmbedFileResponse, ProcessZIMFileResponse, RAGResult, RerankedRAGResult } from '../../types/rag.js'
|
import { ProcessAndEmbedFileResponse, ProcessZIMFileResponse, RAGResult, RerankedRAGResult } from '../../types/rag.js'
|
||||||
|
|
@ -1090,16 +1091,18 @@ export class RagService {
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Compute conditional warnings (RFC #883 §6) for every source the scanner
|
* Compute conditional warnings (RFC #883 §6) for every source the scanner
|
||||||
* sees on disk. Returns a map from source path → list of warnings, with
|
* sees on disk. Returns `{ ok, warnings }` — `ok: false` distinguishes a
|
||||||
* sources that have no warnings omitted entirely (so the frontend can
|
* computation failure (Qdrant unreachable, DB outage, FS error) from the
|
||||||
* `warningsBySource[source] ?? []` for clean defaults).
|
* healthy-but-empty case, which is critical because the whole point of this
|
||||||
|
* surface is to expose silent failures; reporting "everything healthy" when
|
||||||
|
* we couldn't actually check would reintroduce the bug we set out to fix.
|
||||||
*
|
*
|
||||||
* Per-source chunk counts come from a single Qdrant scroll over the
|
* Per-source chunk counts come from a single Qdrant scroll over the
|
||||||
* collection's points; expected-chunk estimates come from the ratio
|
* collection's points; expected-chunk estimates come from the ratio
|
||||||
* registry. Files in the scanner's directories that have no qdrant points
|
* registry. Files in the scanner's directories that have no qdrant points
|
||||||
* at all show up with `chunksInQdrant: 0` so Warning A can fire.
|
* at all show up with `chunksInQdrant: 0` so Warning A can fire.
|
||||||
*/
|
*/
|
||||||
public async computeFileWarnings(): Promise<Record<string, FileWarning[]>> {
|
public async computeFileWarnings(): Promise<FileWarningsResult> {
|
||||||
try {
|
try {
|
||||||
await this._ensureCollection(
|
await this._ensureCollection(
|
||||||
RagService.CONTENT_COLLECTION_NAME,
|
RagService.CONTENT_COLLECTION_NAME,
|
||||||
|
|
@ -1165,10 +1168,10 @@ export class RagService {
|
||||||
if (warnings.length > 0) out[source] = warnings
|
if (warnings.length > 0) out[source] = warnings
|
||||||
}
|
}
|
||||||
|
|
||||||
return out
|
return { ok: true, warnings: out }
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
logger.error('[RAG] Error computing file warnings:', error)
|
logger.error('[RAG] Error computing file warnings:', error)
|
||||||
return {}
|
return { ok: false, warnings: {} }
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -51,15 +51,17 @@ export default function KnowledgeBaseModal({ aiAssistantName = "AI Assistant", o
|
||||||
select: (data) => data || [],
|
select: (data) => data || [],
|
||||||
})
|
})
|
||||||
|
|
||||||
// Per-file conditional warnings (RFC #883 §6). Only sources with at least
|
// Per-file conditional warnings (RFC #883 §6). `ok: false` means the
|
||||||
// one triggered warning are returned, so an empty map means everything is
|
// computation itself failed (Qdrant/DB/FS) — distinct from `ok: true` with
|
||||||
// healthy. Polled at the same idle cadence as health for low overhead.
|
// an empty map, which means everything is healthy. We surface the failure
|
||||||
const { data: fileWarnings = {} } = useQuery({
|
// explicitly so a silent backend failure doesn't masquerade as health.
|
||||||
|
const { data: warningsResult } = useQuery({
|
||||||
queryKey: ['kbFileWarnings'],
|
queryKey: ['kbFileWarnings'],
|
||||||
queryFn: () => api.getKbFileWarnings(),
|
queryFn: () => api.getKbFileWarnings(),
|
||||||
select: (data) => data ?? {},
|
|
||||||
refetchInterval: 30_000,
|
refetchInterval: 30_000,
|
||||||
})
|
})
|
||||||
|
const fileWarnings = warningsResult?.warnings ?? {}
|
||||||
|
const warningsUnavailable = warningsResult !== undefined && warningsResult.ok === false
|
||||||
|
|
||||||
// Global auto-index policy. KVStore returns `null` for an unset key, which
|
// Global auto-index policy. KVStore returns `null` for an unset key, which
|
||||||
// we treat as 'Always' for backward compatibility with installs that predate
|
// we treat as 'Always' for backward compatibility with installs that predate
|
||||||
|
|
@ -444,7 +446,15 @@ export default function KnowledgeBaseModal({ aiAssistantName = "AI Assistant", o
|
||||||
|
|
||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
<StyledTable<KbFileGroup>
|
{warningsUnavailable && (
|
||||||
|
<div className="mb-4 inline-flex items-center gap-2 text-xs text-amber-700 dark:text-amber-300 bg-amber-50 dark:bg-amber-950/40 border border-amber-200 dark:border-amber-800 rounded px-3 py-2">
|
||||||
|
<span aria-hidden="true">⚠</span>
|
||||||
|
<span>
|
||||||
|
File warnings unavailable — couldn't read storage state. Retrying…
|
||||||
|
</span>
|
||||||
|
</div>
|
||||||
|
)}
|
||||||
|
<StyledTable<{ source: string }>
|
||||||
className="font-semibold"
|
className="font-semibold"
|
||||||
rowLines={true}
|
rowLines={true}
|
||||||
columns={[
|
columns={[
|
||||||
|
|
|
||||||
|
|
@ -5,7 +5,7 @@ import { FileEntry } from '../../types/files'
|
||||||
import { CheckLatestVersionResult, SystemInformationResponse, SystemUpdateStatus } from '../../types/system'
|
import { CheckLatestVersionResult, SystemInformationResponse, SystemUpdateStatus } from '../../types/system'
|
||||||
import { DownloadJobWithProgress, WikipediaState } from '../../types/downloads'
|
import { DownloadJobWithProgress, WikipediaState } from '../../types/downloads'
|
||||||
import type { Country, CountryCode, CountryGroup, MapExtractPreflight } from '../../types/maps'
|
import type { Country, CountryCode, CountryGroup, MapExtractPreflight } from '../../types/maps'
|
||||||
import { EmbedJobWithProgress, FileWarning } from '../../types/rag'
|
import { EmbedJobWithProgress, FileWarningsResult } from '../../types/rag'
|
||||||
import type { CategoryWithStatus, CollectionWithStatus, ContentUpdateCheckResult, ResourceUpdateInfo } from '../../types/collections'
|
import type { CategoryWithStatus, CollectionWithStatus, ContentUpdateCheckResult, ResourceUpdateInfo } from '../../types/collections'
|
||||||
import { catchInternal } from './util'
|
import { catchInternal } from './util'
|
||||||
import { NomadChatResponse, NomadInstalledModel, NomadOllamaModel, OllamaChatRequest } from '../../types/ollama'
|
import { NomadChatResponse, NomadInstalledModel, NomadOllamaModel, OllamaChatRequest } from '../../types/ollama'
|
||||||
|
|
@ -477,10 +477,8 @@ class API {
|
||||||
|
|
||||||
async getKbFileWarnings() {
|
async getKbFileWarnings() {
|
||||||
return catchInternal(async () => {
|
return catchInternal(async () => {
|
||||||
const response = await this.client.get<{ warnings: Record<string, FileWarning[]> }>(
|
const response = await this.client.get<FileWarningsResult>('/rag/file-warnings')
|
||||||
'/rag/file-warnings'
|
return response.data
|
||||||
)
|
|
||||||
return response.data.warnings
|
|
||||||
})()
|
})()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -44,4 +44,16 @@ export type RerankedRAGResult = Omit<RAGResult, 'keywords'> & {
|
||||||
|
|
||||||
export type FileWarning =
|
export type FileWarning =
|
||||||
| { kind: 'zero_chunks'; fileSizeBytes: number }
|
| { kind: 'zero_chunks'; fileSizeBytes: number }
|
||||||
| { kind: 'partial_stall'; chunksEmbedded: number; chunksExpected: number }
|
| { kind: 'partial_stall'; chunksEmbedded: number; chunksExpected: number }
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Result of computing per-file warnings. `ok: false` means the computation
|
||||||
|
* itself failed (Qdrant unreachable, DB outage, FS read error) — distinct from
|
||||||
|
* `ok: true` with an empty map, which means every scanned file is healthy.
|
||||||
|
* The frontend should surface a neutral "warnings unavailable" indicator on
|
||||||
|
* `!ok` rather than implying everything is fine.
|
||||||
|
*/
|
||||||
|
export type FileWarningsResult = {
|
||||||
|
ok: boolean
|
||||||
|
warnings: Record<string, FileWarning[]>
|
||||||
|
}
|
||||||
Loading…
Reference in New Issue
Block a user