From 8de81abc61cc42229a751c138227d69c5ebe161f Mon Sep 17 00:00:00 2001 From: Chris0Jeky Date: Sat, 5 Sep 2026 20:33:29 +0100 Subject: [PATCH 1/3] feat(review): usePaperReviewSelectors exposes a keyed evidence snapshot The six selector values left this composable with no way to say which proposal they belonged to, or whether the read that produced them had landed. A consumer therefore could not tell proposal A's settled values from A's values with B's read still running, nor a pending read from a proven absence. railEvidence answers both in one object: the state of the batch for the ACTIVE key (idle, loading, failed with the outcome kept, settled), the key the values belong to, and the two rail values themselves, withheld in every state but settled. Values and state cannot be passed apart. The bare isLoading boolean is replaced by one record holding the last batch's key and status, so loading is derived from the same fact the snapshot reads and the two cannot disagree. Every write site keeps its previous meaning: fast path settled, new batch loading, failure and success terminal, no proposal clears the record. Read semantics are unchanged: the #2460 keying, the automatic refresh, the same-action retry and waitForCoreBatch all behave as before. Refs #1940 --- .../composables/usePaperReviewSelectors.ts | 164 ++++++++++++-- .../usePaperReviewSelectors.spec.ts | 207 ++++++++++++++++++ 2 files changed, 357 insertions(+), 14 deletions(-) diff --git a/frontend/taskdeck-web/src/composables/usePaperReviewSelectors.ts b/frontend/taskdeck-web/src/composables/usePaperReviewSelectors.ts index 36e200840..97155da5d 100644 --- a/frontend/taskdeck-web/src/composables/usePaperReviewSelectors.ts +++ b/frontend/taskdeck-web/src/composables/usePaperReviewSelectors.ts @@ -127,6 +127,12 @@ export interface SimilarPastRow { date: string } +export interface SimilarPastApplyRate { + applied: number + total: number + ratio: number +} + /** * How one exact-key core evidence batch ended. * @@ -150,6 +156,47 @@ export interface CoreSelectorBatchWaitOptions { signal?: AbortSignal } +/** + * What the review rail is entitled to state about the core batch behind the + * values it is holding (#1940). + * + * - `idle` no proposal is active, so there is nothing to state at all. + * - `loading` no batch has reported for the ACTIVE key yet. The previous + * proposal's values may still sit in the refs, so none of them may be shown. + * - `failed` the batch for the active key ended with at least one rejected + * read. Emptiness is then unknown, not proven. + * - `settled` all six reads landed for the active key, so the values in the + * snapshot belong to it and an empty one is a fact about this proposal. + */ +export type PaperReviewEvidenceStatus = 'idle' | 'loading' | 'failed' | 'settled' + +/** + * The rail's values together with the state of the read that produced them, as + * ONE snapshot (#1940). + * + * Values and state cannot be passed apart, which is the point: the rail shipped + * as a pure props component holding `similarPast` and `confidenceBreakdown` with + * nothing about the fetch, so it rendered proposal A's confidence and A's rows + * under proposal B while B's batch was still in flight, and its cards read an + * empty array as "nothing exists" while the read was pending or had failed. + * A snapshot that withholds the values in every non-`settled` status makes both + * mistakes unrepresentable at the boundary rather than merely unlikely. + */ +export interface PaperReviewRailEvidence { + status: PaperReviewEvidenceStatus + /** The batch outcome behind a `failed` status; null in every other status. */ + failure: CoreSelectorBatchOutcome | null + /** + * The key these values belong to. Non-null only while `status` is `settled`, + * because every other status withholds the values and so has no identity to + * name. + */ + key: SelectorKey | null + confidenceBreakdown: ConfidenceBreakdown + similarPast: SimilarPastRow[] + similarPastApplyRate: SimilarPastApplyRate +} + export interface PaperReviewSelectors { provenance: ComputedRef provenanceMetadata: ComputedRef @@ -159,7 +206,14 @@ export interface PaperReviewSelectors { conflicts: ComputedRef history: ComputedRef similarPast: ComputedRef - similarPastApplyRate: ComputedRef<{ applied: number; total: number; ratio: number }> + similarPastApplyRate: ComputedRef + /** + * The rail-facing keyed snapshot: the two rail values plus the state and + * identity of the batch that produced them (#1940). Prefer it over the bare + * `similarPast` / `confidenceBreakdown` reads for anything that STATES + * something to the reviewer about what the values mean. + */ + railEvidence: ComputedRef loading: ComputedRef waitForCoreBatch: ( proposalId: string, @@ -192,6 +246,11 @@ function emptySideEffects(): SideEffects { const EMPTY_SIDE_EFFECT_ROWS: SideEffectRow[] = Object.freeze( [] as SideEffectRow[], ) as SideEffectRow[] +const EMPTY_APPLY_RATE: SimilarPastApplyRate = Object.freeze({ + applied: 0, + total: 0, + ratio: 0, +}) as SimilarPastApplyRate const EMPTY_CONFIDENCE: ConfidenceBreakdown = Object.freeze({ overall: null, components: Object.freeze([] as ConfidenceBreakdown['components']) as ConfidenceBreakdown['components'], @@ -236,7 +295,13 @@ function nullableIdentifiersEqual( return identifiersEqual(left, right) } -interface SelectorKey { +/** + * Identity of one evidence read: the proposal, the capture it came from and the + * revision being reviewed. Exported since #1940 because a surface that states + * something about these values has to be able to say WHICH proposal they belong + * to; the rail could not, and rendered A's evidence under B's header. + */ +export interface SelectorKey { proposalId: string captureReference: string | null revisionIdentity: string | null @@ -481,6 +546,12 @@ function mapHistory(dtos: CardHistoryRowDto[]): HistoryRow[] { })) } +function applyRateOf(rows: SimilarPastRow[]): SimilarPastApplyRate { + const applied = rows.filter((r) => r.verdict === 'applied').length + const total = rows.length + return { applied, total, ratio: total === 0 ? 0 : applied / total } +} + function mapSimilarPast(dto: SimilarPastResultDto): SimilarPastRow[] { return dto.decisions.map((d) => ({ serial: d.serial, @@ -503,7 +574,21 @@ export function usePaperReviewSelectors( const conflictsData: Ref = ref([]) const historyData: Ref = ref([]) const similarPastData: Ref = ref([]) - const isLoading = ref(false) + /** + * How the most recent core batch ended AND which key it was for (#1940). + * + * This replaces the previous bare `isLoading` boolean, which could say that a + * read was in flight but never which proposal it was for — so a consumer had + * no way to tell "A's values, settled" from "A's values, with B's read still + * running". `loading` below is derived from this record, so the two can never + * disagree. `null` means no batch has been attempted since the last proposal + * change, which is the state a surface must read as `idle`. + */ + const batchRecord: Ref<{ + key: SelectorKey + status: Exclude + failure: CoreSelectorBatchOutcome | null + } | null> = ref(null) let fetchGeneration = 0 let abortController: AbortController | null = null @@ -621,7 +706,7 @@ export function usePaperReviewSelectors( invalidateCoreBatch() discardCaptureLookup() } - isLoading.value = false + batchRecord.value = { key, status: 'settled', failure: null } if (settledCaptureMetadata && selectorKeysEqual(settledCaptureMetadata.key, key)) { provenanceMetadataData.value = settledCaptureMetadata.value } else { @@ -651,7 +736,7 @@ export function usePaperReviewSelectors( const controller = new AbortController() abortController = controller const signal = controller.signal - isLoading.value = true + batchRecord.value = { key, status: 'loading', failure: null } // Never show the previous proposal's producer while the active capture is loading. provenanceMetadataData.value = null @@ -708,7 +793,9 @@ export function usePaperReviewSelectors( discardCaptureLookup() clearSelectorData() } - isLoading.value = false + // The rail must be able to say the read failed rather than let its + // cards read the cleared refs as a proven absence of evidence (#1940). + batchRecord.value = { key, status: 'failed', failure: 'failed' } return 'failed' } @@ -728,7 +815,7 @@ export function usePaperReviewSelectors( similarPastData.value = mapSimilarPast(sim.value) settledCoreKey = key - isLoading.value = false + batchRecord.value = { key, status: 'settled', failure: null } void captureSettlement.then(([serverMetadata, capture]) => { if ( @@ -866,7 +953,7 @@ export function usePaperReviewSelectors( settledCoreKey = null settledCaptureMetadata = null discardCaptureLookup() - isLoading.value = false + batchRecord.value = null clearSelectorData() return } @@ -888,11 +975,56 @@ export function usePaperReviewSelectors( const history = computed(() => historyData.value) const similarPast = computed(() => similarPastData.value) - const similarPastApplyRate = computed(() => { - const rows = similarPast.value - const applied = rows.filter((r) => r.verdict === 'applied').length - const total = rows.length - return { applied, total, ratio: total === 0 ? 0 : applied / total } + const similarPastApplyRate = computed(() => + applyRateOf(similarPast.value), + ) + + /** + * The keyed snapshot (#1940). Two residuals collapse into one rule here: the + * values leave this composable ONLY under the key they were read for, and + * only once that read has landed. + * + * A record for a different key means the batch for the active one has not + * reported yet — the watcher starts it synchronously on the switch, so this + * is the instant between "the reviewer selected B" and "B's reads resolve" — + * and `loading` is the honest description of that instant. It is also the + * safe default if a future edit ever leaves a gap where no batch was started: + * withholding values cannot state a falsehood, publishing them can. + */ + const railEvidence = computed(() => { + const activeKey = selectorKeyForProposal(activeProposal.value) + if (!activeKey) { + return { + status: 'idle', + failure: null, + key: null, + confidenceBreakdown: EMPTY_CONFIDENCE, + similarPast: EMPTY_SIMILAR, + similarPastApplyRate: EMPTY_APPLY_RATE, + } + } + + const record = batchRecord.value + const current = record && selectorKeysEqual(record.key, activeKey) ? record : null + if (!current || current.status !== 'settled') { + return { + status: current?.status ?? 'loading', + failure: current?.failure ?? null, + key: null, + confidenceBreakdown: EMPTY_CONFIDENCE, + similarPast: EMPTY_SIMILAR, + similarPastApplyRate: EMPTY_APPLY_RATE, + } + } + + return { + status: 'settled', + failure: null, + key: activeKey, + confidenceBreakdown: confidenceData.value, + similarPast: similarPastData.value, + similarPastApplyRate: applyRateOf(similarPastData.value), + } }) onScopeDispose(() => { @@ -904,7 +1036,10 @@ export function usePaperReviewSelectors( discardCaptureLookup() }) - const loading = computed(() => isLoading.value) + // Derived from the one record above rather than tracked beside it, so a + // consumer reading `loading` and a consumer reading `railEvidence.status` + // cannot be told different stories about the same batch. + const loading = computed(() => batchRecord.value?.status === 'loading') return { provenance, @@ -916,6 +1051,7 @@ export function usePaperReviewSelectors( history, similarPast, similarPastApplyRate, + railEvidence, loading, waitForCoreBatch, } diff --git a/frontend/taskdeck-web/src/tests/composables/usePaperReviewSelectors.spec.ts b/frontend/taskdeck-web/src/tests/composables/usePaperReviewSelectors.spec.ts index 1a81dac64..33f2f4830 100644 --- a/frontend/taskdeck-web/src/tests/composables/usePaperReviewSelectors.spec.ts +++ b/frontend/taskdeck-web/src/tests/composables/usePaperReviewSelectors.spec.ts @@ -4,8 +4,10 @@ import { usePaperReviewSelectors } from '../../composables/usePaperReviewSelecto import { proposalDeepReviewApi, type CardHistoryRowDto, + type ConfidenceBreakdownDto, type ConflictRowDto, type ProvenanceRowDto, + type SimilarPastResultDto, } from '../../api/proposalDeepReviewApi' import { captureApi } from '../../api/captureApi' import type { Proposal as ApiProposal } from '../../types/automation' @@ -1476,4 +1478,209 @@ describe('usePaperReviewSelectors', () => { }) }) }) + + /** + * The rail-facing keyed snapshot (#1940 — the two residuals recorded with PR + * #2662). + * + * Both come from the same missing fact. The bare reads keep the previous + * proposal's confidence and similar-past rows until the new proposal's batch + * lands, and they hold an empty default both while a read is in flight and + * after one failed — so a consumer could neither tell A's values from B's nor + * a pending read from a proven absence. The snapshot answers both: it names + * the key its values belong to and withholds them in every other state. + */ + describe('railEvidence — the keyed snapshot the review rail renders', () => { + const A_ROW = { + serial: '#PAST-A', + title: 'A prior comparable decision', + verdict: 'Applied', + date: '2026-08-20', + } + + function proposalA() { + return makeProposal({ + id: 'p-1', + sourceType: 'Queue', + sourceReferenceId: 'capture-1', + latestRevisionId: 'rev-1', + }) + } + + function proposalB() { + return makeProposal({ + id: 'p-2', + sourceType: 'Queue', + sourceReferenceId: 'capture-2', + latestRevisionId: 'rev-2', + }) + } + + it('names the key its settled values were read for', async () => { + mockAllEndpointsEmpty() + vi.mocked(proposalDeepReviewApi.getSimilarPast).mockResolvedValue({ + decisions: [A_ROW, { ...A_ROW, serial: '#PAST-B', verdict: 'Rejected' }], + applyRate: 0.5, + }) + const selectors = usePaperReviewSelectors(computed(() => proposalA())) + + await vi.waitFor(() => { + expect(selectors.railEvidence.value.status).toBe('settled') + }) + + const evidence = selectors.railEvidence.value + expect(evidence.key).toEqual({ + proposalId: 'p-1', + captureReference: 'capture-1', + revisionIdentity: 'rev-1', + }) + expect(evidence.failure).toBeNull() + expect(evidence.similarPast.map((row) => row.serial)).toEqual(['#PAST-A', '#PAST-B']) + expect(evidence.similarPastApplyRate).toEqual({ applied: 1, total: 2, ratio: 0.5 }) + expect(evidence.confidenceBreakdown.components).toHaveLength(1) + }) + + it('withholds the previous proposal values while the new key is still loading', async () => { + mockAllEndpointsEmpty() + let releaseSimilar!: (value: SimilarPastResultDto) => void + let releaseConfidence!: (value: ConfidenceBreakdownDto) => void + vi.mocked(proposalDeepReviewApi.getSimilarPast).mockImplementation((id) => + id === 'p-1' + ? Promise.resolve({ decisions: [A_ROW], applyRate: 1 }) + : new Promise((resolve) => { + releaseSimilar = resolve + }), + ) + vi.mocked(proposalDeepReviewApi.getConfidence).mockImplementation((id) => + id === 'p-1' + ? Promise.resolve({ + overall: 0.84, + components: [{ key: 'Operation 1: create card', value: 0.84 }], + note: null, + threshold: null, + source: 'model-reported', + meetsThreshold: null, + }) + : new Promise((resolve) => { + releaseConfidence = resolve + }), + ) + + const proposal = ref(proposalA()) + const selectors = usePaperReviewSelectors(computed(() => proposal.value)) + await vi.waitFor(() => { + expect(selectors.railEvidence.value.status).toBe('settled') + }) + expect(selectors.railEvidence.value.similarPast[0]?.serial).toBe('#PAST-A') + + proposal.value = proposalB() + await nextTick() + await vi.waitFor(() => { + expect(proposalDeepReviewApi.getSimilarPast).toHaveBeenCalledTimes(2) + }) + + // The residual, exactly: the bare reads STILL hold proposal A's evidence + // while B loads. That is what the rail was rendering under B's header. + expect(selectors.similarPast.value[0]?.serial).toBe('#PAST-A') + expect(selectors.confidenceBreakdown.value.overall).toBe(0.84) + + const pending = selectors.railEvidence.value + expect(pending.status).toBe('loading') + expect(pending.failure).toBeNull() + // No identity, because nothing is being offered to render under one. + expect(pending.key).toBeNull() + expect(pending.similarPast).toEqual([]) + expect(pending.similarPastApplyRate).toEqual({ applied: 0, total: 0, ratio: 0 }) + expect(pending.confidenceBreakdown.overall).toBeNull() + expect(pending.confidenceBreakdown.components).toEqual([]) + expect(pending.confidenceBreakdown.source).toBe('not-reported') + expect(selectors.loading.value).toBe(true) + + releaseSimilar({ decisions: [], applyRate: 0 }) + releaseConfidence({ + overall: null, + components: [], + note: null, + threshold: null, + source: 'not-reported', + meetsThreshold: null, + }) + + await vi.waitFor(() => { + expect(selectors.railEvidence.value.status).toBe('settled') + }) + const settled = selectors.railEvidence.value + expect(settled.key).toEqual({ + proposalId: 'p-2', + captureReference: 'capture-2', + revisionIdentity: 'rev-2', + }) + // B's own evidence, and B's own emptiness — now a fact about B. + expect(settled.similarPast).toEqual([]) + expect(settled.confidenceBreakdown.overall).toBeNull() + }) + + it('reports a failed batch as failed, keeping the outcome, never as empty evidence', async () => { + mockAllEndpointsEmpty() + vi.mocked(proposalDeepReviewApi.getSimilarPast).mockImplementation((id) => + id === 'p-1' + ? Promise.resolve({ decisions: [A_ROW], applyRate: 1 }) + : Promise.reject(new Error('similar past unavailable')), + ) + + const proposal = ref(proposalA()) + const selectors = usePaperReviewSelectors(computed(() => proposal.value)) + await vi.waitFor(() => { + expect(selectors.railEvidence.value.status).toBe('settled') + }) + + proposal.value = proposalB() + await nextTick() + await vi.waitFor(() => { + expect(selectors.railEvidence.value.status).toBe('failed') + }) + + const failed = selectors.railEvidence.value + expect(failed.failure).toBe('failed') + expect(failed.key).toBeNull() + expect(failed.similarPast).toEqual([]) + expect(failed.confidenceBreakdown.source).toBe('not-reported') + expect(selectors.loading.value).toBe(false) + }) + + it('reports a settled empty read as settled, so emptiness can be stated', async () => { + mockAllEndpointsEmpty() + vi.mocked(proposalDeepReviewApi.getConfidence).mockResolvedValue({ + overall: null, + components: [], + note: null, + threshold: null, + source: 'deterministic', + meetsThreshold: null, + }) + const selectors = usePaperReviewSelectors(computed(() => proposalA())) + + await vi.waitFor(() => { + expect(selectors.railEvidence.value.status).toBe('settled') + }) + + const evidence = selectors.railEvidence.value + expect(evidence.failure).toBeNull() + expect(evidence.key?.proposalId).toBe('p-1') + expect(evidence.similarPast).toEqual([]) + expect(evidence.confidenceBreakdown.components).toEqual([]) + expect(evidence.confidenceBreakdown.source).toBe('deterministic') + }) + + it('is idle with no active proposal, so the rail states nothing at all', async () => { + const selectors = usePaperReviewSelectors(computed(() => null)) + + await nextTick() + + expect(selectors.railEvidence.value.status).toBe('idle') + expect(selectors.railEvidence.value.key).toBeNull() + expect(selectors.railEvidence.value.similarPast).toEqual([]) + expect(selectors.loading.value).toBe(false) + }) + }) }) From aabfddeda1ddf68f8d7d9b594aea01330be680aa Mon Sep 17 00:00:00 2001 From: Chris0Jeky Date: Sat, 5 Sep 2026 20:33:44 +0100 Subject: [PATCH 2/3] fix(review): the Paper rail renders only the active proposal's settled evidence Two residuals recorded with PR #2662, same root and one fix. Loading identity: PaperReviewView passed the bare selector reads to ReviewRightRail, so while proposal B's batch was in flight the rail showed proposal A's confidence number, A's confidence-source sentence and A's similar-past rows under B's header. The rail's evidence props and authorMeta now come from the keyed snapshot, which withholds the values unless the batch settled for the active key. Settled state: the hoisted card sentences from #2662 asserted emptiness whether the read was pending, had failed, or had genuinely found nothing. ReviewRightRail forwards evidenceState to both cards. ReviewAuthorCard reserves the confidence-source sentence for a settled read and otherwise states that the read is running or that it failed; ReviewSimilarPast does the same for its empty sentence, and the "(none found)" disclosure label, which is the same claim in miniature, is now reserved for the settled empty result too. The disclosure region opens onto a matching line in each state rather than onto nothing, so the count and disclosure semantics from #2662 are unchanged. idle states nothing at all. Copy in review.* for en, it and es. The rail's evidenceState default is loading, the conservative one: a caller that does not know the state gets cards that withhold their claims rather than cards that assert emptiness. The KNOWN GAP comments in both cards are replaced by what the code now does. Out of scope and unchanged: ReviewMain still receives the bare confidence read, so its badge can show the previous proposal's number while the new batch loads. Same root, different component, recorded on the issue. Refs #1940 --- .../taskdeck-web/src/locales/en/review.ts | 11 ++ .../taskdeck-web/src/locales/es/review.ts | 8 + .../taskdeck-web/src/locales/it/review.ts | 8 + .../paper/review/PaperReviewView.spec.ts | 186 ++++++++++++++++++ .../paper/review/ReviewAuthorCard.spec.ts | 80 +++++++- .../paper/review/ReviewSimilarPast.spec.ts | 86 +++++++- .../src/views/paper/PaperReviewView.vue | 20 +- .../views/paper/review/ReviewAuthorCard.vue | 52 +++-- .../views/paper/review/ReviewRightRail.vue | 12 ++ .../views/paper/review/ReviewSimilarPast.vue | 62 ++++-- 10 files changed, 484 insertions(+), 41 deletions(-) diff --git a/frontend/taskdeck-web/src/locales/en/review.ts b/frontend/taskdeck-web/src/locales/en/review.ts index 065fef8b9..bccbeeffb 100644 --- a/frontend/taskdeck-web/src/locales/en/review.ts +++ b/frontend/taskdeck-web/src/locales/en/review.ts @@ -484,6 +484,11 @@ export default { derivedConfidence: '{value} derived average', deterministic: 'Deterministic extraction · no model confidence', notReported: 'No model confidence reported', + // The two sentences above are claims about a response that has landed. While + // the read is in flight, or after it failed, the card says which of those it + // is holding instead of asserting an absence it cannot know (#1940). + confidenceLoading: 'Reading the confidence evidence for this proposal…', + confidenceFailed: 'Confidence evidence could not be read, so its source is unknown.', actor: { assistant: 'Assistant', capture: 'Capture', @@ -502,6 +507,12 @@ export default { // Shown inside the disclosure when there is nothing to list, so opening the // control explains itself instead of revealing an empty region (#1940). emptyDetail: 'Decisions on comparable proposals will be listed here.', + // `empty` is a fact only once the read has landed. These two say what is + // true of the other states instead of claiming an absence (#1940). + loading: 'Reading comparable past decisions…', + loadingDetail: 'Comparable decisions will be listed here once this read finishes.', + failed: 'Comparable past decisions could not be read.', + failedDetail: 'The read failed, so whether there are comparable past decisions is unknown.', details: { show: 'Show similar decisions', // The empty-state label. It keeps the `show` wording and adds the count, diff --git a/frontend/taskdeck-web/src/locales/es/review.ts b/frontend/taskdeck-web/src/locales/es/review.ts index 397a5b89f..243463de7 100644 --- a/frontend/taskdeck-web/src/locales/es/review.ts +++ b/frontend/taskdeck-web/src/locales/es/review.ts @@ -391,6 +391,9 @@ export default { derivedConfidence: 'promedio derivado {value}', deterministic: 'Extracción determinista · sin confianza del modelo', notReported: 'No se declaró confianza del modelo', + confidenceLoading: 'Leyendo la evidencia de confianza de esta propuesta…', + confidenceFailed: + 'No se pudo leer la evidencia de confianza, así que se desconoce su origen.', actor: { assistant: 'Asistente', capture: 'Captura', @@ -407,6 +410,11 @@ export default { heading: 'Decisiones parecidas anteriores', empty: 'No hay decisiones anteriores comparables.', emptyDetail: 'Aquí se mostrarán las decisiones sobre propuestas comparables.', + loading: 'Leyendo decisiones anteriores comparables…', + loadingDetail: 'Las decisiones comparables se mostrarán aquí cuando termine esta lectura.', + failed: 'No se pudieron leer las decisiones anteriores comparables.', + failedDetail: + 'La lectura falló, así que se desconoce si hay decisiones anteriores comparables.', details: { show: 'Mostrar decisiones parecidas', showEmpty: 'Mostrar decisiones parecidas (ninguna encontrada)', diff --git a/frontend/taskdeck-web/src/locales/it/review.ts b/frontend/taskdeck-web/src/locales/it/review.ts index 4504a6d23..eea72813a 100644 --- a/frontend/taskdeck-web/src/locales/it/review.ts +++ b/frontend/taskdeck-web/src/locales/it/review.ts @@ -392,6 +392,9 @@ export default { derivedConfidence: 'media derivata {value}', deterministic: 'Estrazione deterministica · nessuna confidenza del modello', notReported: 'Nessuna confidenza del modello dichiarata', + confidenceLoading: 'Lettura delle prove di confidenza per questa proposta…', + confidenceFailed: + 'Non è stato possibile leggere le prove di confidenza, quindi la loro origine è sconosciuta.', actor: { assistant: 'Assistente', capture: 'Cattura', @@ -408,6 +411,11 @@ export default { heading: 'Decisioni simili passate', empty: 'Nessuna decisione passata comparabile.', emptyDetail: 'Qui compariranno le decisioni su proposte comparabili.', + loading: 'Lettura delle decisioni passate comparabili…', + loadingDetail: 'Le decisioni comparabili compariranno qui al termine di questa lettura.', + failed: 'Non è stato possibile leggere le decisioni passate comparabili.', + failedDetail: + 'La lettura non è riuscita, quindi non si sa se esistano decisioni passate comparabili.', details: { show: 'Mostra decisioni simili', showEmpty: 'Mostra decisioni simili (nessuna trovata)', diff --git a/frontend/taskdeck-web/src/tests/views/paper/review/PaperReviewView.spec.ts b/frontend/taskdeck-web/src/tests/views/paper/review/PaperReviewView.spec.ts index baf68e533..3e6ccd191 100644 --- a/frontend/taskdeck-web/src/tests/views/paper/review/PaperReviewView.spec.ts +++ b/frontend/taskdeck-web/src/tests/views/paper/review/PaperReviewView.spec.ts @@ -7020,4 +7020,190 @@ describe('PaperReviewView', () => { } }) }) + + /** + * Right-rail evidence truth (#1940 — the two residuals recorded with PR + * #2662). + * + * The rail's cards are pure props components. They were handed the selector + * values with nothing about the read behind them, so while proposal B's batch + * was in flight they rendered proposal A's confidence number and A's + * similar-past rows under B's header, and they read an empty array as a + * proven absence whether the read was pending, had failed, or had genuinely + * found nothing. The view now passes ONE keyed snapshot: the values only + * under the key they were read for, and the state alongside them. + */ + describe('right-rail evidence identity and state (#1940)', () => { + const A_ROW = { + serial: '#PAST-A', + title: 'A prior comparable decision', + verdict: 'Applied', + date: '2026-08-20', + } + const A_CONFIDENCE = { + overall: 0.84, + components: [], + note: null, + threshold: null, + source: 'model-reported', + meetsThreshold: null, + } + const A_META = '0.84 model-reported average' + + function rail(wrapper: ReturnType) { + return wrapper.get('[data-testid="paper-review-right-rail"]') + } + + async function selectSecondProposal(wrapper: ReturnType) { + const row = wrapper + .findAll('.paper-review-q') + .find((candidate) => candidate.text().includes('Second proposal')) + expect(row, 'expected the second queue row to be present').toBeDefined() + await row!.trigger('click') + await flushPromises() + } + + it('shows none of the previous proposal evidence under the new one while its batch loads', async () => { + let releaseSimilar!: (value: unknown) => void + let releaseConfidence!: (value: unknown) => void + mocks.getSimilarPast.mockImplementation((id: string) => + id === 'proposal-a' + ? Promise.resolve({ decisions: [A_ROW], applyRate: 1 }) + : new Promise((resolve) => { + releaseSimilar = resolve + }), + ) + mocks.getConfidence.mockImplementation((id: string) => + id === 'proposal-a' + ? Promise.resolve(A_CONFIDENCE) + : new Promise((resolve) => { + releaseConfidence = resolve + }), + ) + + const wrapper = await mountView( + [ + makeProposal({ id: 'proposal-a', summary: 'First proposal' }), + makeProposal({ id: 'proposal-b', summary: 'Second proposal' }), + ], + '/workspace/review#proposal-proposal-a', + ) + + expect( + rail(wrapper).get('[data-testid="paper-review-author-confidence-source"]').text(), + ).toBe('No model confidence reported') + expect(rail(wrapper).text()).toContain(A_META) + expect( + rail(wrapper).get('[data-testid="paper-review-similar-past-details"]').text(), + ).toContain(A_ROW.title) + + await selectSecondProposal(wrapper) + + expect(wrapper.get('[data-testid="paper-review-main"]').text()).toContain('Second proposal') + // The residual: every one of these was proposal A's evidence, rendered + // under proposal B's header while B's batch was still in flight. + expect( + rail(wrapper).find('[data-testid="paper-review-author-confidence-source"]').exists(), + ).toBe(false) + expect(rail(wrapper).text()).not.toContain(A_META) + expect(rail(wrapper).text()).not.toContain(A_ROW.title) + expect(rail(wrapper).text()).not.toContain('No comparable past decisions.') + expect( + rail(wrapper).get('[data-testid="paper-review-author-confidence-state"]').text(), + ).toBe('Reading the confidence evidence for this proposal…') + expect(rail(wrapper).get('[data-testid="paper-review-similar-past-state"]').text()).toBe( + 'Reading comparable past decisions…', + ) + expect( + rail(wrapper).get('[data-testid="paper-review-similar-past-disclosure"]').text(), + ).not.toContain('none found') + + releaseSimilar({ decisions: [], applyRate: 0 }) + releaseConfidence({ + overall: null, + components: [], + note: null, + threshold: null, + source: 'not-reported', + meetsThreshold: null, + }) + await flushPromises() + + // B's own read has landed, so B's emptiness is now a fact about B. + expect(rail(wrapper).find('[data-testid="paper-review-similar-past-state"]').exists()).toBe( + false, + ) + expect(rail(wrapper).get('[data-testid="paper-review-similar-past-empty"]').text()).toBe( + 'No comparable past decisions.', + ) + expect( + rail(wrapper).get('[data-testid="paper-review-author-confidence-source"]').text(), + ).toBe('No model confidence reported') + expect(rail(wrapper).text()).not.toContain(A_ROW.title) + }) + + it('says the evidence read failed instead of claiming there is nothing to show', async () => { + mocks.getSimilarPast.mockImplementation((id: string) => + id === 'proposal-a' + ? Promise.resolve({ decisions: [A_ROW], applyRate: 1 }) + : Promise.reject(new Error('similar past unavailable')), + ) + mocks.getConfidence.mockImplementation((id: string) => + id === 'proposal-a' + ? Promise.resolve(A_CONFIDENCE) + : Promise.resolve({ + overall: null, + components: [], + note: null, + threshold: null, + source: 'not-reported', + meetsThreshold: null, + }), + ) + + const wrapper = await mountView( + [ + makeProposal({ id: 'proposal-a', summary: 'First proposal' }), + makeProposal({ id: 'proposal-b', summary: 'Second proposal' }), + ], + '/workspace/review#proposal-proposal-a', + ) + + await selectSecondProposal(wrapper) + + expect(rail(wrapper).get('[data-testid="paper-review-similar-past-state"]').text()).toBe( + 'Comparable past decisions could not be read.', + ) + expect( + rail(wrapper).get('[data-testid="paper-review-author-confidence-state"]').text(), + ).toBe('Confidence evidence could not be read, so its source is unknown.') + expect(rail(wrapper).text()).not.toContain('No comparable past decisions.') + expect(rail(wrapper).text()).not.toContain('No model confidence reported') + expect(rail(wrapper).text()).not.toContain(A_ROW.title) + expect(rail(wrapper).text()).not.toContain(A_META) + expect( + rail(wrapper).get('[data-testid="paper-review-similar-past-disclosure"]').text(), + ).not.toContain('none found') + }) + + it('keeps the settled empty sentences from #2662 exactly as they were', async () => { + const wrapper = await mountView([makeProposal()]) + + expect(rail(wrapper).get('[data-testid="paper-review-similar-past-empty"]').text()).toBe( + 'No comparable past decisions.', + ) + expect( + rail(wrapper).get('[data-testid="paper-review-author-confidence-source"]').text(), + ).toBe('No model confidence reported') + expect( + rail(wrapper).get('[data-testid="paper-review-similar-past-disclosure"]').text(), + ).toContain('none found') + expect( + rail(wrapper).find('[data-testid="paper-review-similar-past-state"]').exists(), + ).toBe(false) + expect( + rail(wrapper).find('[data-testid="paper-review-author-confidence-state"]').exists(), + ).toBe(false) + }) + }) }) diff --git a/frontend/taskdeck-web/src/tests/views/paper/review/ReviewAuthorCard.spec.ts b/frontend/taskdeck-web/src/tests/views/paper/review/ReviewAuthorCard.spec.ts index 8498d119f..1b7a634fa 100644 --- a/frontend/taskdeck-web/src/tests/views/paper/review/ReviewAuthorCard.spec.ts +++ b/frontend/taskdeck-web/src/tests/views/paper/review/ReviewAuthorCard.spec.ts @@ -4,6 +4,7 @@ import ReviewAuthorCard from '../../../../views/paper/review/ReviewAuthorCard.vu import type { ConfidenceBreakdown, ConfidenceValueSource, + PaperReviewEvidenceStatus, } from '../../../../composables/usePaperReviewSelectors' /** @@ -31,7 +32,15 @@ function breakdownOf( return { overall: components.length > 0 ? 0.9 : null, components, threshold: null, source, note } } -function mountCard(breakdown: ConfidenceBreakdown, authorMeta = '') { +/** + * `settled` is the default because every case below is about what a landed + * confidence read says. The state cases pass their own. + */ +function mountCard( + breakdown: ConfidenceBreakdown, + authorMeta = '', + evidenceState: PaperReviewEvidenceStatus = 'settled', +) { return mount(ReviewAuthorCard, { attachTo: document.body, props: { @@ -41,6 +50,7 @@ function mountCard(breakdown: ConfidenceBreakdown, authorMeta = '') { proposedTime: '18:00', proposedNum: '001', breakdown, + evidenceState, }, }) } @@ -148,6 +158,74 @@ describe('ReviewAuthorCard', () => { }) }) + /** + * #1940, the second residual recorded with PR #2662. `EMPTY_CONFIDENCE` — + * zero components, source `not-reported` — is what the composable holds while + * a read is in flight and after one failed, so the sentence above was also + * being rendered as a statement about a response that never arrived. + */ + describe('an absent breakdown it cannot yet explain', () => { + const stateLine = '[data-testid="paper-review-author-confidence-state"]' + + it('says the read is still running rather than that no confidence was reported', () => { + const wrapper = mountCard(breakdownOf('not-reported'), '', 'loading') + + expect(wrapper.find(sourceLine).exists()).toBe(false) + expect(wrapper.text()).not.toContain('No model confidence reported') + expect(wrapper.get(stateLine).text()).toBe( + 'Reading the confidence evidence for this proposal…', + ) + expect(wrapper.get(stateLine).isVisible()).toBe(true) + + wrapper.unmount() + }) + + it('says the read failed rather than that no confidence was reported', () => { + const wrapper = mountCard(breakdownOf('not-reported'), '', 'failed') + + expect(wrapper.find(sourceLine).exists()).toBe(false) + expect(wrapper.text()).not.toContain('No model confidence reported') + expect(wrapper.get(stateLine).text()).toBe( + 'Confidence evidence could not be read, so its source is unknown.', + ) + + wrapper.unmount() + }) + + // The deterministic wording is a claim about the producer, not about the + // number, so it is withheld by the same rule. + it('withholds the deterministic sentence too until the read has landed', () => { + const wrapper = mountCard(breakdownOf('deterministic'), '', 'loading') + + expect(wrapper.find(sourceLine).exists()).toBe(false) + expect(wrapper.text()).not.toContain('Deterministic extraction') + + wrapper.unmount() + }) + + it('states nothing at all when no proposal is active', () => { + const wrapper = mountCard(breakdownOf('not-reported'), '', 'idle') + + expect(wrapper.find(sourceLine).exists()).toBe(false) + expect(wrapper.find(stateLine).exists()).toBe(false) + + wrapper.unmount() + }) + + it('adds no state line to bars that already show where the number came from', () => { + const wrapper = mountCard( + breakdownOf('model-reported', [{ key: 'Operation 1: create card', value: 0.92 }]), + '0.90 model-reported average', + 'loading', + ) + + expect(wrapper.find(stateLine).exists()).toBe(false) + expect(wrapper.find(sourceLine).exists()).toBe(false) + + wrapper.unmount() + }) + }) + it('keeps the disclosure present and correctly paired while collapsed', () => { const wrapper = mountCard(breakdownOf('deterministic')) const button = wrapper.get('[data-testid="paper-review-confidence-disclosure"]') diff --git a/frontend/taskdeck-web/src/tests/views/paper/review/ReviewSimilarPast.spec.ts b/frontend/taskdeck-web/src/tests/views/paper/review/ReviewSimilarPast.spec.ts index 12d0a36e0..12ebe4b71 100644 --- a/frontend/taskdeck-web/src/tests/views/paper/review/ReviewSimilarPast.spec.ts +++ b/frontend/taskdeck-web/src/tests/views/paper/review/ReviewSimilarPast.spec.ts @@ -1,7 +1,10 @@ import { describe, expect, it } from 'vitest' import { mount } from '@vue/test-utils' import ReviewSimilarPast from '../../../../views/paper/review/ReviewSimilarPast.vue' -import type { SimilarPastRow } from '../../../../composables/usePaperReviewSelectors' +import type { + PaperReviewEvidenceStatus, + SimilarPastRow, +} from '../../../../composables/usePaperReviewSelectors' /** * ReviewSimilarPast — the card must tell the truth BEFORE the disclosure opens @@ -25,7 +28,12 @@ const ROWS: SimilarPastRow[] = [ const EMPTY_SENTENCE = 'No comparable past decisions.' -function mountCard(rows: SimilarPastRow[]) { +/** + * `settled` is the default because it is the state the cases below were written + * for: every assertion about what an empty list MEANS presumes the read that + * proves it has landed. The state cases pass their own. + */ +function mountCard(rows: SimilarPastRow[], evidenceState: PaperReviewEvidenceStatus = 'settled') { return mount(ReviewSimilarPast, { attachTo: document.body, props: { @@ -34,6 +42,7 @@ function mountCard(rows: SimilarPastRow[]) { rows.length === 0 ? { applied: 0, total: 0, ratio: 0 } : { applied: 1, total: 2, ratio: 0.5 }, + evidenceState, }, }) } @@ -149,6 +158,79 @@ describe('ReviewSimilarPast', () => { }) }) + /** + * #1940, the second residual recorded with PR #2662. An empty array reaches + * this card from three different situations and only one of them makes the + * empty sentence true. The card now receives which one it is holding. + */ + describe('an empty list it cannot yet call empty', () => { + const stateLine = '[data-testid="paper-review-similar-past-state"]' + const stateDetail = '[data-testid="paper-review-similar-past-state-detail"]' + + it('says the read is still running rather than that nothing was found', async () => { + const wrapper = mountCard([], 'loading') + const button = wrapper.get('[data-testid="paper-review-similar-past-disclosure"]') + + expect(wrapper.find('[data-testid="paper-review-similar-past-empty"]').exists()).toBe(false) + expect(wrapper.text()).not.toContain(EMPTY_SENTENCE) + expect(wrapper.get(stateLine).text()).toBe('Reading comparable past decisions…') + expect(wrapper.get(stateLine).isVisible()).toBe(true) + // The label is the same claim in miniature: "(none found)" is a finding. + expect(button.text()).toContain('Show similar decisions') + expect(button.text()).not.toContain('none found') + + await button.trigger('click') + + expect(wrapper.get(stateDetail).text()).toBe( + 'Comparable decisions will be listed here once this read finishes.', + ) + expect( + wrapper.find('[data-testid="paper-review-similar-past-empty-detail"]').exists(), + ).toBe(false) + + wrapper.unmount() + }) + + it('says the read failed rather than that nothing was found', async () => { + const wrapper = mountCard([], 'failed') + const button = wrapper.get('[data-testid="paper-review-similar-past-disclosure"]') + + expect(wrapper.find('[data-testid="paper-review-similar-past-empty"]').exists()).toBe(false) + expect(wrapper.text()).not.toContain(EMPTY_SENTENCE) + expect(wrapper.get(stateLine).text()).toBe('Comparable past decisions could not be read.') + expect(button.text()).not.toContain('none found') + + await button.trigger('click') + + expect(wrapper.get(stateDetail).text()).toBe( + 'The read failed, so whether there are comparable past decisions is unknown.', + ) + + wrapper.unmount() + }) + + it('states nothing at all when no proposal is active', () => { + const wrapper = mountCard([], 'idle') + + expect(wrapper.find('[data-testid="paper-review-similar-past-empty"]').exists()).toBe(false) + expect(wrapper.find(stateLine).exists()).toBe(false) + expect(wrapper.text()).not.toContain(EMPTY_SENTENCE) + + wrapper.unmount() + }) + + it('adds no state line to rows that already answer the question', () => { + const wrapper = mountCard(ROWS, 'loading') + + expect(wrapper.find(stateLine).exists()).toBe(false) + expect(wrapper.get('[data-testid="paper-review-similar-past-details"]').text()).toContain( + 'A prior comparable decision', + ) + + wrapper.unmount() + }) + }) + // Matches ReviewProvenance.vue: `v-show` alone leaves the collapsed region in // the accessibility tree for anything that reads the DOM rather than the // computed style. diff --git a/frontend/taskdeck-web/src/views/paper/PaperReviewView.vue b/frontend/taskdeck-web/src/views/paper/PaperReviewView.vue index a67acf5ad..9d43c49ed 100644 --- a/frontend/taskdeck-web/src/views/paper/PaperReviewView.vue +++ b/frontend/taskdeck-web/src/views/paper/PaperReviewView.vue @@ -1079,7 +1079,10 @@ const proposedNum = computed(() => { }) const authorMeta = computed(() => { - const c = selectors.confidenceBreakdown.value + // The KEYED snapshot, not the bare read: this sentence carries a confidence + // number into the rail, and the bare read still holds the previous proposal's + // number while the new one's batch is in flight (#1940). + const c = selectors.railEvidence.value.confidenceBreakdown if (c.overall === null || !Number.isFinite(c.overall)) return '' if (c.source === 'model-reported') { return t('review.author.modelConfidence', { value: c.overall.toFixed(2) }) @@ -3046,6 +3049,14 @@ async function onClearBoardScope() { + import { computed, ref } from 'vue' import PaperStamp from '../../../components/paper/PaperStamp.vue' -import type { ConfidenceBreakdown } from '../../../composables/usePaperReviewSelectors' +import type { + ConfidenceBreakdown, + PaperReviewEvidenceStatus, +} from '../../../composables/usePaperReviewSelectors' /** * ReviewAuthorCard — author badge with absolute-positioned PaperStamp @@ -16,21 +19,15 @@ import type { ConfidenceBreakdown } from '../../../composables/usePaperReviewSel * sources, which left this sentence as the only statement on screen about where * the number came from — hidden behind a control with no reason to be opened. * - * KNOWN GAP, unfixable at this layer (#1940), mirroring ReviewSimilarPast. The - * card receives a breakdown and nothing about the fetch that produced it, and - * `usePaperReviewSelectors` initialises `confidenceData` to `EMPTY_CONFIDENCE` - * — zero components, source `not-reported`, no note — resets it there on every - * proposal switch, and LEAVES it there when the batch fails, with no flag - * (`evidenceUnavailable` is set only from the Apply-time refresh, never from - * the page-load batch). So "No model confidence reported" also renders while - * the read is in flight and after it failed, where it is a claim about a - * response that never arrived. The composable exposes `loading`, but nothing - * threads it into `ReviewRightRail`, and the only place that could is - * `PaperReviewView.vue`. + * The empty breakdown is ambiguous on its own, exactly as in ReviewSimilarPast: + * `usePaperReviewSelectors` holds `EMPTY_CONFIDENCE` — zero components, source + * `not-reported` — while a read is in flight and after one failed, so "No model + * confidence reported" was also a claim about a response that never arrived. + * `evidenceState` resolves it (#1940): the source sentence is reserved for a + * settled read, and the other two states say what is actually true of them. * - * The wrong-state copy predates #1940: the same sentence rendered inside the - * disclosure. Hoisting it makes an existing false claim easier to see rather - * than creating one, and the gap stays tracked on #1940. + * `idle` states nothing at all. It means no proposal is active, and the rail + * does not render without one, so there is no sentence to write for it. */ const props = defineProps<{ authorName: string @@ -39,6 +36,8 @@ const props = defineProps<{ proposedTime: string proposedNum: string breakdown: ConfidenceBreakdown + /** State of the core evidence batch this breakdown came from. */ + evidenceState: PaperReviewEvidenceStatus }>() const confidenceDetailsExpanded = ref(false) @@ -48,6 +47,20 @@ const confidenceDisclosureId = 'paper-review-confidence-disclosure' /** No per-component bars to show, so the source sentence is all there is. */ const noComponents = computed(() => props.breakdown.components.length === 0) +/** The only state in which the absent breakdown says something about the model. */ +const settledSource = computed(() => noComponents.value && props.evidenceState === 'settled') + +/** + * Which honest not-yet-known sentence replaces it otherwise. Bars on screen + * already show where the number came from, so no state line is added to them. + */ +const pendingStateKey = computed<'confidenceLoading' | 'confidenceFailed' | null>(() => { + if (!noComponents.value) return null + if (props.evidenceState === 'loading') return 'confidenceLoading' + if (props.evidenceState === 'failed') return 'confidenceFailed' + return null +}) + /** * The heading is derived from what is actually rendered, not from the claimed * source: `model-reported` with an empty components array would otherwise @@ -88,7 +101,7 @@ function barColor(value: number): string {

@@ -98,6 +111,13 @@ function barColor(value: number): string { : $t('review.author.notReported') }}

+

+ {{ $t(`review.author.${pendingStateKey}`) }} +