From 63ed059cfee34712225bf8df8c0ef7820fa51837 Mon Sep 17 00:00:00 2001 From: John Siracusa Date: Sat, 8 Aug 2026 16:30:41 -0400 Subject: [PATCH 01/10] feat(engine): serve search and report source state honestly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A QA pass driving the real app found the engine staying quiet where it should speak up, and one write path that trusted its callers. - add GET /api/search over the resolved cascade: the ranking search.mjs already gives MCP clients was unreachable over HTTP, so the console could only filter titles - a layer folder deleted underneath a running engine now reports "Layer folder no longer exists" instead of reading as an empty folder; a subdirectory vanishing mid-walk is still tolerated - count skipped dot-entries per source (skippedHidden) so a vault with content under a dot-directory is not silently short - track lastSuccessAt on the index entry: local sources have no health() of their own, so /api/graph reported "Not yet" beside a synced badge - PUT /api/file requires the modified timestamp it already documents, unless force: true — the guard was skipped whenever a caller omitted the field, which is exactly when it was needed Co-Authored-By: Claude Opus 5 Signed-off-by: John Siracusa --- packages/core/src/layer-files.mjs | 17 ++++- packages/core/src/service.mjs | 70 ++++++++++++++++++-- packages/core/src/sources/okf-local.mjs | 24 +++++-- packages/core/tests/files-source-test.sh | 33 ++++++++- packages/core/tests/service-test.sh | 33 ++++++++- packages/core/tests/setup-robustness-test.sh | 9 ++- 6 files changed, 164 insertions(+), 22 deletions(-) diff --git a/packages/core/src/layer-files.mjs b/packages/core/src/layer-files.mjs index 64eccab..adbd504 100644 --- a/packages/core/src/layer-files.mjs +++ b/packages/core/src/layer-files.mjs @@ -8,7 +8,8 @@ // GET /api/files → { layers: [{ layer, kind, root, fileCount, files[] }] } // GET /api/file?path=/ → { path, layer, rel, ext, kind, editable, text? } // GET /api/file/raw?path=… → the bytes (images/PDF preview) -// PUT /api/file → { path, text, modified? } overwrite an existing text file +// PUT /api/file → { path, text, modified } overwrite an existing text file +// (modified required unless force: true) // PUT /api/section → write one resolved section across layers // // Every path is resolved against its layer root and checked with @@ -185,8 +186,18 @@ export async function writeFileApi(rawBody, roots) { let stat; try { stat = await fsp.stat(abs); } catch { throw httpError(404, `Refusing to create new files: ${apiPath}`); } if (!stat.isFile()) throw httpError(400, `Not a file: ${apiPath}`); - if (body.modified !== undefined && body.modified !== stat.mtime.toISOString()) { - throw httpError(409, `This file changed on disk after you opened it. Reopen ${apiPath} and merge your edit.`); + // `modified` used to be optional, which meant a client that forgot to send + // it silently skipped the stale-editor check below and overwrote whatever + // changed on disk since it last read the file. It is required now — the + // caller must either name the version it read (from GET /api/file) or say + // out loud that it means to overwrite regardless. + if (body.force !== true) { + if (body.modified === undefined) { + throw httpError(400, `Provide modified: the file's last-read modified timestamp (from GET /api/file), or force: true to overwrite deliberately.`); + } + if (body.modified !== stat.mtime.toISOString()) { + throw httpError(409, `This file changed on disk after you opened it. Reopen ${apiPath} and merge your edit.`); + } } await fsp.writeFile(abs, body.text, "utf8"); const after = await fsp.stat(abs); diff --git a/packages/core/src/service.mjs b/packages/core/src/service.mjs index 7ed4b56..8d0ada8 100644 --- a/packages/core/src/service.mjs +++ b/packages/core/src/service.mjs @@ -30,6 +30,7 @@ import { probeDocs, MAX_DOC_BYTES } from "./sources/okf-local.mjs"; import { FILES_EXTENSIONS } from "./sources/files.mjs"; import { createMcpSource } from "./sources/mcp.mjs"; import { mergeConcepts, resolveConcept } from "./resolver.mjs"; +import { searchConcepts } from "./search.mjs"; import { countTokens, conceptText, warmTokenizer, TOKENIZER } from "./tokenize.mjs"; import { resolveSettings, walkLimitsFrom, validateSettingsPatch, settingsCatalog } from "./settings.mjs"; import { @@ -148,7 +149,7 @@ async function snapshotSource(source, entry, signal = null) { // `notes` rides along the same way: what a walk had to leave out — a document // over the size cap, a subtree it lacks permission to read — belongs on the // snapshot, because the row the user sees is built from the snapshot. - const notes = { skipped: [], unreadable: [] }; + const notes = { skipped: [], unreadable: [], hidden: 0 }; const ids = typeof source.listConceptIds === "function" ? await source.listConceptIds({ signal, notes }) : []; throwIfAborted(); entry.phase = "loading"; @@ -178,7 +179,7 @@ async function snapshotSource(source, entry, signal = null) { // buildGraph key a memo on live state instead of on invalidation events. return { ids, concepts, tokens, tokensById, gen: ++SNAPSHOT_SEQ, - skipped: notes.skipped, unreadable: notes.unreadable, + skipped: notes.skipped, unreadable: notes.unreadable, hidden: notes.hidden, }; } @@ -616,7 +617,9 @@ export function createEngineService({ * instead of cancelling, and the entry owes exactly one follow-up when this * pass lands (see scheduleFollowUp for when it actually runs). */ - function startIndex(source, key, settings, { validity = null, previousSnap = null, passes = 1 } = {}) { + function startIndex(source, key, settings, { + validity = null, previousSnap = null, previousSuccessAt = null, passes = 1, + } = {}) { const controller = new AbortController(); const refreshing = previousSnap !== null; const entry = { @@ -626,6 +629,10 @@ export function createEngineService({ loaded: 0, total: null, snap: previousSnap, + // Carried across a refresh/rename the same way `snap` is, so a source + // with no health() of its own does not forget its last real success the + // moment a follow-up pass starts. + lastSuccessAt: previousSuccessAt, validity, // Whether a pass is in flight, tracked separately from `status` — which // now says "ready" through a background refresh — because every decision @@ -657,6 +664,7 @@ export function createEngineService({ entry.snap = snap; entry.status = "ready"; entry.phase = "ready"; + entry.lastSuccessAt = new Date().toISOString(); }) .catch((err) => { if (indexes.get(entry.key) !== entry) return; @@ -733,6 +741,7 @@ export function createEngineService({ indexes.set(key, startIndex(open.sources[i], key, open.settings, { validity: open.validities[i], previousSnap: entry.snap, + previousSuccessAt: entry.lastSuccessAt, passes: entry.passes + 1, })); } @@ -811,6 +820,7 @@ export function createEngineService({ indexes.set(key, startIndex(source, key, open.settings, { validity: open.validities[i], previousSnap: previous?.snap ?? null, + previousSuccessAt: previous?.lastSuccessAt ?? null, passes: (previous?.passes ?? 0) + 1, })); }); @@ -938,6 +948,12 @@ export function createEngineService({ error: entry.error, progress: indexProgress(entry), health: typeof source.health === "function" ? source.health() : null, + // The index's own record of the last successful pass, kept for sources + // with no health() of their own (okf-local, files): without it a local + // layer's lastSuccessAt read null forever, even after every index since + // boot succeeded, because health (the field graph/status prefer first) + // never exists for those adapters. + lastSuccessAt: entry.lastSuccessAt, }; } @@ -976,6 +992,7 @@ export function createEngineService({ if (p === "/api/status") { json(res, 200, statusApi()); return true; } if (p === "/api/resolve") { json(res, 200, await resolveOne(url.searchParams.get("concept"))); return true; } if (p === "/api/resolve-all") { json(res, 200, await resolveAllApi(waitParam(url))); return true; } + if (p === "/api/search") { json(res, 200, await searchApi(url)); return true; } if (p === "/api/discrepancies" && req.method === "GET") { json(res, 200, await discrepanciesApi(waitParam(url))); return true; @@ -1115,7 +1132,7 @@ export function createEngineService({ const contributing = perSource.filter((p) => p.snap); const { concepts, resolvedTokens, latestPerSource } = await resolvedIndex(contributing); - const sourcesOut = perSource.map(({ source: s, snap, status, error, progress, health }) => { + const sourcesOut = perSource.map(({ source: s, snap, status, error, progress, health, lastSuccessAt }) => { const meta = layerMeta.get(s.name) ?? {}; const kind = s.quarantinedKind ?? meta.source ?? "okf-local"; // `health` was read with the rest of this row (see pinEntry): whether the @@ -1180,10 +1197,16 @@ export function createEngineService({ warnings: warningMessages.length, warningMessages: warningMessages.slice(0, 10), indexing: progress, + // Dotfiles/dot-dirs skipped by the walk — a count, not a listing. + skippedHidden: snap?.hidden ?? 0, // Enough for "last synced X, failed Y ago" without a second request. - // Null on sources that keep no health (local bundles, MCP children). + // lastErrorAt is null on sources that keep no health of their own + // (local bundles, MCP children) — there is nothing to have failed. + // lastSuccessAt falls back to the index's own record of its last + // successful pass for those same sources, so a local layer that has + // never failed still reports when it was last read. lastErrorAt: health?.lastErrorAt ?? null, - lastSuccessAt: health?.lastSuccessAt ?? null, + lastSuccessAt: health?.lastSuccessAt ?? lastSuccessAt ?? null, }; }); @@ -1371,6 +1394,10 @@ export function createEngineService({ // re-reading behind it. Never a reason to show a source as unready. refreshing: progress.refreshing, error: degraded ? health.lastError : error ?? null, + // Dotfiles/dot-dirs are still skipped silently — this only makes the + // count visible, never what is inside them. Additive, and already + // computed by the walk, so it costs nothing on this cheap route. + skippedHidden: snap?.hidden ?? 0, }; }); // A source with nothing to serve yet. A source refreshing behind a good @@ -1397,6 +1424,24 @@ export function createEngineService({ return resolved; } + // Same live-sources read as resolveOne — search.mjs is the ranking module + // the retrieval eval scores, so this route only ever calls it, never + // reimplements any part of it. + async function searchApi(url) { + const query = url.searchParams.get("q"); + if (typeof query !== "string" || !query.trim()) throw httpError(400, "Provide ?q="); + let limit = 10; + const rawLimit = url.searchParams.get("limit"); + if (rawLimit !== null) { + const n = Number(rawLimit); + if (!Number.isFinite(n) || n <= 0) throw httpError(400, "limit must be a positive number"); + limit = Math.min(Math.floor(n), 50); + } + const { sources } = openSources(); + const hits = await searchConcepts(sources, { query, limit }); + return { hits }; + } + function decorateResolvedDispositions(resolved, decisions) { for (const section of resolved.sections) { if (!section.conflicts?.length) continue; @@ -1783,7 +1828,10 @@ export function createEngineService({ if (!discrepancy || body.revision !== discrepancy.revision) { throw httpError(409, "This discrepancy changed after you opened it. Reload before deciding it."); } - const allowedReasons = new Set(["different_scopes", "temporary_migration", "source_specific_authority", "other"]); + // target_missing is broken-link-shaped: acknowledging why a link target + // will never resolve (the concept was retired, renamed elsewhere, etc.) + // needs a reason distinct from a genuine scoped disagreement. + const allowedReasons = new Set(["different_scopes", "temporary_migration", "source_specific_authority", "target_missing", "other"]); if (action === "acknowledge" && !allowedReasons.has(reasonCode)) throw httpError(400, "Choose why this scoped difference should remain"); const chosen = action === "choose_contribution" ? discrepancy.contributions.find((item) => item.source === selectedSource) @@ -1828,6 +1876,14 @@ export function createEngineService({ return { ok: true, decision: await conflictResolutionLog.append(decision), written: [] }; } if (originalKind === "broken_link") throw httpError(409, "Open the source file to repair this link, or acknowledge the scoped difference."); + // renderScalar's scalar branch would rewrite a YAML list as a quoted + // string, silently downgrading the field's type in every writable layer — + // compose only ever produces a string, so a list-typed field has no safe + // reconciled answer to write. + if (originalKind === "frontmatter_value" && action === "compose" + && discrepancy.contributions.some((item) => Array.isArray(item.value))) { + throw httpError(400, "This field is a list; compose isn't available for list values — use \"Use this answer everywhere\" or edit the file directly."); + } const value = action === "compose" ? content : chosen.value; const writableSources = discrepancy.contributions.map((item) => item.source).filter((source) => fileRoots().has(source)); diff --git a/packages/core/src/sources/okf-local.mjs b/packages/core/src/sources/okf-local.mjs index 3e427f2..573153a 100644 --- a/packages/core/src/sources/okf-local.mjs +++ b/packages/core/src/sources/okf-local.mjs @@ -372,6 +372,7 @@ export async function walkDocs(root, extensions, limits = null, { signal = null, const { maxFiles, maxEntries } = { ...defaultWalkLimits(), ...(limits ?? {}) }; const files = []; let scanned = 0; + let hiddenSkipped = 0; const stack = [root]; while (stack.length > 0) { throwIfAborted(signal); @@ -380,10 +381,18 @@ export async function walkDocs(root, extensions, limits = null, { signal = null, try { dirents = await fsp.readdir(current, { withFileTypes: true }); } catch (err) { - // Skipping is still right — one locked folder must not fail the whole - // layer — but a folder we are not ALLOWED to read is a different fact - // from a folder with nothing in it, and only one of them is worth saying. - // A missing root is already reported as an empty source. + // The layer ROOT failing to read is a different fact from an ordinary + // subdirectory going missing mid-walk: the root is the whole source, so + // losing it means there is nothing left behind this layer at all — not + // the empty-folder answer a deleted vault used to get, indistinguishable + // from one that was simply never populated. + if (current === root && (err.code === "ENOENT" || err.code === "ENOTDIR")) { + throw new Error(`Layer folder no longer exists: ${root}`); + } + // Skipping is still right for everything else — one locked or vanished + // subfolder must not fail the whole layer — but a folder we are not + // ALLOWED to read is a different fact from a folder with nothing in it, + // and only one of them is worth saying. if (err.code === "EACCES" || err.code === "EPERM") { notes?.unreadable.push({ rel: relTo(root, current), code: err.code }); } @@ -391,7 +400,8 @@ export async function walkDocs(root, extensions, limits = null, { signal = null, } const candidates = []; for (const dirent of dirents) { - if (dirent.name.startsWith(".") || dirent.name === "node_modules") continue; + if (dirent.name.startsWith(".")) { hiddenSkipped += 1; continue; } + if (dirent.name === "node_modules") continue; scanned += 1; if (scanned > maxEntries) { throw new Error( @@ -421,6 +431,10 @@ export async function walkDocs(root, extensions, limits = null, { signal = null, } } } + // A skip an entry never mentions is a skip nobody can act on. Counted, not + // itemized — one number per source is enough to say "this is deliberate", + // and the walk never descends into a dot-dir to name what is inside it. + if (notes) notes.hidden = (notes.hidden ?? 0) + hiddenSkipped; return files.sort(); } diff --git a/packages/core/tests/files-source-test.sh b/packages/core/tests/files-source-test.sh index d72b388..ec6ecfd 100755 --- a/packages/core/tests/files-source-test.sh +++ b/packages/core/tests/files-source-test.sh @@ -285,6 +285,37 @@ for evil in "../secret" ".." "/etc/passwd" "a/.." "decisions/../../secret"; do [ "$out" = "null" ] || fail "traversal id '$evil' should load as null" "$out" done +# --- 5b. A vanished (or never-existed) layer root is a status error, not an -- +# empty read (F19). walkDocs used to catch-and-continue on ANY readdir failure, +# including the layer ROOT itself — so a deleted source folder came back as an +# empty listing, indistinguishable from a layer that was genuinely empty. The +# root failing is different: there is nothing behind this source at all. + +cat > "$tmpdir/vanished.mjs" < "$tmpdir/cache.mjs" <s+=d).on("end",()=>{const g=JSON.parse(s);process.stdout.write(`${g.tokenizer}:${g.totals.sources}:${g.totals.sourceTokens>0}`)})')" [ "$G" = "o200k_base:1:true" ] && pass "graph payload intact behind auth" || fail "graph payload ($G)" # A source with no health of its own (a local bundle) reports plain "ok" and -# null health fields — "degraded" is reserved for an adapter that says so. -H="$(curl -s "${AUTH[@]}" "$BASE/api/graph" | node -e 'let s="";process.stdin.on("data",d=>s+=d).on("end",()=>{const x=JSON.parse(s).sources[0];process.stdout.write(`${x.status}:${x.error}:${x.lastErrorAt}:${x.lastSuccessAt}`)})')" -[ "$H" = "ok:null:null:null" ] && pass "healthless source reports plain ok" || fail "local source health shape ($H)" +# no error — "degraded" is reserved for an adapter that says so. lastSuccessAt +# still gets set, from the index's own record of its last successful pass +# rather than from health() (which this source has none of). +H="$(curl -s "${AUTH[@]}" "$BASE/api/graph" | node -e 'let s="";process.stdin.on("data",d=>s+=d).on("end",()=>{const x=JSON.parse(s).sources[0];process.stdout.write(`${x.status}:${x.error}:${x.lastErrorAt}:${x.lastSuccessAt!==null}`)})')" +[ "$H" = "ok:null:null:true" ] && pass "healthless source reports plain ok, with its own lastSuccessAt" || fail "local source health shape ($H)" code 401 "$(C "$BASE/api/resolve?concept=note")" "resolve without header rejected" code 200 "$(C "${AUTH[@]}" "$BASE/api/resolve?concept=note")" "resolve with Bearer accepted" code 401 "$(C "$BASE/api/host-only")" "unknown /api/* path still gated without token" code 401 "$(C "$BASE/api/files")" "layer file API gated without token" +echo "GET /api/search (F4)" +code 401 "$(C "$BASE/api/search?q=hello")" "search without header rejected" +code 400 "$(C "${AUTH[@]}" "$BASE/api/search")" "search with no q is refused" +code 400 "$(C "${AUTH[@]}" "$BASE/api/search?q=")" "search with an empty q is refused" +SR="$(curl -s "${AUTH[@]}" "$BASE/api/search?q=hello")" +[ "$(JQ 'String(d.hits.length > 0 && d.hits[0].id === "note")' <<<"$SR")" = "true" ] && pass "search finds the seeded concept" || fail "search hit ($SR)" +[ "$(JQ 'String(Array.isArray(d.hits[0].layers))' <<<"$SR")" = "true" ] && pass "hits carry the shape searchConcepts returns" || fail "search hit shape ($SR)" +code 200 "$(C "${AUTH[@]}" "$BASE/api/search?q=hello&limit=1")" "search honors an explicit limit" +code 400 "$(C "${AUTH[@]}" "$BASE/api/search?q=hello&limit=-1")" "a non-positive limit is refused" + +echo "hidden-entry skips are counted, never indexed (F17)" +mkdir -p "$TMP/hidden/.obsidian" "$TMP/hidden/.git" +printf '# Visible\n\n## Body\n\nvisible content.\n' > "$TMP/hidden/visible.md" +printf '# Should never index\n\n## Body\n\nhidden content.\n' > "$TMP/hidden/.obsidian/workspace.md" +code 200 "$(C -X POST "${AUTH[@]}" -H 'content-type: application/json' -d "{\"kind\":\"files\",\"name\":\"hidden\",\"level\":1,\"path\":\"$TMP/hidden\"}" "$BASE/api/sources")" "add a source with dot-dirs beside real content" +HID="" +for _ in $(seq 1 60); do + HID="$(curl -s "${AUTH[@]}" "$BASE/api/status")" + [ "$(JQ 'd.sources.find((s) => s.name === "hidden")?.status ?? ""' <<<"$HID")" = "ok" ] && break + sleep 0.1 +done +[ "$(JQ 'String(d.sources.find((s) => s.name === "hidden")?.conceptCount)' <<<"$HID")" = "1" ] && pass "only the visible document is indexed" || fail "hidden folder indexed wrong count ($HID)" +[ "$(JQ 'String(d.sources.find((s) => s.name === "hidden")?.skippedHidden >= 2)' <<<"$HID")" = "true" ] && pass "/api/status counts the skipped dot-dirs" || fail "skippedHidden missing or wrong ($HID)" +code 200 "$(C -X DELETE "${AUTH[@]}" "$BASE/api/sources?name=hidden")" "cleanup hidden-entry source" + echo "console mount is static UI, not gated data" curl -s "$BASE/console/" | grep -q CONSOLE_OK && pass "/console/ serves without auth" || fail "/console/ without auth" diff --git a/packages/core/tests/setup-robustness-test.sh b/packages/core/tests/setup-robustness-test.sh index 21ce95c..cab0cea 100644 --- a/packages/core/tests/setup-robustness-test.sh +++ b/packages/core/tests/setup-robustness-test.sh @@ -244,9 +244,12 @@ printf '# Meeting notes\n\n## Decision\n\nExternal edit wins.\n' > "$TMP/notes/m STALE_BODY="{\"path\":\"notes/meeting.md\",\"text\":\"stale editor text\",\"modified\":\"$(JQ 'd.modified' <<<"$OPEN")\"}" code 409 "$(C -X PUT -H 'content-type: application/json' -d "$STALE_BODY" "$BASE/api/file")" "a stale editor cannot overwrite an external edit" grep -q 'External edit wins' "$TMP/notes/meeting.md" && pass "the external edit remains intact after the conflict" || fail "stale save overwrote disk" -# Restore the fixture for the indexed-read assertions below. Omitting modified -# remains a supported compatibility path for older playground clients. -code 200 "$(C -X PUT -H 'content-type: application/json' -d "{\"path\":\"notes/meeting.md\",\"text\":\"# Meeting notes\n\n## Decision\n\nShip on Monday.\n\"}" "$BASE/api/file")" "legacy save without a revision remains supported" +# modified is required (F20) — a save that names neither modified nor force +# is refused rather than blindly overwriting whatever is on disk. +code 400 "$(C -X PUT -H 'content-type: application/json' -d "{\"path\":\"notes/meeting.md\",\"text\":\"# Meeting notes\n\n## Decision\n\nShip on Monday.\n\"}" "$BASE/api/file")" "a save without modified or force is refused" +# Restore the fixture for the indexed-read assertions below. force: true is +# the deliberate-overwrite escape hatch that replaces the old "omit modified" path. +code 200 "$(C -X PUT -H 'content-type: application/json' -d "{\"path\":\"notes/meeting.md\",\"text\":\"# Meeting notes\n\n## Decision\n\nShip on Monday.\n\",\"force\":true}" "$BASE/api/file")" "force:true restores the fixture despite no modified" code 404 "$(C -X PUT -H 'content-type: application/json' -d '{"path":"notes/brand-new.md","text":"nope"}' "$BASE/api/file")" "the editor refuses to create new files" code 403 "$(C "$BASE/api/file?path=notes/../../escape.md")" "traversal out of a layer root blocked" code 404 "$(C "$BASE/api/file?path=nosuchlayer/x.md")" "unknown layer rejected" From 93293a52beacf70f8ba707874c8145ede2eb0c37 Mon Sep 17 00:00:00 2001 From: John Siracusa Date: Sat, 8 Aug 2026 16:30:50 -0400 Subject: [PATCH 02/10] fix(engine): resolve each discrepancy exactly once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A decision carries both its canonical discrepancyId and the legacy conflictId, and decisionMap indexes it under both. The resolved-record reconstruction then walked every key, so one section-content resolution came back as two resolved rows — a curator counting a cleared queue saw six rows for four decisions. Dedupe by canonical id; map insertion order already puts the canonical entry first. Also adds "target_missing" to the acknowledgement reason enum, so a broken link can be acknowledged as a target that does not exist yet rather than the catch-all "Other". The transaction tests only ever asserted failure and rollback, which is how a composed value that never replaced anything would have gone unnoticed; they now assert committed file content, and the frontmatter transaction path gets its first coverage. Co-Authored-By: Claude Opus 5 Signed-off-by: John Siracusa --- packages/core/src/discrepancies.mjs | 11 +- packages/core/src/discrepancy-rules.mjs | 2 +- packages/core/tests/discrepancies.test.mjs | 41 +++++++ .../tests/discrepancy-transactions.test.mjs | 116 +++++++++++++++++- 4 files changed, 167 insertions(+), 3 deletions(-) diff --git a/packages/core/src/discrepancies.mjs b/packages/core/src/discrepancies.mjs index 425ce03..5f7c3a1 100644 --- a/packages/core/src/discrepancies.mjs +++ b/packages/core/src/discrepancies.mjs @@ -90,16 +90,25 @@ export function buildDiscrepancies(concepts, { // current output. Keep its evidence discoverable from the append-only log so // "resolved" never means "forgotten". A current finding always wins this // projection (including a reopened finding after a later source edit). + // decisionMap indexes a decision under BOTH its canonical discrepancyId and + // its legacy conflictId, so this loop would otherwise visit the same decided + // discrepancy twice and emit two resolved rows for one resolution. Dedupe on + // the canonical id — discrepancyId when the decision recorded one, else the + // map key itself. const currentIds = new Set(out.map((item) => item.id)); + const seenCanonical = new Set(); for (const [id, history] of decisionsById) { if (!id.includes("::") || currentIds.has(id)) continue; const latest = history.at(-1); if (latest?.schemaVersion !== 2 || latest.action === "acknowledge") continue; + const canonicalId = latest.discrepancyId ?? id; + if (seenCanonical.has(canonicalId)) continue; + seenCanonical.add(canonicalId); const contributions = (latest.contributions ?? []).map((item) => contribution( item.layer, item.level, item.updated, item.content, item.layer === latest.chosen?.layer, )); out.push({ - id, legacyId: latest.conflictId, kind: latest.discrepancyKind ?? "section_content", + id: canonicalId, legacyId: latest.conflictId, kind: latest.discrepancyKind ?? "section_content", originalKind: latest.discrepancyKind ?? "section_content", conceptId: latest.conceptId, conceptTitle: latest.title ?? latest.conceptId, conceptType: latest.conceptType ?? "concept", key: latest.sectionKey ?? latest.fieldKey ?? latest.linkTarget ?? "unknown", diff --git a/packages/core/src/discrepancy-rules.mjs b/packages/core/src/discrepancy-rules.mjs index 0ca81d1..0212724 100644 --- a/packages/core/src/discrepancy-rules.mjs +++ b/packages/core/src/discrepancy-rules.mjs @@ -127,7 +127,7 @@ function validateRule(rule) { if (rule.action.type === "prefer_source") { if (typeof rule.action.source !== "string" || !sources.includes(rule.action.source)) throw new Error("Invalid preferred source"); } else if (rule.action.type === "acknowledge") { - if (!["different_scopes", "temporary_migration", "source_specific_authority", "other"].includes(rule.action.reasonCode)) { + if (!["different_scopes", "temporary_migration", "source_specific_authority", "target_missing", "other"].includes(rule.action.reasonCode)) { throw new Error("Invalid acknowledgement reason"); } } else throw new Error("Invalid discrepancy rule action"); diff --git a/packages/core/tests/discrepancies.test.mjs b/packages/core/tests/discrepancies.test.mjs index d3f7401..bb23372 100644 --- a/packages/core/tests/discrepancies.test.mjs +++ b/packages/core/tests/discrepancies.test.mjs @@ -103,6 +103,47 @@ test("conflicting matching rules disable automation and surface the ambiguity", assert.equal(item.status, "needs_review"); }); +test("a resolved section_content decision emits exactly one record, under its canonical id", () => { + // schemaVersion-2, non-acknowledge, committed decision carrying BOTH the + // canonical discrepancyId and the legacy conflictId. decisionMap indexes it + // under both keys, so the resolved-record reconstruction must dedupe by + // canonical id or this becomes two rows for one resolution (F12). + const decisions = [{ + schemaVersion: 2, id: "d1", + discrepancyId: "section_content::decisions/settled::choice", + conflictId: "decisions/settled::choice", + action: "choose_contribution", transactionState: "committed", + conceptId: "decisions/settled", title: "Settled", conceptType: "decision", + sectionKey: "choice", sectionHeading: "Choice", owner: "Platform", + chosen: { layer: "team", content: "Use Postgres.", updated: "2026-08-01" }, + contributions: [ + { layer: "team", level: 2, content: "Use Postgres.", updated: "2026-08-01" }, + { layer: "company", level: 0, content: "Use MySQL.", updated: "2026-07-01" }, + ], + }]; + // No current conflict for this concept — buildDiscrepancies' first pass + // produces nothing for it, so the decision is only visible through the + // resolved-record reconstruction loop this test targets. + const settled = { ...concept, id: "decisions/settled", sections: [], frontmatterConflicts: [] }; + const result = buildDiscrepancies([settled], { decisions, coverageComplete: false }); + const rows = result.discrepancies.filter((item) => item.conceptId === "decisions/settled"); + assert.equal(rows.length, 1); + assert.equal(rows[0].id, "section_content::decisions/settled::choice"); + assert.equal(rows[0].legacyId, "decisions/settled::choice"); + assert.equal(rows[0].status, "resolved"); +}); + +test("active-discrepancy decoration still finds a decision by its legacy id", () => { + // finalize() looks a discrepancy's history up by discrepancyId first, then + // falls back to legacyId — the same fallback the dedupe above must not break. + const decisions = [{ + schemaVersion: 2, id: "d1", conflictId: "decisions/db::choice", action: "acknowledge", reasonCode: "other", + }]; + const result = buildDiscrepancies([concept], { decisions, coverageComplete: false }); + const section = result.discrepancies.find((item) => item.originalKind === "section_content"); + assert.equal(section.status, "acknowledged"); +}); + test("serialized shared rules contain structural metadata only", () => { const text = serializeRuleDocument([{ id: "r1", scope: "local", mode: "automatic", enabled: true, diff --git a/packages/core/tests/discrepancy-transactions.test.mjs b/packages/core/tests/discrepancy-transactions.test.mjs index 1664383..80cb0b1 100644 --- a/packages/core/tests/discrepancy-transactions.test.mjs +++ b/packages/core/tests/discrepancy-transactions.test.mjs @@ -1,10 +1,12 @@ import assert from "node:assert/strict"; import fsp from "node:fs/promises"; +import http from "node:http"; import os from "node:os"; import path from "node:path"; import test from "node:test"; -import { stageSectionTransaction } from "../src/layer-files.mjs"; +import { stageSectionTransaction, stageFrontmatterTransaction } from "../src/layer-files.mjs"; import { createDiscrepancyTransactionJournal } from "../src/conflict-resolutions.mjs"; +import { createEngineService } from "../src/service.mjs"; const document = (value) => `---\ntype: decision\ntitle: Database\n---\n\n# Database\n\n## Choice {#choice}\n\n${value}\n`; @@ -77,6 +79,118 @@ test("startup recovery preserves a write confirmed by the committed decision log } finally { await fsp.rm(dir, { recursive: true, force: true }); } }); +test("stageSectionTransaction commit writes exactly the new content, no fragment of old content (F10)", async () => { + const { dir, roots } = await fixture(); + try { + const staged = await stageSectionTransaction(JSON.stringify({ + conceptId: "database", sectionKey: "choice", layers: ["team", "company"], + content: "Use SQLite for embedded deployments.", expectedContent: { team: "Postgres", company: "MySQL" }, requireAll: true, + }), roots, "tx-happy-path"); + await staged.commit(); + await staged.cleanup(); + for (const { root } of roots.values()) { + const text = await fsp.readFile(path.join(root, "database.md"), "utf8"); + assert.match(text, /## Choice \{#choice\}\n\nUse SQLite for embedded deployments\.\n/); + assert.doesNotMatch(text, /Postgres/); + assert.doesNotMatch(text, /MySQL/); + } + } finally { await fsp.rm(dir, { recursive: true, force: true }); } +}); + +test("compose on a scalar frontmatter field replaces cleanly on disk", async () => { + const dir = await fsp.mkdtemp(path.join(os.tmpdir(), "cc-discrepancy-fm-")); + try { + const roots = new Map(); + for (const [name, owner] of [["team", "Platform"], ["company", "Architecture"]]) { + const root = path.join(dir, name); + await fsp.mkdir(root); + await fsp.writeFile( + path.join(root, "governed.md"), + `---\ntype: decision\ntitle: Governed\nowner: ${owner}\n---\n\n# Governed\n\n## Pick {#pick}\n\nsame everywhere\n`, + ); + roots.set(name, { root, kind: "okf-local" }); + } + const staged = await stageFrontmatterTransaction(JSON.stringify({ + conceptId: "governed", key: "owner", layers: ["team", "company"], value: "Composed Owner", + expectedValues: { team: "Platform", company: "Architecture" }, + }), roots, "tx-compose-scalar"); + await staged.commit(); + await staged.cleanup(); + for (const name of ["team", "company"]) { + const text = await fsp.readFile(path.join(dir, name, "governed.md"), "utf8"); + assert.equal(text, `---\ntype: decision\ntitle: Governed\nowner: "Composed Owner"\n---\n\n# Governed\n\n## Pick {#pick}\n\nsame everywhere\n`); + } + } finally { await fsp.rm(dir, { recursive: true, force: true }); } +}); + +test("stageFrontmatterTransaction/replaceFrontmatterValue round-trip an array value without downgrading its type", async () => { + const dir = await fsp.mkdtemp(path.join(os.tmpdir(), "cc-discrepancy-array-rt-")); + try { + const root = path.join(dir, "team"); + await fsp.mkdir(root); + await fsp.writeFile(path.join(root, "governed.md"), "---\ntype: decision\ntitle: Governed\ntags: [a, b]\n---\n\n# Governed\n\n## Pick {#pick}\n\nx\n"); + const roots = new Map([["team", { root, kind: "okf-local" }]]); + const staged = await stageFrontmatterTransaction(JSON.stringify({ + conceptId: "governed", key: "tags", layers: ["team"], value: ["x", "y", "z"], + expectedValues: { team: ["a", "b"] }, + }), roots, "tx-array-rt"); + await staged.commit(); + await staged.cleanup(); + const text = await fsp.readFile(path.join(root, "governed.md"), "utf8"); + assert.match(text, /^tags: \["x", "y", "z"\]$/m); + } finally { await fsp.rm(dir, { recursive: true, force: true }); } +}); + +test("applyDiscrepancyDecision refuses compose on an array-typed frontmatter field with a 400 (F11)", async () => { + const dir = await fsp.mkdtemp(path.join(os.tmpdir(), "cc-discrepancy-array-guard-")); + let svc = null; + let server = null; + try { + const teamDir = path.join(dir, "team"); + const companyDir = path.join(dir, "company"); + await fsp.mkdir(teamDir); + await fsp.mkdir(companyDir); + const doc = (tags) => `---\ntype: decision\ntitle: Governed\ntags: [${tags.join(", ")}]\n---\n\n# Governed\n\n## Pick {#pick}\n\nsame everywhere\n`; + await fsp.writeFile(path.join(teamDir, "governed.md"), doc(["a", "b"])); + await fsp.writeFile(path.join(companyDir, "governed.md"), doc(["c", "d"])); + const manifestPath = path.join(dir, "manifest.json"); + await fsp.writeFile(manifestPath, JSON.stringify({ + layers: [ + { name: "team", level: 2, path: teamDir }, + { name: "company", level: 0, path: companyDir }, + ], + })); + svc = createEngineService({ manifestPath, token: null }); + server = http.createServer(async (req, res) => { + if (await svc.handleRequest(req, res)) return; + res.writeHead(404); res.end(); + }); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + const base = `http://127.0.0.1:${server.address().port}`; + + const dset = await (await fetch(`${base}/api/discrepancies?wait=15000`)).json(); + const disc = dset.discrepancies.find((item) => item.originalKind === "frontmatter_value" && item.conceptId === "governed"); + assert.ok(disc, "expected a frontmatter_value discrepancy over the tags field"); + + const res = await fetch(`${base}/api/discrepancy-decisions`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ discrepancyId: disc.id, revision: disc.revision, action: "compose", content: "merged" }), + }); + assert.equal(res.status, 400); + const body = await res.json(); + assert.match(body.error, /list/i); + + // The refusal must be a pure guard: nothing written, files untouched. + const teamText = await fsp.readFile(path.join(teamDir, "governed.md"), "utf8"); + assert.match(teamText, /tags: \[a, b\]/); + } finally { + svc?.close(); + server?.close(); + await fsp.rm(dir, { recursive: true, force: true }); + } +}); + test("failed startup recovery records recovery_required and rejects", async () => { const { dir, roots } = await fixture(); try { From c9636bcbca254683cd006e355e1edbb06b97d624 Mon Sep 17 00:00:00 2001 From: John Siracusa Date: Sat, 8 Aug 2026 16:31:02 -0400 Subject: [PATCH 03/10] fix(console): name the real source and fit the whole cascade Provenance is the product's promise, and the cascade was rounding it off. - section provenance, "suppressed by", and dissent chips showed the lane name, so a value won by a second personal-level source read as "Personal" dissenting against "Personal". They name the source now; chip colour still comes from the lane. - lanes were a fixed level threshold: anything below 2 fell to Company, so a level-1 source sat in Company while the Team lane sat empty. Levels are ranked among those the manifest actually has, and lane headers list the sources behind them rather than implying one each. - Fit floored its scale at 0.2 while manual zoom floored at 0.4, so on a 3,000-concept vault Fit landed 100x too close and the first zoom-out jumped further in. Both clamp to the same epsilon now. - that canvas also rendered every concept as real DOM across a 726,134px row. Each lane caps at 250 nodes with a "Showing N of M" hand-off to Knowledge, which is where browsing everything belongs. Co-Authored-By: Claude Opus 5 Signed-off-by: John Siracusa --- apps/console/src/api.test.ts | 214 ++++++++++++++++-- apps/console/src/api.ts | 112 +++++++-- .../src/components/ConceptDetail.test.tsx | 91 ++++++++ apps/console/src/components/ConceptDetail.tsx | 44 +++- apps/console/src/data.ts | 14 ++ apps/console/src/types.ts | 17 +- apps/console/src/views/Canvas.test.tsx | 107 ++++++++- apps/console/src/views/Canvas.tsx | 140 ++++++++++-- apps/console/src/views/Overview.test.tsx | 31 +++ apps/console/src/views/Overview.tsx | 11 +- 10 files changed, 702 insertions(+), 79 deletions(-) create mode 100644 apps/console/src/components/ConceptDetail.test.tsx diff --git a/apps/console/src/api.test.ts b/apps/console/src/api.test.ts index bb24363..eb65d5d 100644 --- a/apps/console/src/api.test.ts +++ b/apps/console/src/api.test.ts @@ -1,10 +1,18 @@ // @vitest-environment jsdom import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { - adaptConcept, adaptConflicts, adaptSources, apiFetch, LiveDataError, mergeSourceStatus, selectMode, + adaptConcept, adaptConflicts, adaptDiscrepancies, adaptSources, apiFetch, computeLevelBuckets, LiveDataError, mergeSourceStatus, selectMode, trivialConflictReason, } from './api' -import type { GraphSummary, ResolvedConcept } from './types' +import type { DiscrepancyRecord, GraphSummary, ResolvedConcept } from './types' + +// The rank-based level→lane mapping (see computeLevelBuckets in api.ts) needs +// the full set of levels present across a resolve pass, not just the levels a +// single test concept happens to carry. Every fixture below uses the +// canonical trio of levels, so this single buckets value reproduces the old +// fixed-threshold mapping (0 → company, 2 → team, 3 → personal) exactly — +// tests that specifically exercise the rank behavior build their own. +const STANDARD_BUCKETS = computeLevelBuckets([0, 2, 3]) // ---- selectMode ------------------------------------------------------- @@ -106,6 +114,55 @@ describe('LiveSource error taxonomy', () => { }) }) +describe('LiveSource.search', () => { + beforeEach(() => { + vi.stubGlobal('fetch', vi.fn()) + }) + afterEach(() => { + vi.unstubAllGlobals() + }) + + it('returns hits from /api/search on the happy path', async () => { + const { createDataSource } = await import('./api') + const hits = [{ id: 'decisions/primary-db', title: 'Primary database', score: 4.2, layers: ['team'], snippet: '...SingleStore for HTAP...' }] + vi.mocked(fetch).mockResolvedValue(new Response(JSON.stringify({ hits }), { status: 200 })) + const source = createDataSource('live') + + await expect(source.search('singlestore')).resolves.toEqual(hits) + const [calledUrl] = vi.mocked(fetch).mock.calls[0] + expect(String(calledUrl)).toBe('/api/search?q=singlestore&limit=20') + }) + + it('encodes the query and honors a custom limit', async () => { + const { createDataSource } = await import('./api') + vi.mocked(fetch).mockResolvedValue(new Response(JSON.stringify({ hits: [] }), { status: 200 })) + const source = createDataSource('live') + + await source.search('primary db?', 5) + const [calledUrl] = vi.mocked(fetch).mock.calls[0] + expect(String(calledUrl)).toBe(`/api/search?q=${encodeURIComponent('primary db?')}&limit=5`) + }) + + // The existing older-engine-fallback idiom (see status() above): a 404 + // means this engine predates /api/search, and the signal is `null`, not a + // thrown error — the caller (store.search) decides what to do with that. + it('resolves to null on 404 — an engine older than this console', async () => { + const { createDataSource } = await import('./api') + vi.mocked(fetch).mockResolvedValue({ ok: false, status: 404, json: async () => ({}) } as Response) + const source = createDataSource('live') + + await expect(source.search('anything')).resolves.toBeNull() + }) + + it('rethrows a non-404 failure — only the 404 case is a silent fallback signal here', async () => { + const { createDataSource } = await import('./api') + vi.mocked(fetch).mockResolvedValue({ ok: false, status: 500, json: async () => ({}) } as Response) + const source = createDataSource('live') + + await expect(source.search('anything')).rejects.toMatchObject({ kind: 'bad-status', status: 500 }) + }) +}) + describe('desktop API credential transport', () => { afterEach(() => { delete window.__CC_DESKTOP @@ -196,6 +253,31 @@ describe('desktop API credential transport', () => { // ---- Adapters: raw engine types -> console view model ------------------- +describe('computeLevelBuckets', () => { + it('ranks the highest level present as personal, the next as team, the rest as company', () => { + const buckets = computeLevelBuckets([0, 1, 2, 3]) + expect(buckets.get(3)).toBe('personal') + expect(buckets.get(2)).toBe('team') + expect(buckets.get(1)).toBe('company') + expect(buckets.get(0)).toBe('company') + }) + + it('puts the sole level present in the top lane rather than folding it into company', () => { + expect(computeLevelBuckets([1]).get(1)).toBe('personal') + }) + + it('promotes a second-place level to team even when it is not 2', () => { + expect(computeLevelBuckets([3, 1]).get(1)).toBe('team') + expect(computeLevelBuckets([5, 4]).get(4)).toBe('team') + }) + + it('ignores duplicate levels when ranking', () => { + const buckets = computeLevelBuckets([2, 2, 0, 0]) + expect(buckets.get(2)).toBe('personal') + expect(buckets.get(0)).toBe('team') + }) +}) + describe('adaptConcept', () => { const sample: ResolvedConcept = { id: 'decisions/primary-db', @@ -219,30 +301,37 @@ describe('adaptConcept', () => { } it('maps id, title, and type from frontmatter', () => { - const c = adaptConcept(sample) + const c = adaptConcept(sample, STANDARD_BUCKETS) expect(c.id).toBe('decisions/primary-db') expect(c.title).toBe('Primary database') expect(c.type).toBe('decision') }) it('orders contributing layers by precedence (personal, team, company)', () => { - const c = adaptConcept(sample) + const c = adaptConcept(sample, STANDARD_BUCKETS) expect(c.layers).toEqual(['team', 'company']) }) + it('carries the real contributor source names, winner first — a zero-section concept has no section to read one from', () => { + const c = adaptConcept(sample, STANDARD_BUCKETS) + expect(c.contributorLayers).toEqual(['team', 'company']) + const empty: ResolvedConcept = { ...sample, sections: [] } + expect(adaptConcept(empty, STANDARD_BUCKETS).contributorLayers).toEqual(['team', 'company']) + }) + it('marks conflict true when any section has dissents', () => { - const c = adaptConcept(sample) + const c = adaptConcept(sample, STANDARD_BUCKETS) expect(c.conflict).toBe(true) }) it('marks draft only from OKF frontmatter (write.mjs stamps auto-captures)', () => { const stamped: ResolvedConcept = { ...sample, frontmatter: { ...sample.frontmatter, draft: true } } - expect(adaptConcept(stamped).draft).toBe(true) - expect(adaptConcept(sample).draft).toBe(false) + expect(adaptConcept(stamped, STANDARD_BUCKETS).draft).toBe(true) + expect(adaptConcept(sample, STANDARD_BUCKETS).draft).toBe(false) // A concept owned by a single layer is NOT draft — finished knowledge // commonly lives in exactly one layer. const solo: ResolvedConcept = { ...sample, contributors: [sample.contributors[0]], sections: [] } - expect(adaptConcept(solo).draft).toBe(false) + expect(adaptConcept(solo, STANDARD_BUCKETS).draft).toBe(false) }) it('maps non-canonical layer names via contributor levels, not the name', () => { @@ -266,16 +355,16 @@ describe('adaptConcept', () => { }, ], } - const c = adaptConcept(custom) + const c = adaptConcept(custom, STANDARD_BUCKETS) expect(c.sections[0].winner).toBe('team') expect(c.layers).toEqual(['team', 'company']) - const cards = adaptConflicts([custom]) + const cards = adaptConflicts([custom], [], STANDARD_BUCKETS) expect(cards[0].winner).toBe('team') expect(cards[0].contributions[0].layer).toBe('team') }) it('maps section winner, value, and provenance date', () => { - const c = adaptConcept(sample) + const c = adaptConcept(sample, STANDARD_BUCKETS) const s = c.sections[0] expect(s.name).toBe('Choice') expect(s.winner).toBe('team') @@ -284,7 +373,7 @@ describe('adaptConcept', () => { }) it('surfaces dissenting layers on the section, not hidden', () => { - const c = adaptConcept(sample) + const c = adaptConcept(sample, STANDARD_BUCKETS) const s = c.sections[0] expect(s.dissents).toHaveLength(1) expect(s.dissents?.[0]).toMatchObject({ layer: 'company', value: 'Postgres (org standard).', updated: '2025-06-01' }) @@ -295,7 +384,7 @@ describe('adaptConcept', () => { ...sample, sections: [{ ...sample.sections[0], suppressed: true, conflicts: undefined }], } - const c = adaptConcept(suppressed) + const c = adaptConcept(suppressed, STANDARD_BUCKETS) expect(c.sections[0].suppressed).toBe(true) expect(c.sections[0].dissents).toEqual([]) }) @@ -394,7 +483,7 @@ describe('adaptConcept with headingless documents', () => { } it('names a headingless section by its key instead of throwing', () => { - const concept = adaptConcept(headless) + const concept = adaptConcept(headless, STANDARD_BUCKETS) expect(concept.sections[0].name).toBe('body') }) @@ -404,7 +493,7 @@ describe('adaptConcept with headingless documents', () => { contributors: [...headless.contributors, { layer: 'team', level: 2, updated: '2026-02-10' }], sections: [{ ...headless.sections[0], conflicts: [{ layer: 'team', updated: '2026-02-10', content: 'Talked to Priya on Tuesday.' }] }], } - const [conflict] = adaptConflicts([contested]) + const [conflict] = adaptConflicts([contested], [], STANDARD_BUCKETS) expect(conflict.section).toBe('body') expect(conflict.title).toBe('body — 2026-02-11') }) @@ -539,6 +628,26 @@ describe('adaptSources', () => { expect(adaptSources(graph)[0].layer).toBe('personal') }) + // The fixed-threshold mapping this replaced sent any level < 2 straight to + // 'company' — so a level-1 source sat in Company next to level 0, and the + // Team lane, with nothing at level 2, sat empty. Ranked among the levels + // that actually exist (3 and 1 here), level 1 is the *second* highest and + // now lands in 'team'. + it('ranks a level-1 source into team, not company, when a higher level exists', () => { + const graph: GraphSummary = { + totals: { sourceTokens: 10, resolvedTokens: 10, concepts: 2, sources: 2 }, + sources: [ + { name: 'personal', level: 3, kind: 'okf-local', conceptCount: 1, tokens: 10, latestUpdated: null, status: 'ok', error: null }, + { name: 'messy-vault', level: 1, kind: 'files', conceptCount: 1, tokens: 10, latestUpdated: null, status: 'ok', error: null }, + ], + concepts: [], + } + const [, vault] = adaptSources(graph) + expect(vault.name).toBe('messy-vault') + expect(vault.layer).toBe('team') + expect(vault.layer).not.toBe('company') + }) + it('never paints a zero-concept MCP source as serving (the false green)', () => { // A dead MCP child answers [] instead of throwing, so its row arrives // status 'ok' with nothing served — that must not read as healthy. @@ -622,7 +731,7 @@ describe('adaptConflicts', () => { ], }, ] - const out = adaptConflicts(concepts) + const out = adaptConflicts(concepts, [], STANDARD_BUCKETS) expect(out).toHaveLength(1) expect(out[0]).toMatchObject({ id: 'decisions/primary-db::choice', @@ -644,7 +753,7 @@ describe('adaptConflicts', () => { sections: [{ key: 'steps', heading: '## Steps {#steps}', content: 'Deploy.', sourceLayer: 'team', sourceUpdated: null }], }, ] - expect(adaptConflicts(concepts)).toEqual([]) + expect(adaptConflicts(concepts, [], STANDARD_BUCKETS)).toEqual([]) }) it('classifies formatting-only prose but never guesses when words or code change', () => { @@ -671,11 +780,66 @@ describe('adaptConflicts', () => { reason: 'You chose the acme-eng answer.', actor: 'local-user', decidedAt: '2026-08-05T00:00:00.000Z', - }]) + }], STANDARD_BUCKETS) expect(resolved.status).toBe('resolved') expect(resolved.winner).toBe('team') expect(resolved.contributions.map((item) => item.layer)).toEqual(['team', 'company']) + // F13 prerequisite: a resolved card carries the winning source directly, + // so the Conflicts source filter can match it even when the contributions + // snapshot it carries doesn't happen to include that source by name. + expect(resolved.effectiveSource).toBe('acme-eng') + }) +}) + +describe('adaptDiscrepancies', () => { + function frontmatterRecord(overrides: Partial = {}): DiscrepancyRecord { + return { + id: 'frontmatter_value::decisions/primary-db::tags', + kind: 'frontmatter_value', + originalKind: 'frontmatter_value', + conceptId: 'decisions/primary-db', + conceptTitle: 'Primary database', + conceptType: 'concept', + key: 'tags', + label: 'tags', + revision: 'rev-1', + status: 'needs_review', + contributions: [ + { source: 'team', level: 2, updated: '2026-01-01', value: 'oltp', fingerprint: 'fp1', effective: true }, + { source: 'company', level: 0, updated: '2025-01-01', value: 'oltp', fingerprint: 'fp2', effective: false }, + ], + effectiveSource: 'team', + effectiveValue: 'oltp', + winnerReason: 'team wins by configured layer precedence.', + owner: 'Unassigned', + priority: 'unassigned', + fresherDissent: false, + freshness: { effectiveUpdated: '2026-01-01', newestUpdated: '2026-01-01', hasNewerDissent: false }, + affectedLinks: [], + sourceHealth: [], + history: [], + matchingRules: [], + ...overrides, + } + } + + it('flags a discrepancy isList when any raw contribution value is an array — the engine 400s compose against it', () => { + const record = frontmatterRecord({ + contributions: [ + { source: 'team', level: 2, updated: '2026-01-01', value: ['postgres', 'oltp'], fingerprint: 'fp1', effective: true }, + { source: 'company', level: 0, updated: '2025-01-01', value: ['mysql'], fingerprint: 'fp2', effective: false }, + ], + }) + const [card] = adaptDiscrepancies([record], true, STANDARD_BUCKETS) + expect(card.isList).toBe(true) + // The display value is still the honest stringified form, never the raw array. + expect(card.contributions[0].value).toBe(JSON.stringify(['postgres', 'oltp'], null, 2)) + }) + + it('never flags isList for an ordinary string-valued frontmatter field', () => { + const [card] = adaptDiscrepancies([frontmatterRecord()], true, STANDARD_BUCKETS) + expect(card.isList).toBeUndefined() }) }) @@ -700,9 +864,9 @@ describe('fresherDissent (C-b)', () => { } it('carries the section flag through adaptConcept onto the view section', () => { - const c = adaptConcept(conflicted({ fresherDissent: true })) + const c = adaptConcept(conflicted({ fresherDissent: true }), STANDARD_BUCKETS) expect(c.sections[0].fresherDissent).toBe(true) - expect(adaptConcept(conflicted({})).sections[0].fresherDissent).toBeUndefined() + expect(adaptConcept(conflicted({}), STANDARD_BUCKETS).sections[0].fresherDissent).toBeUndefined() }) it('marks exactly the strictly-newer dissent contribution on the conflict card', () => { @@ -714,7 +878,7 @@ describe('fresherDissent (C-b)', () => { ], }) concept.contributors.push({ layer: 'company', level: 0, updated: '2025-01-01' }) - const [card] = adaptConflicts([concept]) + const [card] = adaptConflicts([concept], [], STANDARD_BUCKETS) expect(card.contributions[0].fresherDissent).toBeUndefined() // the winner is never its own dissent expect(card.contributions[1]).toMatchObject({ layer: 'team', fresherDissent: true }) expect(card.contributions[2].fresherDissent).toBeUndefined() // older dissent stays unmarked @@ -723,7 +887,7 @@ describe('fresherDissent (C-b)', () => { it('never marks a dissent when the engine did not flag the section', () => { // The engine owns the rule (it also knows about suppression and // formatting-equivalence); the console must not out-guess it. - const [card] = adaptConflicts([conflicted({})]) + const [card] = adaptConflicts([conflicted({})], [], STANDARD_BUCKETS) expect(card.contributions.every((k) => k.fresherDissent === undefined)).toBe(true) }) @@ -737,7 +901,7 @@ describe('fresherDissent (C-b)', () => { ], }) concept.contributors.push({ layer: 'company', level: 0, updated: '2026-06-01' }) - const [card] = adaptConflicts([concept]) + const [card] = adaptConflicts([concept], [], STANDARD_BUCKETS) expect(card.contributions[1].fresherDissent).toBeUndefined() expect(card.contributions[2].fresherDissent).toBe(true) }) @@ -748,13 +912,13 @@ describe('fresherDissent (C-b)', () => { sourceUpdated: null, conflicts: [{ layer: 'team', updated: '2026-06-01', content: 'Dated dissent.' }], }) - const [card] = adaptConflicts([concept]) + const [card] = adaptConflicts([concept], [], STANDARD_BUCKETS) expect(card.contributions[1].fresherDissent).toBeUndefined() const garbled = conflicted({ fresherDissent: true, conflicts: [{ layer: 'team', updated: 'not-a-date', content: 'Undated dissent.' }], }) - expect(adaptConflicts([garbled])[0].contributions[1].fresherDissent).toBeUndefined() + expect(adaptConflicts([garbled], [], STANDARD_BUCKETS)[0].contributions[1].fresherDissent).toBeUndefined() }) }) diff --git a/apps/console/src/api.ts b/apps/console/src/api.ts index 51bfc2c..c817eb0 100644 --- a/apps/console/src/api.ts +++ b/apps/console/src/api.ts @@ -15,7 +15,7 @@ import demoBundleRaw from './generated/demo-cascade.json' import type { ConflictResolutionRecord, DemoBundle, DiscrepanciesResponse, DiscrepancyDecisionRequest, DiscrepancyRecord, DiscrepancyRule, DiscrepancyRuleSuggestion, GraphSummary, GraphSource, ResolveConflictRequest, - ResolvedConcept, ResolvedSection, SourceStatus, StatusSummary, + ResolvedConcept, ResolvedSection, SearchHit, SourceStatus, StatusSummary, } from './types' import type { Concept, ConceptSection, Conflict, Dissent, Source } from './data' import type { LayerId } from './theme' @@ -59,6 +59,14 @@ export interface DataSource { * to reading progress off the graph. */ status(): Promise + /** + * Full-text search over section content (GET /api/search), for the + * Knowledge search box. `null` means the same thing it means for `status()` + * above: an engine too old to have the route. Demo mode never calls this — + * it has no engine behind it — so `DemoSource` answers `null` unconditionally + * rather than reading its own bundle. + */ + search(query: string, limit?: number): Promise conflictResolutions(): Promise resolveConflict(request: ResolveConflictRequest): Promise discrepancies(): Promise @@ -195,9 +203,12 @@ class DemoSource implements DataSource { })), } } + /** Demo mode is pure client-side substring filtering — no engine to search. */ + async search(): Promise { return null } async conflictResolutions(): Promise { return this.resolutions } async discrepancies(): Promise { - const conflicts = adaptConflicts(this.bundle.concepts, this.resolutions) + const buckets = computeLevelBuckets(this.bundle.graph.sources.map((s) => s.level)) + const conflicts = adaptConflicts(this.bundle.concepts, this.resolutions, buckets) return { discrepancies: conflicts.map((conflict) => legacyConflictRecord(conflict)), coverageComplete: true, indexing: false, indexingSources: [], errors: [], generation: 1, @@ -327,6 +338,19 @@ class LiveSource implements DataSource { throw error } } + async search(query: string, limit = 20): Promise { + try { + return (await this.get<{ hits: SearchHit[] }>(`/api/search?q=${encodeURIComponent(query)}&limit=${limit}`)).hits + } catch (error) { + // Same older-engine idiom as status() above: a 404 means this engine has + // no /api/search route, and the caller falls back to the substring + // filter. Any other failure (network, timeout, malformed body) is the + // caller's problem too — it wraps this call and treats every rejection + // the same way, so nothing here needs to distinguish them. + if (error instanceof LiveDataError && error.kind === 'bad-status' && error.status === 404) return null + throw error + } + } async conflictResolutions(): Promise { try { return (await this.get<{ resolutions: ConflictResolutionRecord[] }>('/api/conflict-resolutions')).resolutions @@ -420,12 +444,34 @@ export function createDataSource(mode: Mode = selectMode()): DataSource { const LAYER_IDS: LayerId[] = ['company', 'team', 'personal'] const isLayerId = (s: string): s is LayerId => (LAYER_IDS as string[]).includes(s) -/** Map a source/layer name (falling back to level) to a console LayerId. */ -function layerOf(name: string, level: number): LayerId { +/** + * Rank-based bucket assignment for one resolve pass. `LayerId` stays a + * closed, three-value union — styling in ~8 files depends on it — so an + * arbitrary manifest level still needs an honest lane without widening that + * type. The highest level actually present becomes 'personal', the next + * 'team', and everything else 'company'. That fixes the fixed-threshold bug + * where a lone level-1 source (nothing above it) read as 'company' with the + * Team lane sitting empty: ranked among the levels that actually exist, level + * 1 is the *second* highest and lands in 'team'. + * + * Must be computed once per resolve pass from every source in play (not per + * concept or per record) and threaded through every adapter below — computing + * it from a narrower slice would bucket the same source differently + * depending on what happened to touch it. + */ +export type LevelBuckets = ReadonlyMap + +export function computeLevelBuckets(levels: Iterable): LevelBuckets { + const distinct = [...new Set(levels)].sort((a, b) => b - a) + const buckets = new Map() + distinct.forEach((level, rank) => buckets.set(level, rank === 0 ? 'personal' : rank === 1 ? 'team' : 'company')) + return buckets +} + +/** Map a source/layer name (falling back to its rank bucket) to a console LayerId. */ +function layerOf(name: string, level: number, buckets: LevelBuckets): LayerId { if (isLayerId(name)) return name - if (level >= 3) return 'personal' - if (level === 2) return 'team' - return 'company' + return buckets.get(level) ?? 'company' } /** @@ -478,10 +524,10 @@ function newerAtDayGranularity(dissentUpdated: string | null, winnerUpdated: str } /** A resolved section → the console's ConceptSection (with provenance + dissent). */ -function adaptSection(s: ResolvedSection, levels: Map): ConceptSection { - const winner = layerOf(s.sourceLayer, levels.get(s.sourceLayer) ?? 0) +function adaptSection(s: ResolvedSection, levels: Map, buckets: LevelBuckets): ConceptSection { + const winner = layerOf(s.sourceLayer, levels.get(s.sourceLayer) ?? 0, buckets) const dissents: Dissent[] = (s.conflicts ?? []).map((c) => ({ - layer: layerOf(c.layer, levels.get(c.layer) ?? 0), + layer: layerOf(c.layer, levels.get(c.layer) ?? 0, buckets), sourceLayer: c.layer, value: c.content, updated: c.updated, @@ -499,11 +545,12 @@ function adaptSection(s: ResolvedSection, levels: Map): ConceptS } } -/** A resolved concept → the console's Concept. */ -export function adaptConcept(r: ResolvedConcept): Concept { +/** A resolved concept → the console's Concept. `buckets` is the rank-based + * level→lane assignment for this resolve pass (see `computeLevelBuckets`). */ +export function adaptConcept(r: ResolvedConcept, buckets: LevelBuckets): Concept { const levels = contributorLevels(r) - const layerIds = orderLayers(r.contributors.map((c) => layerOf(c.layer, c.level))) - const sections = r.sections.map((s) => adaptSection(s, levels)) + const layerIds = orderLayers(r.contributors.map((c) => layerOf(c.layer, c.level, buckets))) + const sections = r.sections.map((s) => adaptSection(s, levels, buckets)) return { id: r.id, title: (r.frontmatter?.title as string) ?? r.id, @@ -515,6 +562,7 @@ export function adaptConcept(r: ResolvedConcept): Concept { // draft signal. Owning a concept in a single layer does not make it draft. draft: r.frontmatter?.draft === true, sections, + contributorLayers: r.contributors.map((c) => c.layer), } } @@ -552,6 +600,7 @@ export function progressPercent(p: { loaded?: number; total?: number | null } | /** Graph sources → the console's Source[] (coverage/focus/status derived honestly). */ export function adaptSources(g: GraphSummary): Source[] { + const buckets = computeLevelBuckets(g.sources.map((s) => s.level)) return g.sources.map((s: GraphSource) => { const errored = s.status === 'error' // A remote source that can't reach its API doesn't throw — it answers with @@ -587,7 +636,7 @@ export function adaptSources(g: GraphSummary): Source[] { return { name: s.name, kind: s.kind === 'mcp' ? 'mcp' : 'okf-local', - layer: layerOf(s.name, s.level), + layer: layerOf(s.name, s.level, buckets), // A source contributing nothing shouldn't show a full bar, however it // got there — errored, degraded to empty, or genuinely empty. While it // indexes the bar tracks real progress instead of standing in for it. @@ -713,8 +762,10 @@ export function trivialConflictReason(values: string[]): string | null { : 'The answers use the same words in the same order; only formatting differs.' } -/** Derive open conflicts plus resolved decisions retained by the local log. */ -export function adaptConflicts(concepts: ResolvedConcept[], resolutions: ConflictResolutionRecord[] = []): Conflict[] { +/** Derive open conflicts plus resolved decisions retained by the local log. + * `buckets` is the rank-based level→lane assignment for this resolve pass + * (see `computeLevelBuckets`). */ +export function adaptConflicts(concepts: ResolvedConcept[], resolutions: ConflictResolutionRecord[] = [], buckets: LevelBuckets): Conflict[] { const out: Conflict[] = [] const historyByConflict = new Map() for (const resolution of resolutions) { @@ -727,13 +778,13 @@ export function adaptConflicts(concepts: ResolvedConcept[], resolutions: Conflic const levels = contributorLevels(c) for (const s of c.sections) { if (!s.conflicts?.length) continue - const winner = layerOf(s.sourceLayer, levels.get(s.sourceLayer) ?? 0) + const winner = layerOf(s.sourceLayer, levels.get(s.sourceLayer) ?? 0, buckets) const id = `${c.id}::${s.key}` const history = historyByConflict.get(id) ?? [] const contributions = [ { layer: winner, sourceLayer: s.sourceLayer, value: s.content, updated: s.sourceUpdated ?? '' }, ...s.conflicts.map((k) => ({ - layer: layerOf(k.layer, levels.get(k.layer) ?? 0), + layer: layerOf(k.layer, levels.get(k.layer) ?? 0, buckets), sourceLayer: k.layer, value: k.content, updated: k.updated ?? '', @@ -764,7 +815,7 @@ export function adaptConflicts(concepts: ResolvedConcept[], resolutions: Conflic if (out.some((item) => item.id === id)) continue const latest = history[history.length - 1] const contributions = latest.contributions.map((item) => ({ - layer: layerOf(item.layer, item.level ?? (item.layer === 'personal' ? 3 : item.layer === 'team' ? 2 : 0)), + layer: layerOf(item.layer, item.level ?? (item.layer === 'personal' ? 3 : item.layer === 'team' ? 2 : 0), buckets), sourceLayer: item.layer, value: item.content, updated: item.updated ?? '', @@ -776,10 +827,11 @@ export function adaptConflicts(concepts: ResolvedConcept[], resolutions: Conflic section: headingText(latest.sectionHeading), title: `${headingText(latest.sectionHeading)} — ${latest.title}`, status: 'resolved', - winner: layerOf(latest.chosen?.layer ?? latest.contributions[0]?.layer ?? '', latest.chosen?.level ?? latest.contributions[0]?.level ?? 0), + winner: layerOf(latest.chosen?.layer ?? latest.contributions[0]?.layer ?? '', latest.chosen?.level ?? latest.contributions[0]?.level ?? 0, buckets), contributions, safe: false, history, + effectiveSource: latest.chosen?.layer ?? latest.contributions[0]?.layer ?? null, }) } return out @@ -809,11 +861,20 @@ function legacyConflictRecord(conflict: Conflict): DiscrepancyRecord { } } -/** Raw professional discrepancy records → the existing navigator view model. */ -export function adaptDiscrepancies(records: DiscrepancyRecord[], coverageComplete = true): Conflict[] { +/** Raw professional discrepancy records → the existing navigator view model. + * `buckets` is the rank-based level→lane assignment for this resolve pass + * (see `computeLevelBuckets`). */ +export function adaptDiscrepancies(records: DiscrepancyRecord[], coverageComplete = true, buckets: LevelBuckets): Conflict[] { return records.map((record) => { + // The raw contribution value carries its real type (the engine never + // stringifies a list-typed frontmatter field before serving it) — check + // it BEFORE the display value below coerces every non-string into JSON + // text. `isList` rides with the discrepancy, not a contribution, because + // the engine's own compose guard (service.mjs) rejects the action for the + // whole field, not per-contributor. + const isList = record.contributions.some((item) => Array.isArray(item.value)) const contributions = record.contributions.map((item) => ({ - layer: layerOf(item.source, item.level), sourceLayer: item.source, + layer: layerOf(item.source, item.level, buckets), sourceLayer: item.source, value: typeof item.value === 'string' ? item.value : JSON.stringify(item.value, null, 2), updated: item.updated ?? '', ...(record.fresherDissent && !item.effective ? { fresherDissent: true } : {}), @@ -823,13 +884,14 @@ export function adaptDiscrepancies(records: DiscrepancyRecord[], coverageComplet id: record.id, concept: record.conceptId, sectionKey: record.key, section: record.label, title: `${record.label} — ${record.conceptTitle}`, status: record.status === 'resolved' ? 'resolved' : 'open', - winner: layerOf(effective?.source ?? '', effective?.level ?? 0), + winner: layerOf(effective?.source ?? '', effective?.level ?? 0, buckets), contributions, safe: false, history: record.history, kind: record.kind, discrepancyStatus: record.status, revision: record.revision, owner: record.owner, priority: record.priority, winnerReason: record.winnerReason, effectiveSource: record.effectiveSource, coverageComplete, sourceHealth: record.sourceHealth, matchingRules: record.matchingRules, ruleConflict: record.ruleConflict, target: record.target, affectedLinks: record.affectedLinks, + ...(isList ? { isList: true } : {}), } }) } diff --git a/apps/console/src/components/ConceptDetail.test.tsx b/apps/console/src/components/ConceptDetail.test.tsx new file mode 100644 index 0000000..711605e --- /dev/null +++ b/apps/console/src/components/ConceptDetail.test.tsx @@ -0,0 +1,91 @@ +// @vitest-environment jsdom +// ConceptDetail is shared by the Canvas slide-over and the Knowledge page. A +// section's provenance line, its "suppressed by" note, and a dissent chip all +// used to name the three-lane bucket (layerName(winner)/layerName(layer)) +// instead of the real source that produced the value — so two sources sharing +// a lane (e.g. two personal-level MCP servers) were indistinguishable in the +// inspector. Every place that used to print a lane name now prints +// `sourceLayer`, the manifest's own name for the contributor. +import { act } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { ConceptDetail } from './ConceptDetail' +import type { Concept } from '../data' + +vi.mock('../layer-files', () => ({ + filesRevalidation: () => 'rev', + useLayerFiles: () => ({ layers: [] }), +})) + +const mocks = vi.hoisted(() => ({ store: null as unknown as Record })) +vi.mock('../store', () => { + const store = () => mocks.store + return { useStore: store, useStoreData: store, useStoreNav: store, useStoreInput: store } +}) + +let container: HTMLDivElement +let root: Root + +beforeEach(() => { + ;(globalThis as typeof globalThis & { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true + mocks.store = { mode: 'demo', sources: [], reloadKey: 0, openFilesScope: vi.fn() } + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) +}) + +afterEach(async () => { + await act(async () => root.unmount()) + container.remove() +}) + +function concept(): Concept { + return { + id: 'decisions/primary-db', + title: 'Primary database', + type: 'decision', + layers: ['personal', 'team'], + sections: [ + { + name: 'Choice', + winner: 'personal', + sourceLayer: 'maya-notes', + value: 'SingleStore for HTAP workloads.', + updated: '2026-08-01', + dissents: [ + { layer: 'team', sourceLayer: 'acme-eng', value: 'Postgres (org standard).', updated: '2026-06-01' }, + ], + }, + { + name: 'Rollback plan', + winner: 'personal', + sourceLayer: 'maya-notes', + value: '', + suppressed: true, + }, + ], + } +} + +describe('ConceptDetail provenance', () => { + it('names the real contributing source, not the lane it renders in', async () => { + await act(async () => root.render()) + expect(container.textContent).toContain('maya-notes · 2026-08-01') + // The lane bucket name never appears as the section's provenance text — + // it stays a color cue (the dot) plus the top-of-panel layer chips. + expect(container.querySelector('code')).toBeTruthy() + }) + + it('names the real source in the suppressed-by note, not "personal"', async () => { + await act(async () => root.render()) + expect(container.textContent).toContain('suppressed by maya-notes') + expect(container.textContent).not.toContain('suppressed by personal') + }) + + it('names the real dissenting source on the dissent chip, keeping the lane color', async () => { + await act(async () => root.render()) + const chip = Array.from(container.querySelectorAll('span')).find((el) => el.textContent === 'acme-eng') + expect(chip, 'dissent chip should read the source name, not the lane').toBeTruthy() + expect(container.textContent).not.toContain('Team says') + }) +}) diff --git a/apps/console/src/components/ConceptDetail.tsx b/apps/console/src/components/ConceptDetail.tsx index 6682187..6299ac5 100644 --- a/apps/console/src/components/ConceptDetail.tsx +++ b/apps/console/src/components/ConceptDetail.tsx @@ -1,6 +1,5 @@ import { useMemo } from 'react' import { C, css, lc, MONO, conceptTypeStyle } from '../theme' -import { layerName } from '../data' import type { Concept } from '../data' import { filesRevalidation, useLayerFiles } from '../layer-files' import { useStoreData } from '../store' @@ -56,6 +55,38 @@ function OpenFile({ layer, path, conceptId }: { layer: string; path: string | un ) } +/** + * A concept with no sections is a dead end — the resolver produced an id and + * some frontmatter, but nothing to read. Rather than rendering an empty + * `
` with no explanation, name the situation and, where a source file is + * identifiable, offer a way to it: the winning contributor's file, or — + * absent a listing for it (an MCP or REST-read contributor keeps no file + * here) — a plain way into the Files tab, scoped to that source, so browsing + * is still one click away. + */ +function EmptyConcept({ concept, fileFor }: { concept: Concept; fileFor: (sourceLayer: string) => string | undefined }) { + const { openFilesScope } = useStoreData() + const winner = concept.contributorLayers?.[0] + const path = winner ? fileFor(winner) : undefined + return ( +
+

This concept has no sections — the file may be empty.

+ {winner && ( + path + ? + : ( + + ) + )} +
+ ) +} + /** The resolved read of a concept — provenance chips per section + inline dissent. * Shared by the Concepts view and the Canvas node slide-over. */ export function ConceptDetail({ concept }: { concept: Concept }) { @@ -77,10 +108,15 @@ export function ConceptDetail({ concept }: { concept: Concept }) {
+ {concept.sections.length === 0 && } {concept.sections.map((s) => { const col = lc(s.winner) const dissents = s.dissents ?? [] - const provenance = `${layerName(s.winner)}${s.updated ? ' · ' + s.updated : ''}` + // The real source that won this section, not the three-lane bucket it + // renders in — two sources can share a lane, and only the source name + // says which one is behind the value. The colored dot beside the + // heading already carries the lane; this text carries provenance. + const provenance = `${s.sourceLayer}${s.updated ? ' · ' + s.updated : ''}` return (
@@ -93,7 +129,7 @@ export function ConceptDetail({ concept }: { concept: Concept }) { {s.suppressed ? (
- suppressed by {layerName(s.winner)} + suppressed by {s.sourceLayer}
) : (
{s.value}
@@ -107,7 +143,7 @@ export function ConceptDetail({ concept }: { concept: Concept }) {
- {layerName(d.layer)} says "{d.value}" — overridden here. + {d.sourceLayer} says "{d.value}" — overridden here.
{d.updated && {d.updated}} diff --git a/apps/console/src/data.ts b/apps/console/src/data.ts index dcac85b..c27737b 100644 --- a/apps/console/src/data.ts +++ b/apps/console/src/data.ts @@ -79,6 +79,13 @@ export interface Conflict { ruleConflict?: boolean target?: string affectedLinks?: string[] + /** + * True when any raw contribution behind this discrepancy is an array-typed + * frontmatter value (a list field). The engine 400s a compose against such + * a field (service.mjs), so the UI disables the compose disposition rather + * than letting the request round-trip into an error. + */ + isList?: boolean } /** `sourceLayer` is the source's real name; `layer` is the lane it renders in. */ @@ -100,6 +107,13 @@ export interface ConceptSection { export interface Concept { id: string; title: string; type: string; layers: LayerId[] conflict?: boolean; draft?: boolean; sections: ConceptSection[] + /** + * The real source names behind this concept, winner first — kept + * separately from `layers` (the three-lane buckets) because a concept with + * zero sections has no `ConceptSection.sourceLayer` to read a contributor's + * real name from, and the "Open file" affordance needs one anyway. + */ + contributorLayers?: string[] } export interface Activity { diff --git a/apps/console/src/types.ts b/apps/console/src/types.ts index 5739935..2a59856 100644 --- a/apps/console/src/types.ts +++ b/apps/console/src/types.ts @@ -95,6 +95,21 @@ export interface StatusSummary { sources: SourceStatus[] } +/** + * One hit from GET /api/search — BM25F over stemmed section content + * (`searchConcepts` in packages/core/src/search.mjs). `layers` names every + * contributing layer, best-scoring first; `snippet` is pre-extracted around + * the matched terms, so the console never has to re-tokenize the body. + * Hits arrive pre-sorted by score, highest first. + */ +export interface SearchHit { + id: string + title: string | null + score: number + layers: string[] + snippet: string +} + /** A source (layer) row in the graph summary. */ export interface GraphSource { name: string @@ -257,7 +272,7 @@ export interface ConflictResolutionRecord { export type DiscrepancyKind = 'section_content' | 'frontmatter_value' | 'broken_link' | 'changed_after_decision' export type DiscrepancyStatus = 'needs_review' | 'recommended' | 'auto_ready' | 'acknowledged' | 'resolved' | 'reopened' | 'blocked' export type DiscrepancyAction = 'choose_contribution' | 'compose' | 'acknowledge' -export type AcknowledgementReason = 'different_scopes' | 'temporary_migration' | 'source_specific_authority' | 'other' +export type AcknowledgementReason = 'different_scopes' | 'temporary_migration' | 'source_specific_authority' | 'target_missing' | 'other' export interface DiscrepancyContribution { source: string diff --git a/apps/console/src/views/Canvas.test.tsx b/apps/console/src/views/Canvas.test.tsx index 995548d..281df0b 100644 --- a/apps/console/src/views/Canvas.test.tsx +++ b/apps/console/src/views/Canvas.test.tsx @@ -1,7 +1,7 @@ // @vitest-environment jsdom -import { describe, expect, it } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' import type { Concept } from '../data' -import { computeLayout } from './Canvas' +import { capConceptsPerLane, clampZoom, computeFitScale, computeLayout } from './Canvas' function concept(id: string, layer: Concept['layers'][number], dissent?: Concept['layers'][number]): Concept { return { @@ -36,6 +36,109 @@ describe('computeLayout', () => { }) }) +describe('computeFitScale', () => { + // The old floor (Math.max(0.2, ...)) overrode content-driven fit: a huge + // cascade needing a scale well under 0.2 to actually fit got clamped up to + // 0.2 anyway, so "Fit" stopped fitting. The floor is now a near-zero + // epsilon that only keeps the transform off exactly zero. + it('reaches a scale well below the old 0.2 floor for a huge world', () => { + const result = computeFitScale(2000, 1200, 200_000, 100_000) + expect(result).not.toBeNull() + expect(result!.scale).toBeLessThan(0.2) + expect(result!.scale).toBeGreaterThan(0) + expect(result!.scale).toBeCloseTo((2000 - 48) / 200_000, 5) + }) + + it('still guards a not-yet-laid-out element', () => { + expect(computeFitScale(0, 0, 1000, 1000)).toBeNull() + }) +}) + +describe('clampZoom', () => { + // The manual zoom/wheel clamp used to floor at 0.4, so zooming out after a + // Fit that landed below 0.4 snapped the view back up — the opposite of + // "zoom out". It now shares computeFitScale's epsilon floor. + it('allows a scale well below the old 0.4 floor', () => { + expect(clampZoom(0.05)).toBeCloseTo(0.05, 5) + }) + + it('still clamps at the top end', () => { + expect(clampZoom(50)).toBe(2) + }) +}) + +describe('capConceptsPerLane', () => { + function many(layer: Concept['layers'][number], count: number): Concept[] { + return Array.from({ length: count }, (_, i) => concept(`${layer}-${i}`, layer)) + } + + it('passes a small cascade through unchanged', () => { + const input = [...many('personal', 3), ...many('team', 2)] + const result = capConceptsPerLane(input, 250) + expect(result.shown).toBe(5) + expect(result.total).toBe(5) + expect(result.concepts).toHaveLength(5) + }) + + it('slices each lane independently and reports per-lane + total counts', () => { + const input = [...many('personal', 400), ...many('team', 10), ...many('company', 5)] + const result = capConceptsPerLane(input, 250) + expect(result.laneCounts.personal).toEqual({ shown: 250, total: 400 }) + expect(result.laneCounts.team).toEqual({ shown: 10, total: 10 }) + expect(result.laneCounts.company).toEqual({ shown: 5, total: 5 }) + expect(result.shown).toBe(250 + 10 + 5) + expect(result.total).toBe(415) + expect(result.concepts).toHaveLength(result.shown) + }) + + it('keeps the first N in incoming order (no cheap per-concept date to sort by)', () => { + const input = many('personal', 5) + const result = capConceptsPerLane(input, 3) + expect(result.concepts.map((c) => c.id)).toEqual(['personal-0', 'personal-1', 'personal-2']) + }) +}) + +describe('lane header honesty (F3)', () => { + // vi.doMock (not vi.mock) so this stays scoped to a resetModules() import — + // the legend test below needs the real StoreProvider, and a file-level mock + // of '../store' would break it. + afterEach(() => { + vi.doUnmock('../store') + vi.resetModules() + }) + + it('names the real source and level behind a lane instead of the static trio', async () => { + ;(globalThis as typeof globalThis & { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true + vi.resetModules() + vi.doMock('../store', () => { + const noop = () => {} + const state = { + mode: 'live', concepts: [], conflicts: [], sources: [{ name: 'messy-vault', layer: 'team', level: 1 }], + setSelConcept: noop, setSelConflict: noop, setView: noop, + } + const useState = () => state + return { useStore: useState, useStoreData: useState, useStoreNav: useState, useStoreInput: useState } + }) + const { act } = await import('react') + const { createRoot } = await import('react-dom/client') + const { Canvas } = await import('./Canvas') + + const container = document.createElement('div') + document.body.append(container) + const root = createRoot(container) + await act(async () => root.render()) + + expect(container.textContent).toContain('messy-vault') + // The team lane's round badge shows the real level (1), not the static 2 — + // and the lane's static "runbooks, decisions, system docs" blurb is gone, + // replaced by the source name. + expect(container.textContent).not.toContain('runbooks, decisions, system docs') + + await act(async () => root.unmount()) + container.remove() + }) +}) + describe('the canvas legend', () => { it('stays translucent, because the graph moves underneath it', async () => { // Not a style preference: the legend is absolutely positioned over the diff --git a/apps/console/src/views/Canvas.tsx b/apps/console/src/views/Canvas.tsx index df77730..7efe489 100644 --- a/apps/console/src/views/Canvas.tsx +++ b/apps/console/src/views/Canvas.tsx @@ -1,6 +1,6 @@ import { memo, useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react' import { C, css, lc, MONO, type LayerId } from '../theme' -import { layerLevel, layers, type Concept } from '../data' +import { layerLevel, layerName, layers, type Concept } from '../data' import { LayerChip } from '../components/LayerChip' import { ConceptDetail } from '../components/ConceptDetail' import { useStoreData } from '../store' @@ -13,6 +13,20 @@ const LANE_TOP = 60, LANE_H = 196, LANE_GAP = 16 const LANE_INNER = LANE_H - LANE_GAP const NODE_DY = 46, GHOST_DY = 62 +// A real-DOM canvas with no virtualization: every node is a live element in +// the pan/zoom transform, so a vault with thousands of concepts in one lane +// stops being interactive well before it stops being legible. Cap what's +// rendered rather than let the browser choke on it — Knowledge (unpaginated, +// list-based) is where the rest is still reachable. +const MAX_NODES_PER_LANE = 250 +// Not a floor: Fit must be able to reach whatever scale the content actually +// needs. 0.2 as a floor meant a large cascade's "Fit" button silently stopped +// fitting — this is only here to keep the transform away from exactly zero. +const MIN_SCALE = 0.001 +const MAX_SCALE = 2 + +const NUM = new Intl.NumberFormat() + // lanes top→bottom: highest precedence (Personal) on top so "up = wins" const LANE_ORDER: LayerId[] = ['personal', 'team', 'company'] const laneIndex = (id: LayerId) => LANE_ORDER.indexOf(id) @@ -20,6 +34,47 @@ const laneY = (i: number) => LANE_TOP + i * LANE_H const primaryLayer = (c: Concept): LayerId => c.layers.slice().sort((a, b) => layerLevel(b) - layerLevel(a))[0] +/** Fit scale/pan for a `cw`×`ch` viewport around `worldW`×`worldH` content, or + * `null` while the element is not yet laid out (see the caller's guard). */ +export function computeFitScale(cw: number, ch: number, worldW: number, worldH: number) { + if (cw < 40 || ch < 40) return null + const scale = Math.max(MIN_SCALE, Math.min(1, (cw - 48) / worldW, (ch - 48) / worldH)) + return { scale, tx: (cw - worldW * scale) / 2, ty: Math.max(24, (ch - worldH * scale) / 2) } +} + +/** Clamp a manual zoom (wheel or +/− button) to the app's zoom range. */ +export function clampZoom(scale: number): number { + return Math.min(MAX_SCALE, Math.max(MIN_SCALE, scale)) +} + +export interface LaneCapResult { + concepts: Concept[] + shown: number + total: number + laneCounts: Record +} + +/** + * Cap how many concepts land on the canvas per lane. Selection keeps the + * first N in resolve-all order: `Concept` carries no single "last updated" + * timestamp of its own (only per-section dates), and scanning every section + * of every concept just to sort would undercut the point of a cheap cap at + * the scale this exists for. + */ +export function capConceptsPerLane(concepts: Concept[], max = MAX_NODES_PER_LANE): LaneCapResult { + const byLane: Record = { company: [], team: [], personal: [] } + for (const c of concepts) byLane[primaryLayer(c)].push(c) + const laneCounts = {} as Record + const out: Concept[] = [] + for (const id of LANE_ORDER) { + const all = byLane[id] + const shown = all.slice(0, max) + laneCounts[id] = { shown: shown.length, total: all.length } + out.push(...shown) + } + return { concepts: out, shown: out.length, total: concepts.length, laneCounts } +} + interface NodePos { c: Concept; x: number; y: number; conflict: boolean } interface GhostPos { key: string; parent: NodePos; layer: LayerId; value: string; x: number; y: number } @@ -72,14 +127,31 @@ function edgePath(x1: number, y1: number, x2: number, y2: number) { } function CanvasInner({ keyboardSuspended = false }: { keyboardSuspended?: boolean }) { - const { setSelConcept, setSelConflict, setView, conflicts, concepts } = useStoreData() + const { setSelConcept, setSelConflict, setView, conflicts, concepts, sources, mode } = useStoreData() + // Capped before layout: a real-DOM canvas with no virtualization stops + // being usable well before thousands of nodes finish laying out. Ghost + // (dissent) cards derive from `nodes` below, so they respect the cap too — + // there is no separate ghost list to cap. + const capped = useMemo(() => capConceptsPerLane(concepts), [concepts]) // Memoized: pan/zoom re-renders every pointermove — don't re-lay-out for those. - const { nodes, ghosts, worldW, worldH } = useMemo(() => computeLayout(concepts), [concepts]) + const { nodes, ghosts, worldW, worldH } = useMemo(() => computeLayout(capped.concepts), [capped.concepts]) + // Full counts, not the capped subset — the lane header's "N concepts" stays + // an honest total even while the canvas itself only renders some of them. const laneCounts = useMemo(() => { const counts: Record = { company: 0, team: 0, personal: 0 } for (const c of concepts) counts[primaryLayer(c)] += 1 return counts }, [concepts]) + // Real (source name, level) pairs behind each lane, for honest lane headers + // (Fix F3): demo mode's sources are already the canonical company/team/ + // personal trio, so this reduces to the static labels there — the fallback + // below only changes what a live, non-canonical cascade renders. + const laneSourceRows = useMemo(() => { + const rows: Record = { company: [], team: [], personal: [] } + if (mode === 'demo') return rows + for (const s of sources) rows[s.layer].push({ name: s.name, level: s.level }) + return rows + }, [sources, mode]) const wrapRef = useRef(null) const [view, setViewT] = useState({ tx: 40, ty: 20, scale: 1 }) @@ -94,13 +166,12 @@ function CanvasInner({ keyboardSuspended = false }: { keyboardSuspended?: boolea const fit = useCallback(() => { const el = wrapRef.current if (!el) return - const cw = el.clientWidth, ch = el.clientHeight - // Guard against a not-yet-laid-out element (async data can populate before - // layout settles): a zero width would yield a negative scale that never - // self-corrects, collapsing the whole canvas to a speck. - if (cw < 40 || ch < 40) return - const scale = Math.max(0.2, Math.min(1, (cw - 48) / worldW, (ch - 48) / worldH)) - setViewT({ scale, tx: (cw - worldW * scale) / 2, ty: Math.max(24, (ch - worldH * scale) / 2) }) + // computeFitScale's own guard covers a not-yet-laid-out element (async + // data can populate before layout settles): a zero width would otherwise + // yield a negative scale that never self-corrects, collapsing the canvas + // to a speck. + const next = computeFitScale(el.clientWidth, el.clientHeight, worldW, worldH) + if (next) setViewT(next) }, [worldW, worldH]) useLayoutEffect(() => { fit() }, [fit]) @@ -125,7 +196,7 @@ function CanvasInner({ keyboardSuspended = false }: { keyboardSuspended?: boolea const rect = el.getBoundingClientRect() const px = e.clientX - rect.left, py = e.clientY - rect.top setViewT((v) => { - const next = Math.min(2, Math.max(0.4, v.scale * Math.exp(-e.deltaY * 0.0015))) + const next = clampZoom(v.scale * Math.exp(-e.deltaY * 0.0015)) const wx = (px - v.tx) / v.scale, wy = (py - v.ty) / v.scale return { scale: next, tx: px - wx * next, ty: py - wy * next } }) @@ -185,7 +256,7 @@ function CanvasInner({ keyboardSuspended = false }: { keyboardSuspended?: boolea }, [keyboardSuspended, openId, wideInspector]) const zoom = (dir: number) => setViewT((v) => { const el = wrapRef.current!, px = el.clientWidth / 2, py = el.clientHeight / 2 - const next = Math.min(2, Math.max(0.4, v.scale * (dir > 0 ? 1.2 : 1 / 1.2))) + const next = clampZoom(v.scale * (dir > 0 ? 1.2 : 1 / 1.2)) const wx = (px - v.tx) / v.scale, wy = (py - v.ty) / v.scale return { scale: next, tx: px - wx * next, ty: py - wy * next } }) @@ -205,18 +276,27 @@ function CanvasInner({ keyboardSuspended = false }: { keyboardSuspended?: boolea style={{ position: 'absolute', top: 0, left: 0, bottom: 0, right: wideInspector && openConceptObj ? 360 : 0, cursor: dragging ? 'grabbing' : 'grab', touchAction: 'none' }} >
- {/* lane backgrounds + labels */} + {/* lane backgrounds + labels — real levels and source names behind + each lane, not the static trio (F3): a level-1 source that ranks + into 'team' should say so, and two sources sharing a lane should + both be named rather than only the lane's generic blurb. */} {LANE_ORDER.map((id, i) => { const L = layers.find((l) => l.id === id)! const col = lc(id) + const rows = laneSourceRows[id] + const levels = [...new Set(rows.map((r) => r.level))].sort((a, b) => a - b) + const conventional = rows.some((r) => r.name === id) + const badgeText = levels.length ? levels.join('/') : String(L.level) + const primary = conventional || rows.length === 0 ? L.name : `L${levels.join('/')}` + const detail = rows.length ? rows.map((r) => r.name).join(', ') : L.members return (
- {L.level} + {badgeText}
-
{L.name}
-
{L.members} · {laneCounts[id]} concept{laneCounts[id] === 1 ? '' : 's'}
+
{primary}
+
{detail} · {laneCounts[id]} concept{laneCounts[id] === 1 ? '' : 's'}
@@ -252,6 +332,7 @@ function CanvasInner({ keyboardSuspended = false }: { keyboardSuspended?: boolea onMouseEnter={() => setHoverId(g.parent.c.id)} onMouseLeave={() => setHoverId(null)} title="Layers disagree — open the conflict" + aria-label={`${g.parent.c.title} — ${layerName(g.layer)} dissents, has conflict`} style={{ position: 'absolute', left: g.x, top: g.y, width: GHOST_W, height: GHOST_H, ...css(`display:flex; flex-direction:column; justify-content:center; gap:4px; text-align:left; padding:10px 12px; background:${C.surface}; border:1px dashed var(--cc-edge-conflict); border-radius:11px; cursor:pointer; font:inherit;`) }} >
@@ -275,6 +356,7 @@ function CanvasInner({ keyboardSuspended = false }: { keyboardSuspended?: boolea onClick={(event) => openConcept(n.c, event.currentTarget)} onMouseEnter={() => setHoverId(n.c.id)} onMouseLeave={() => setHoverId(null)} + aria-label={`${n.c.title} — ${layerName(primaryLayer(n.c))}${n.conflict ? ', has conflict' : n.c.draft ? ', draft' : ''}`} style={{ position: 'absolute', left: n.x, top: n.y, width: NODE_W, height: NODE_H, boxShadow: glow, ...css(`display:flex; flex-direction:column; gap:0; text-align:left; padding:12px 14px; background:${C.raised}; border:1px solid ${selected ? col.strokeE : C.line}; border-left:3px solid ${col.strokeE}; border-radius:12px; cursor:pointer; font:inherit;`) }} >
@@ -314,17 +396,33 @@ function CanvasInner({ keyboardSuspended = false }: { keyboardSuspended?: boolea {/* zoom controls */}
- {[['+', () => zoom(1)], ['−', () => zoom(-1)], ['⤢', fit]].map(([label, fn]) => ( + {([['+', 'Zoom in', () => zoom(1)], ['−', 'Zoom out', () => zoom(-1)], ['⤢', 'Fit to view', fit]] as const).map(([label, name, fn]) => ( + >{label} ))}
+ {/* cap banner — a real-DOM canvas with no virtualization stops being + usable well before a large cascade finishes laying out (F7); this + says what's hidden and where the rest still is. */} + {capped.shown < capped.total && ( +
+ Showing {NUM.format(capped.shown)} of {NUM.format(capped.total)} + +
+ )} + {/* node detail slide-over */} {openConceptObj && (
diff --git a/apps/console/src/views/Overview.test.tsx b/apps/console/src/views/Overview.test.tsx index 7b9e063..e25d6ae 100644 --- a/apps/console/src/views/Overview.test.tsx +++ b/apps/console/src/views/Overview.test.tsx @@ -42,3 +42,34 @@ it('shows a calm resolved state when nothing needs review', async () => { await act(async () => root.render()) expect(container.textContent).toContain('Nothing needs review') }) + +// F3: the Cascade summary used to render the static company/team/personal +// blurb and level no matter what actually fed a lane. A level-1 source with +// no name matching its lane should read as itself — its real name and level — +// not the generic "runbooks, decisions, system docs" / "2" it happened to +// inherit from the lane it ranked into. +it('names the real source and level behind a lane instead of the static blurb', async () => { + mocks.useStore.mockReturnValue({ + mode: 'live', setView: mocks.setView, + signals: [], conflicts: [], + sources: [{ name: 'messy-vault', status: 'synced', layer: 'team', level: 1, conceptCount: 4 }], + concepts: [], activity: [], loadErrors: [], + }) + await act(async () => root.render()) + expect(container.textContent).toContain('messy-vault') + expect(container.textContent).toContain('1') + expect(container.textContent).not.toContain('runbooks, decisions, system docs') +}) + +// Demo mode's sources are already the canonical trio; the honest-labeling +// pass must not disturb its existing static blurb. +it('keeps the static cascade blurb in demo mode', async () => { + mocks.useStore.mockReturnValue({ + mode: 'demo', setView: mocks.setView, + signals: [], conflicts: [], + sources: [{ name: 'team', status: 'synced', layer: 'team', level: 2, conceptCount: 4 }], + concepts: [], activity: [], loadErrors: [], + }) + await act(async () => root.render()) + expect(container.textContent).toContain('runbooks, decisions, system docs') +}) diff --git a/apps/console/src/views/Overview.tsx b/apps/console/src/views/Overview.tsx index 7b209e4..2fab787 100644 --- a/apps/console/src/views/Overview.tsx +++ b/apps/console/src/views/Overview.tsx @@ -40,7 +40,16 @@ function OverviewInner() {

Cascade summary

Higher layers win only for the sections they define; everything else is inherited.

{[...layers].sort((a, b) => b.level - a.level).map((layer) => { const count = concepts.filter((concept) => concept.layers.includes(layer.id)).length - return
{layer.level}{layer.sub}{count} concept{count === 1 ? '' : 's'}
+ // Real levels and source names behind this lane (F3), not the static + // trio: a level-1 source that ranks into 'team' should say so here + // too, not just on the Canvas. Demo mode's sources are already the + // canonical company/team/personal trio, so this falls back to the + // static blurb there unchanged. + const rows = mode === 'demo' ? [] : sources.filter((source) => source.layer === layer.id) + const levels = [...new Set(rows.map((row) => row.level).filter((level): level is number => typeof level === 'number'))].sort((a, b) => a - b) + const levelLabel = levels.length ? levels.join('/') : String(layer.level) + const sourceLabel = rows.length ? rows.map((row) => row.name).join(', ') : layer.sub + return
{levelLabel}{sourceLabel}{count} concept{count === 1 ? '' : 's'}
})}
From d689276d62b47b2cab6c067f197a40c8d20772af Mon Sep 17 00:00:00 2001 From: John Siracusa Date: Sat, 8 Aug 2026 16:31:15 -0400 Subject: [PATCH 04/10] fix(console): stop the reconciled answer starting from the old one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The compose field opened pre-filled with the value being replaced. Typing at the caret — which is what a click into a filled textarea invites — submitted the old text with the new text welded onto it, and the engine faithfully wrote that to every writable contributor. In a QA session it concatenated a section body and flattened a tags list. The field starts empty, with an explicit "Start from " button for anyone who does want the existing value as a base. Compose is refused outright for list-valued frontmatter, matching the engine's guard, and labelled "Reconciled value" with no Markdown preview when the field is not Markdown. Also: the source filter ignored effectiveSource, hiding a resolved item from a filter naming the very source that won it; a search term stayed active across status tabs while the empty state said only "No discrepancies in this view", so a filtered-out item read as a cleared queue; the disposition radios share a name; and a broken link can be acknowledged with a reason that fits it. Co-Authored-By: Claude Opus 5 Signed-off-by: John Siracusa --- apps/console/src/views/Conflicts.test.tsx | 240 +++++++++++++++++++++- apps/console/src/views/Conflicts.tsx | 61 +++++- 2 files changed, 289 insertions(+), 12 deletions(-) diff --git a/apps/console/src/views/Conflicts.test.tsx b/apps/console/src/views/Conflicts.test.tsx index dd5cbc2..a32913d 100644 --- a/apps/console/src/views/Conflicts.test.tsx +++ b/apps/console/src/views/Conflicts.test.tsx @@ -17,7 +17,7 @@ let root: Root function storeWith(conflicts: Conflict[], selConflict: string) { return { - mode: 'demo', query: '', + mode: 'demo', query: '', setQuery: vi.fn(), conflicts, selConflict, setSelConflict: vi.fn(), @@ -90,6 +90,58 @@ const codeConflict: Conflict = { ], } +const listConflict: Conflict = { + ...freshConflict, + id: 'decisions/primary-db::tags', + sectionKey: 'tags', + section: 'Tags', + title: 'Tags — Primary database', + kind: 'frontmatter_value', + isList: true, + contributions: [ + { layer: 'personal', sourceLayer: 'personal', value: '["postgres","oltp"]', updated: '2026-05-12' }, + { layer: 'team', sourceLayer: 'team', value: '["mysql"]', updated: '2026-06-01' }, + ], +} + +const brokenLinkConflict: Conflict = { + ...freshConflict, + id: 'decisions/primary-db::choice::missing-target', + kind: 'broken_link', + target: 'decisions/missing', + contributions: [ + { layer: 'personal', sourceLayer: 'personal', value: 'decisions/missing', updated: '2026-05-12' }, + ], +} + +const companyContributorConflict: Conflict = { + ...freshConflict, + id: 'other/concept::field', + concept: 'other/concept', + sectionKey: 'field', + section: 'Field', + title: 'Field — Other concept', + contributions: [ + { layer: 'company', sourceLayer: 'company', value: 'Company answer.', updated: '2026-05-12' }, + ], +} + +const resolvedViaEffectiveSource: Conflict = { + ...freshConflict, + id: 'decisions/primary-db::resolved-effective', + sectionKey: 'resolved-effective', + section: 'Resolved effective', + title: 'Resolved effective — Primary database', + status: 'resolved', + discrepancyStatus: 'resolved', + effectiveSource: 'company', + // Deliberately no 'company' contribution in the snapshot — the filter must + // still match on effectiveSource, not only the contributions array (F13). + contributions: [ + { layer: 'team', sourceLayer: 'team', value: 'Postgres.', updated: '2026-01-01' }, + ], +} + beforeEach(() => { ;(globalThis as typeof globalThis & { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true container = document.createElement('div') @@ -156,4 +208,190 @@ describe('Discrepancy Center', () => { await act(async () => { reason.value = 'different_scopes'; reason.dispatchEvent(new Event('change', { bubbles: true })) }) expect(submit.disabled).toBe(false) }) + + it('gives every disposition radio the same name so they behave as one group', async () => { + mocks.useStore.mockReturnValue(storeWith([safeConflict], safeConflict.id)) + await act(async () => root.render()) + const radios = Array.from(container.querySelectorAll('input[type="radio"]')) + expect(radios.length).toBeGreaterThan(1) + expect(new Set(radios.map((input) => input.name)).size).toBe(1) + expect(radios[0].name).not.toBe('') + }) + + // F22a: does ArrowDown move focus/selection between the disposition radios? + // + // jsdom cannot answer this directly — same-name radio-group arrow + // navigation is a browser default action implemented well below the DOM + // event layer (Blink's RadioInputType::handleKeydownEvent), not something + // triggered by dispatching a keydown event, trusted or not. A jsdom probe + // (`input.dispatchEvent(new KeyboardEvent('keydown', {key:'ArrowDown'}))` + // on a bare same-name radio pair, no framework involved) confirmed jsdom + // does not implement it — focus and `checked` were unchanged after the + // dispatch, so a "does ArrowDown move focus" assertion here would only + // test jsdom's fidelity, not this component. + // + // Manually verified instead, in a real Chromium tab (CDP-level keyboard + // input, not a synthetic DOM event) against the running app: focusing the + // first `cc-disposition` radio and pressing the real ArrowDown key moved + // both focus and `checked` to the second radio. No extra keydown handling + // was added — native same-name-radio-group behavior already covers this, + // which is exactly what the structural preconditions below exist to keep + // true: same `name`, no `
` boundary between them (an explicit form + // owner would scope the group to elements sharing THAT owner), and no + // radio hidden in a way (`display:none`, `disabled`) that would pull it out + // of the group's focus order. + it('keeps the disposition radios in one native focus-navigable group (no form owner, none display:none or disabled)', async () => { + mocks.useStore.mockReturnValue(storeWith([safeConflict], safeConflict.id)) + await act(async () => root.render()) + const radios = Array.from(container.querySelectorAll('input[type="radio"][name="cc-disposition"]')) + expect(radios.length).toBeGreaterThan(1) + for (const radio of radios) { + expect(radio.form).toBeNull() + expect(radio.disabled).toBe(false) + expect(getComputedStyle(radio).display).not.toBe('none') + } + }) + + it('starts the compose field empty and submits exactly what was typed, never the old value plus new text', async () => { + const store = storeWith([freshConflict], freshConflict.id) + mocks.useStore.mockReturnValue(store) + await act(async () => root.render()) + + const composeRadio = Array.from(container.querySelectorAll('input[type="radio"]')).find((input) => input.parentElement?.textContent?.includes('Write a reconciled answer'))! + await act(async () => composeRadio.click()) + + const textarea = container.querySelector('textarea[aria-label="Reconciled Markdown"]')! + expect(textarea.value).toBe('') + expect(textarea.placeholder).toContain('Write the reconciled answer') + + const submit = Array.from(container.querySelectorAll('button')).find((button) => button.textContent?.includes('Simulate reconciled answer'))! + expect(submit.disabled).toBe(true) + + await act(async () => { + const setter = Object.getOwnPropertyDescriptor(HTMLTextAreaElement.prototype, 'value')?.set + setter?.call(textarea, 'Only the freshly typed reconciliation.') + textarea.dispatchEvent(new Event('input', { bubbles: true })) + }) + expect(submit.disabled).toBe(false) + + await act(async () => submit.click()) + expect(store.decideDiscrepancy).toHaveBeenCalledWith(expect.objectContaining({ content: 'Only the freshly typed reconciliation.' })) + }) + + it('resets the compose field to empty when the selected conflict changes', async () => { + // Conflicts is a props-less memo (see the note at the bottom of this + // file's subject) and the store hooks are mocked as plain functions, not + // reactive context — so a second root.render() with the same (empty) + // props bails out via memo and never re-invokes the component. Force a + // genuine remount, the same way a real navigation to a different + // discrepancy would, to exercise the conflict.id-keyed reset effect. + mocks.useStore.mockReturnValue(storeWith([freshConflict, staleConflict], freshConflict.id)) + await act(async () => root.render()) + const composeRadio = Array.from(container.querySelectorAll('input[type="radio"]')).find((input) => input.parentElement?.textContent?.includes('Write a reconciled answer'))! + await act(async () => composeRadio.click()) + const textarea = container.querySelector('textarea[aria-label="Reconciled Markdown"]')! + await act(async () => { + const setter = Object.getOwnPropertyDescriptor(HTMLTextAreaElement.prototype, 'value')?.set + setter?.call(textarea, 'Draft for the first conflict.') + textarea.dispatchEvent(new Event('input', { bubbles: true })) + }) + expect(textarea.value).toBe('Draft for the first conflict.') + + await act(async () => root.unmount()) + root = createRoot(container) + mocks.useStore.mockReturnValue(storeWith([freshConflict, staleConflict], staleConflict.id)) + await act(async () => root.render()) + const composeRadioAfter = Array.from(container.querySelectorAll('input[type="radio"]')).find((input) => input.parentElement?.textContent?.includes('Write a reconciled answer'))! + await act(async () => composeRadioAfter.click()) + const textareaAfter = container.querySelector('textarea[aria-label="Reconciled Markdown"]')! + expect(textareaAfter.value).toBe('') + }) + + it('offers to start the compose field from the winning contributor without prefilling it automatically', async () => { + mocks.useStore.mockReturnValue(storeWith([freshConflict], freshConflict.id)) + await act(async () => root.render()) + const composeRadio = Array.from(container.querySelectorAll('input[type="radio"]')).find((input) => input.parentElement?.textContent?.includes('Write a reconciled answer'))! + await act(async () => composeRadio.click()) + + const textarea = container.querySelector('textarea[aria-label="Reconciled Markdown"]')! + expect(textarea.value).toBe('') + const startButton = Array.from(container.querySelectorAll('button')).find((button) => button.textContent?.startsWith('Start from'))! + expect(startButton).toBeTruthy() + + await act(async () => startButton.click()) + expect(textarea.value).toBe('SingleStore.') + }) + + it('disables compose for an array-typed frontmatter discrepancy and explains why', async () => { + mocks.useStore.mockReturnValue(storeWith([listConflict], listConflict.id)) + await act(async () => root.render()) + + const composeRadio = Array.from(container.querySelectorAll('input[type="radio"]')).find((input) => input.parentElement?.textContent?.includes('Write a reconciled answer'))! + expect(composeRadio.disabled).toBe(true) + expect(container.textContent).toContain('This field is a list — pick an existing answer or edit the file directly.') + }) + + it('labels a frontmatter compose field "Reconciled value" and hides the Markdown preview affordance', async () => { + const listConflictComposable: Conflict = { ...listConflict, isList: false } + mocks.useStore.mockReturnValue(storeWith([listConflictComposable], listConflictComposable.id)) + await act(async () => root.render()) + + const composeRadio = Array.from(container.querySelectorAll('input[type="radio"]')).find((input) => input.parentElement?.textContent?.includes('Write a reconciled answer'))! + await act(async () => composeRadio.click()) + + expect(container.querySelector('[aria-label="Reconciled value"]')).toBeTruthy() + expect(container.querySelector('[aria-label="Reconciled Markdown"]')).toBeFalsy() + expect(container.textContent).not.toContain('Preview Markdown') + }) + + it('offers "Target not created yet" for a broken-link discrepancy', async () => { + mocks.useStore.mockReturnValue(storeWith([brokenLinkConflict], brokenLinkConflict.id)) + await act(async () => root.render()) + const acknowledgeRadio = Array.from(container.querySelectorAll('input[type="radio"]')).find((input) => input.parentElement?.textContent?.includes('Keep the scoped difference'))! + await act(async () => acknowledgeRadio.click()) + const options = Array.from(container.querySelectorAll('[aria-label="Acknowledgement reason"] option')).map((option) => option.textContent) + expect(options).toContain('Target not created yet') + }) + + it('never offers "Target not created yet" for a non-broken-link discrepancy', async () => { + mocks.useStore.mockReturnValue(storeWith([freshConflict], freshConflict.id)) + await act(async () => root.render()) + const otherRadio = Array.from(container.querySelectorAll('input[type="radio"]')).find((input) => input.parentElement?.textContent?.includes('Keep the scoped difference'))! + await act(async () => otherRadio.click()) + const otherOptions = Array.from(container.querySelectorAll('[aria-label="Acknowledgement reason"] option')).map((option) => option.textContent) + expect(otherOptions).not.toContain('Target not created yet') + }) + + it('names the active search in the empty state and clears it on request', async () => { + const store = storeWith([freshConflict], freshConflict.id) + store.query = 'nothing will match this' + mocks.useStore.mockReturnValue(store) + await act(async () => root.render()) + + expect(container.textContent).toContain('No matches for "nothing will match this" in this status.') + const clear = Array.from(container.querySelectorAll('button')).find((button) => button.textContent === 'Clear search')! + await act(async () => clear.click()) + expect(store.setQuery).toHaveBeenCalledWith('') + }) + + it('still shows the generic empty state when no search is active', async () => { + mocks.useStore.mockReturnValue(storeWith([], '')) + await act(async () => root.render()) + expect(container.textContent).toContain('No discrepancies in this view') + expect(container.textContent).not.toContain('No matches for') + }) + + it('matches a resolved discrepancy on effectiveSource even when its contribution snapshot lacks that source (F13)', async () => { + mocks.useStore.mockReturnValue(storeWith([companyContributorConflict, resolvedViaEffectiveSource], resolvedViaEffectiveSource.id)) + await act(async () => root.render()) + + const resolvedTab = Array.from(container.querySelectorAll('button')).find((button) => button.textContent === 'Resolved')! + await act(async () => resolvedTab.click()) + expect(container.textContent).toContain('Resolved effective') + + const sourceSelect = container.querySelector('[aria-label="Source"]')! + await act(async () => { sourceSelect.value = 'company'; sourceSelect.dispatchEvent(new Event('change', { bubbles: true })) }) + + expect(container.textContent).toContain('Resolved effective') + }) }) diff --git a/apps/console/src/views/Conflicts.tsx b/apps/console/src/views/Conflicts.tsx index 587fde5..47daff2 100644 --- a/apps/console/src/views/Conflicts.tsx +++ b/apps/console/src/views/Conflicts.tsx @@ -19,6 +19,11 @@ const REASONS: { value: AcknowledgementReason; label: string }[] = [ { value: 'source_specific_authority', label: 'Source-specific authority' }, { value: 'other', label: 'Other' }, ] +// Broken-link-only: acknowledging why a link target doesn't exist yet is a +// distinct reason from the general four above. The engine's allowedReasons +// set (service.mjs) already accepts this value — verified before adding it +// here, since the UI must never offer a reason the API would 400. +const TARGET_MISSING_REASON: { value: AcknowledgementReason; label: string } = { value: 'target_missing', label: 'Target not created yet' } function formatDate(value?: string | null) { if (!value) return 'Date not recorded' @@ -140,17 +145,32 @@ function DecisionPanel({ conflict, onClose }: { conflict: Conflict; onClose: () const { mode, decideDiscrepancy, setDiscrepancyPriority, resolvingConflict, resolutionError, openFilesScope } = useStoreData() const [action, setAction] = useState<'choose_contribution' | 'compose' | 'acknowledge'>('choose_contribution') const [selectedSource, setSelectedSource] = useState(conflict.effectiveSource ?? conflict.contributions[0]?.sourceLayer ?? '') - const [content, setContent] = useState(conflict.contributions[0]?.value ?? '') + // Starts EMPTY, never pre-filled with an existing contributor's value. A + // compose field seeded with the old answer let a caret-position edit submit + // old+new concatenated as the "reconciled" content — real on-disk + // corruption in QA. "Start from " below is the only way old content + // enters this field, and it is an explicit click, never automatic. + const [content, setContent] = useState('') const [reasonCode, setReasonCode] = useState('') const [note, setNote] = useState('') const [preview, setPreview] = useState(false) const busy = resolvingConflict === conflict.id const cannotWrite = conflict.kind === 'broken_link' + // The engine 400s a compose against an array-typed frontmatter value (a + // list field) — service.mjs rejects it outright. Disable the disposition + // here instead of round-tripping into that error. + const composeDisabled = conflict.kind === 'frontmatter_value' && conflict.isList === true + const isFrontmatterValue = conflict.kind === 'frontmatter_value' + const winningSource = conflict.effectiveSource ?? conflict.contributions[0]?.sourceLayer ?? null + const winningValue = conflict.contributions.find((item) => item.sourceLayer === winningSource)?.value + const reasonOptions = conflict.kind === 'broken_link' + ? [...REASONS.slice(0, -1), TARGET_MISSING_REASON, REASONS[REASONS.length - 1]] + : REASONS useEffect(() => { setAction('choose_contribution') setSelectedSource(conflict.effectiveSource ?? conflict.contributions[0]?.sourceLayer ?? '') - setContent(conflict.contributions[0]?.value ?? '') + setContent('') setReasonCode('') setNote('') setPreview(false) @@ -171,12 +191,29 @@ function DecisionPanel({ conflict, onClose }: { conflict: Conflict; onClose: () {resolutionError &&
Decision not applied. {resolutionError.message}
}
Choose a safe disposition - + {action === 'choose_contribution' && } - - {action === 'compose' &&