Skip to content

Commit 53980c1

Browse files
Krishcalinclaude
andcommitted
Nothing said when an answer was measured
`queries.list_systems` returned no date, and `coverage_for_scope` took `DISTINCT ON (system_id) ... ORDER BY started_at DESC` — the newest run per system, with the date discarded. So a dashboard built on an upload from March was indistinguishable from one built this morning. Measured on the live sample estate, seven systems rendered identically in the systems table: PRD/100 assessed that morning D01/300 assessed 20 days earlier P01/100 assessed 20 days earlier T01/200 assessed 20 days earlier DEV/300 NEVER ASSESSED — no complete run, ever QAS/200 NEVER ASSESSED — no complete run, ever under a heading reading "Open findings across 7 systems", which is a claim about two systems nothing had ever looked at. Same shape as the release-gate defect fixed in cbd5d00: an absence of measurement rendering as a result. Every system now carries `last_assessed`, `days_since_assessed` and `assessed_runs`, counting COMPLETED runs only — a failed run says the scanner stopped, not that the system is clean. Three states rather than two: current, stale, and never measured. `null` is not zero. STALE_AFTER_DAYS is 35 and is not an invented number: SAP publishes Security Notes on Security Patch Day, the second Tuesday of each month, so an export older than one cycle cannot account for a patch day that has since passed. 35 is one cycle plus the slack between two second Tuesdays. It governs EMPHASIS ONLY — the measured date is returned whatever it is set to, so no threshold can hide the age of an answer. The mutation run found this rule correct only by accident: `(days or 0) > threshold` excludes a never-assessed system because `None or 0` is 0, which is the None-is-zero conflation the feature exists to prevent, sitting inside the function that reports it. Rewritten to filter on `is not None`, with a test for the third state. Eight mutations, all caught. AND A PRE-EXISTING DEFECT, found by checking whether a class I had just written existed. Findings.tsx used `text-medium`; the token is `med`. Tailwind emits nothing for a class naming no token — no error, no build failure — so the "partial data" marker, which flags a finding resting on incomplete evidence, rendered in plain body text. That is precisely a qualification nobody reaches. tests/test_colour_tokens_resolve.py now scans every .tsx against the @theme block; reintroducing the bug fails it with the file and line. Not done, deliberately: the Domains, CSF and Trend roll-ups discard `started_at` the same way through `latest_coverage`. Changing that contract touches six call sites and their tests, and is better as its own piece of work than as a rushed extension of this one. 13 new Postgres tests, 4 new console tests. 4761 Python tests pass with the database attached, 119 frontend. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent cbd5d00 commit 53980c1

11 files changed

Lines changed: 734 additions & 8 deletions

File tree

CLAUDE.md

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -877,6 +877,17 @@ a page.** Everything below exists so they do not.
877877
`SODCOV-000` rendering as card 151 of 419 while telling the reader to read it
878878
first; both landing in the offline report and not the console. If you add a
879879
statement meant to be read, check where it renders.
880+
6. **An answer has a date, and a system with no date has no answer.**
881+
`queries.list_systems` carries `last_assessed` / `days_since_assessed` /
882+
`assessed_runs`, counting **completed runs only** — a failed run says the
883+
scanner stopped, not that the system is clean. `null` means never assessed,
884+
and it is a THIRD state: not fresh, not stale, unmeasured. The `days or 0`
885+
idiom files it as the freshest thing in the estate, so `queries` filters on
886+
`is not None` explicitly and `tests/test_estate_freshness.py` holds it.
887+
Measured on the sample estate when this landed: seven systems, two of which
888+
(DEV/300, QAS/200) had never been scanned at all, rendering identically to a
889+
system assessed that morning, under a heading that read *"Open findings
890+
across 7 systems"*.
880891

881892
### What exists
882893

@@ -890,6 +901,9 @@ a page.** Everything below exists so they do not.
890901
| `EXPORT-001/002` | Supplied exports that could not be decoded, or decoded only via a fallback |
891902
| `finding["evidence"]` | Per-**check** completeness, attached in `BaseAuditor.finding()` beside the standards mapping |
892903
| `docs/CHECK_FIRING.md` | How many of our own checks are proven to fire. CI fails if it drifts |
904+
| `queries.list_systems` | Per system: when a completed run last assessed it. `null` = never |
905+
| `queries.estate_freshness` | Current / stale / never, the oldest age, and which systems — built from the same rows the table renders, so headline and table cannot drift |
906+
| `STALE_AFTER_DAYS` | 35: one SAP Security Patch Day cycle plus slack. Governs EMPHASIS only — the measured date is always returned, so no threshold can hide an age |
893907

894908
### If you touch this
895909

@@ -903,6 +917,12 @@ a page.** Everything below exists so they do not.
903917
`coverage.check_sources()`, so it follows automatically — but only for reads it
904918
can see. A read through a new helper shape needs the accessor detection
905919
extended, or every finding falls back to the module's whole source list.
920+
- Writing a colour into the console? Use a token from `index.css`'s `@theme
921+
inline` block. Tailwind emits **nothing** for a class naming no token — no
922+
error, no warning — and the element renders in whatever it inherited. That is
923+
how `text-medium` (the token is `med`) left the "partial data" marker
924+
untinted, which is precisely a qualification nobody reaches. Guarded by
925+
`tests/test_colour_tokens_resolve.py`.
906926

907927
## Conventions & gotchas (learned the hard way)
908928

frontend/src/api/types.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,33 @@ export interface SapSystem {
134134
created_at: string
135135
landscape_name: string
136136
deployment_mode: DeploymentMode
137+
/** When a complete run last assessed this system, or `null` for a system
138+
* nothing has ever scanned. NULL IS NOT ZERO: a system with no run has no
139+
* age, and rendering it as "0 days" or as an empty cell beside populated
140+
* ones puts it in the same visual class as one assessed this morning. */
141+
last_assessed: string | null
142+
/** Days since `last_assessed`, computed by PostgreSQL against its own clock
143+
* so two readers in different time zones cannot disagree about staleness.
144+
* `null` exactly when `last_assessed` is. */
145+
days_since_assessed: number | null
146+
/** Completed runs only. A failed or cancelled run is not an assessment. */
147+
assessed_runs: number
148+
}
149+
150+
/** server/queries.py `estate_freshness` — how much of the estate's answer is
151+
* current. Built from the same rows the systems table renders, so the headline
152+
* and the table can never disagree. */
153+
export interface EstateFreshness {
154+
systems: number
155+
current: number
156+
stale: number
157+
never_assessed: number
158+
stale_after_days: number
159+
/** Age of the oldest assessment, or `null` when nothing has been assessed at
160+
* all. Systems never scanned are excluded — they have no age to be oldest. */
161+
oldest_days: number | null
162+
never_assessed_labels: string[]
163+
stale_labels: string[]
137164
}
138165

139166
/** server/queries.py `list_landscapes` — landscape.*. */
@@ -349,6 +376,7 @@ export interface DashboardSummary {
349376
export interface Dashboard {
350377
summary: DashboardSummary
351378
systems: SapSystem[]
379+
freshness: EstateFreshness
352380
recent_runs: ScanRun[]
353381
crq: CrqPortfolio | null
354382
crq_scenarios: CrqScenario[]
Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
/**
2+
* When was this answer measured?
3+
*
4+
* THE DEFECT. The systems table listed platform, tier, criticality, exposure,
5+
* mode and owner — and nothing about when any of it was last looked at. On the
6+
* live sample estate seven systems rendered identically, and two of them
7+
* (DEV/300 and QAS/200) had never been scanned at all: no complete run, ever.
8+
* Above that table sat the sentence
9+
*
10+
* Open findings across 7 systems.
11+
*
12+
* which counted every REGISTERED system whether or not anything had measured
13+
* it. The figure was drawn from five and read as a claim about seven.
14+
*
15+
* The distinction these tests hold is the one the whole feature turns on: a
16+
* system nobody has scanned has no age. It must never render as an em dash
17+
* beside six populated cells, and never as "0 days".
18+
*/
19+
import { render, screen, waitFor } from '@testing-library/react'
20+
import { MemoryRouter } from 'react-router'
21+
import { beforeEach, describe, expect, it, vi } from 'vitest'
22+
23+
import { Dashboard } from './Dashboard'
24+
25+
vi.mock('../api/client', () => ({
26+
dashboard: vi.fn(),
27+
csf: vi.fn(),
28+
domains: vi.fn(),
29+
ApiError: class ApiError extends Error {
30+
status: number
31+
constructor(status: number, message: string) { super(message); this.status = status }
32+
},
33+
}))
34+
vi.mock('../lib/title', () => ({ useTitle: () => {} }))
35+
36+
import { csf, dashboard, domains } from '../api/client'
37+
38+
function system(over: Record<string, unknown>) {
39+
return {
40+
id: 1, landscape_id: 1, platform: 'abap', external_key: null,
41+
label: 'PRD/100', sid: 'PRD', client: '100', tier: 'prod',
42+
product: null, release: null, kernel_patch: null, btp_subaccount: null,
43+
criticality: 'critical', exposure_zone: 'internal', owner: null,
44+
tags: [], created_at: '2026-01-01T00:00:00Z',
45+
landscape_name: 'L', deployment_mode: 'rise_pce',
46+
last_assessed: '2026-09-01T00:00:00Z', days_since_assessed: 0,
47+
assessed_runs: 1,
48+
...over,
49+
}
50+
}
51+
52+
function view(systems: unknown[], freshness: Record<string, unknown>) {
53+
return {
54+
summary: {
55+
by_severity: { CRITICAL: 1, HIGH: 2, MEDIUM: 3, LOW: 4 },
56+
by_remediation_owner: { customer_fixable: 10 },
57+
by_state: { open: 10 }, open_total: 10,
58+
expired_acceptances: 0, weak_identity: 0, regressed: 0, sod_trust: null,
59+
},
60+
systems,
61+
freshness: {
62+
systems: systems.length, current: systems.length, stale: 0,
63+
never_assessed: 0, stale_after_days: 35, oldest_days: 0,
64+
never_assessed_labels: [], stale_labels: [],
65+
...freshness,
66+
},
67+
recent_runs: [], crq: null, crq_scenarios: [],
68+
}
69+
}
70+
71+
function draw() {
72+
return render(<MemoryRouter><Dashboard /></MemoryRouter>)
73+
}
74+
75+
beforeEach(() => {
76+
vi.clearAllMocks()
77+
vi.mocked(csf).mockRejectedValue(new Error('not under test'))
78+
vi.mocked(domains).mockRejectedValue(new Error('not under test'))
79+
})
80+
81+
describe('the estate says when it was last measured', () => {
82+
it('does not count a never-scanned system in a claim drawn from findings', async () => {
83+
vi.mocked(dashboard).mockResolvedValue(view(
84+
[system({ id: 1 }), system({ id: 2, label: 'DEV/300', last_assessed: null,
85+
days_since_assessed: null, assessed_runs: 0 })],
86+
{ systems: 2, current: 1, never_assessed: 1,
87+
never_assessed_labels: ['DEV/300'] },
88+
) as never)
89+
draw()
90+
expect(await screen.findByText(/across 1 of 2 registered systems/))
91+
.toBeInTheDocument()
92+
expect(screen.getByText(/never been assessed, so nothing here is a statement/))
93+
.toBeInTheDocument()
94+
})
95+
96+
it('says "never", not a dash, for a system nothing has scanned', async () => {
97+
// An em dash is what the Owner column shows for "not recorded". Reusing it
98+
// here would put "nobody has ever looked at this system" in the same
99+
// visual class as "no owner set".
100+
vi.mocked(dashboard).mockResolvedValue(view(
101+
[system({ label: 'DEV/300', last_assessed: null,
102+
days_since_assessed: null, assessed_runs: 0 })],
103+
{ systems: 1, current: 0, never_assessed: 1, oldest_days: null,
104+
never_assessed_labels: ['DEV/300'] },
105+
) as never)
106+
draw()
107+
expect(await screen.findByText('never')).toBeInTheDocument()
108+
})
109+
110+
it('names the stale systems and how old the oldest answer is', async () => {
111+
vi.mocked(dashboard).mockResolvedValue(view(
112+
[system({ label: 'D01/300', days_since_assessed: 90,
113+
last_assessed: '2026-06-03T00:00:00Z' })],
114+
{ systems: 1, current: 0, stale: 1, oldest_days: 90,
115+
stale_labels: ['D01/300'] },
116+
) as never)
117+
draw()
118+
expect(await screen.findByText(/Last assessed over 35 days ago: D01\/300/))
119+
.toBeInTheDocument()
120+
expect(screen.getByText(/oldest answer here is 90 days old/))
121+
.toBeInTheDocument()
122+
expect(screen.getByText('90d ago')).toBeInTheDocument()
123+
})
124+
125+
it('stays quiet when every system is current', async () => {
126+
// A banner that is always on is a banner nobody reads. This one has to
127+
// stay silent on a healthy estate to mean anything on an unhealthy one.
128+
vi.mocked(dashboard).mockResolvedValue(view([system({})], {}) as never)
129+
draw()
130+
await screen.findByText(/across 1 of 1 registered system/)
131+
await waitFor(() => {
132+
expect(screen.queryByText(/never been assessed/)).not.toBeInTheDocument()
133+
expect(screen.queryByText(/Last assessed over/)).not.toBeInTheDocument()
134+
})
135+
expect(screen.getByText('today')).toBeInTheDocument()
136+
})
137+
})

frontend/src/routes/Dashboard.test.tsx

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,11 @@ function view(crq: unknown) {
6363
sod_trust: null,
6464
},
6565
systems: [],
66+
freshness: {
67+
systems: 0, current: 0, stale: 0, never_assessed: 0,
68+
stale_after_days: 35, oldest_days: null,
69+
never_assessed_labels: [], stale_labels: [],
70+
},
6671
recent_runs: [],
6772
crq,
6873
crq_scenarios: [],

frontend/src/routes/Dashboard.tsx

Lines changed: 76 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { ShieldHalf } from 'lucide-react'
44
import { ApiError, csf, dashboard, domains } from '../api/client'
55
import type {
66
CsfView, Dashboard as DashboardData, DomainsView, FindingState,
7-
RemediationOwner, ScanRun, SecurityDomain, Severity,
7+
RemediationOwner, SapSystem, ScanRun, SecurityDomain, Severity,
88
} from '../api/types'
99
import { Donut } from '../components/Donut'
1010
import { useTitle } from '../lib/title'
@@ -78,7 +78,11 @@ export function Dashboard() {
7878
if (error) return <div className="banner banner-bad" role="alert">{error}</div>
7979
if (!data) return <p className="text-ink3 text-[13px]">Loading…</p>
8080

81-
const { summary, systems, recent_runs, crq, crq_scenarios } = data
81+
const { summary, systems, freshness, recent_runs, crq, crq_scenarios } = data
82+
// Registered minus never-scanned. Every aggregate on this screen is built
83+
// from findings, and a system with no completed run contributes none — so it
84+
// inflates the denominator of a claim it is not part of.
85+
const assessed = freshness.systems - freshness.never_assessed
8286
const reducible = num(crq?.detail?.['reducible_ale_p90'])
8387

8488
return (
@@ -114,10 +118,50 @@ export function Dashboard() {
114118
<ShieldHalf size={22} className="text-accent" />
115119
Security Posture
116120
</h1>
121+
{/* IT USED TO SAY "across {systems.length} systems", which counted every
122+
REGISTERED system whether or not anything had ever scanned it. On the
123+
sample estate that made a figure drawn from five systems read as a
124+
figure about seven, and the two it silently included were the two
125+
nobody had ever looked at. */}
117126
<p className="text-ink2 mb-5">
118-
Open findings across {systems.length} system{systems.length === 1 ? '' : 's'}.
127+
Open findings across {assessed} of {systems.length}{' '}
128+
registered system{systems.length === 1 ? '' : 's'}.
129+
{freshness.never_assessed > 0 && (
130+
<> {freshness.never_assessed}{' '}
131+
{freshness.never_assessed === 1 ? 'has' : 'have'} never been
132+
assessed, so nothing here is a statement about
133+
{freshness.never_assessed === 1 ? ' it' : ' them'}.</>
134+
)}
119135
</p>
120136

137+
{/* WHEN THE ANSWER IS OLD, SAY SO WHERE THE ANSWER IS. A staleness figure
138+
on a settings page nobody opens is not a control; this sits directly
139+
above the numbers it qualifies. */}
140+
{(freshness.never_assessed > 0 || freshness.stale > 0) && (
141+
<div className={freshness.never_assessed > 0 ? 'banner banner-bad'
142+
: 'banner banner-warn'}>
143+
<strong className="font-semibold">
144+
{freshness.never_assessed > 0 && (
145+
<>Never assessed: {freshness.never_assessed_labels.join(', ')}.{' '}</>
146+
)}
147+
{freshness.stale > 0 && (
148+
<>Last assessed over {freshness.stale_after_days} days ago:{' '}
149+
{freshness.stale_labels.join(', ')}.</>
150+
)}
151+
</strong>{' '}
152+
<span className="text-ink2">
153+
SAP publishes Security Notes monthly, so an export older than one
154+
cycle cannot account for the patch day that has passed since
155+
{freshness.oldest_days !== null && (
156+
<> — the oldest answer here is {freshness.oldest_days} days old</>
157+
)}.{' '}
158+
<Link className="text-accent hover:underline" to="/upload">
159+
Upload a current export →
160+
</Link>
161+
</span>
162+
</div>
163+
)}
164+
121165
{summary.expired_acceptances > 0 && (
122166
<div className="banner banner-bad">
123167
<strong className="font-semibold">
@@ -437,7 +481,7 @@ export function Dashboard() {
437481
<th className={TH}>System</th><th className={TH}>Platform</th>
438482
<th className={TH}>Tier</th><th className={TH}>Criticality</th>
439483
<th className={TH}>Exposure</th><th className={TH}>Mode</th>
440-
<th className={TH}>Owner</th>
484+
<th className={TH}>Owner</th><th className={TH}>Last assessed</th>
441485
</tr>
442486
</thead>
443487
<tbody>
@@ -458,11 +502,17 @@ export function Dashboard() {
458502
<td className={`${TD} text-ink2`}>{s.exposure_zone}</td>
459503
<td className={`${TD} text-[12px] text-ink2`}>{s.deployment_mode}</td>
460504
<td className={`${TD} text-ink2`}>{s.owner ?? '—'}</td>
505+
{/* An em dash would put "never scanned" in the same visual
506+
class as "no owner recorded". They are not the same kind of
507+
absence: one is a missing detail, the other means every
508+
other cell on this row describes a system nothing has
509+
looked at. */}
510+
<td className={TD}>{assessedCell(s)}</td>
461511
</tr>
462512
))}
463513
{systems.length === 0 && (
464514
<tr>
465-
<td colSpan={7} className={EMPTY}>
515+
<td colSpan={8} className={EMPTY}>
466516
No systems registered yet. Upload a bundle to create one.
467517
</td>
468518
</tr>
@@ -590,6 +640,27 @@ const CSF_SLUG: Record<string, string> = {
590640
DE: 'detect', RS: 'respond', RC: 'recover',
591641
}
592642

643+
/** The one cell on the systems table that can invalidate the whole row. */
644+
function assessedCell(s: SapSystem) {
645+
if (s.last_assessed === null) {
646+
return <span className="text-crit font-semibold">never</span>
647+
}
648+
const days = s.days_since_assessed ?? 0
649+
const stale = days > STALE_AFTER_DAYS
650+
return (
651+
<span className={stale ? 'text-high' : 'text-ink2'}
652+
title={new Date(s.last_assessed).toISOString().slice(0, 10)}>
653+
{days === 0 ? 'today' : `${days}d ago`}
654+
</span>
655+
)
656+
}
657+
658+
/** Mirrors server/queries.py STALE_AFTER_DAYS. The server is the authority —
659+
* `freshness.stale_after_days` carries its value for anything the reader is
660+
* TOLD — and this constant only tints a cell, so a drift between them can
661+
* change a colour and never a number. */
662+
const STALE_AFTER_DAYS = 35
663+
593664
const CARD = 'rounded-lg border border-cardline bg-panel p-4'
594665
const G4 = 'grid gap-3.5 [grid-template-columns:repeat(auto-fit,minmax(210px,1fr))]'
595666
const G2 = 'grid gap-3.5 [grid-template-columns:repeat(auto-fit,minmax(340px,1fr))]'

frontend/src/routes/Findings.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@ export function Findings() {
303303
page carries the full explanation. Only the incomplete
304304
case is marked — its absence already means complete. */}
305305
{f.latest_evidence?.complete === false && (
306-
<span className="pill st text-medium ml-1.5"
306+
<span className="pill st text-med ml-1.5"
307307
title={`This check ran without ${
308308
(f.latest_evidence.missing_sources ?? []).length
309309
} of its module's exports: ${

server/app.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -990,6 +990,11 @@ def api_dashboard(user: Dict[str, Any] = Depends(current_user)):
990990
return {
991991
"summary": queries.dashboard_summary(scope),
992992
"systems": queries.list_systems(scope),
993+
# HOW MUCH OF THE ABOVE IS CURRENT. Every other number on this screen is
994+
# an aggregate over systems, and an aggregate cannot say that two of its
995+
# members have never been scanned — it just counts nothing for them and
996+
# reads as a clean result. See queries.list_systems.
997+
"freshness": queries.estate_freshness(scope),
993998
"recent_runs": queries.recent_runs(scope, limit=10),
994999
"crq": latest_crq,
9951000
"crq_scenarios": crq.scenarios_for_run(latest_crq["run_id"]) if latest_crq else [],

server/mcp.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,10 @@
5353
TOOLS: Dict[str, Dict[str, Any]] = {
5454
"list_systems": {
5555
"description": "The SAP systems and SaaS tenants in scope, with their "
56-
"deployment mode and tier.",
56+
"deployment mode, tier, and when each was last assessed. "
57+
"last_assessed is null for a system no completed scan has "
58+
"ever covered — that is not the same as recently clean, "
59+
"and nothing else in this estate is a statement about it.",
5760
"properties": {},
5861
"handler": lambda scope, args: queries.list_systems(scope),
5962
},

0 commit comments

Comments
 (0)