## Summary
Follow-up to [PR 1406](https://github.com/AI-Builder-Team/Aerie/pull/1406) (merged), which fixed the Real Estate tab's shape error so it renders. Reviewing the rendered tab exposed two real problems, plus a family of siblings with the same cause.
- A stale OK read as healthy. Surtr's Observer stopped evaluating on 2026-09-18 while the pipeline kept running daily, yet the tab showed a green OK, "evaluated 2d ago" next to "Last Run 1h ago", and said nothing about the mismatch. That OK is a verdict about an earlier run, not current assurance.
- "Disabled" schedule wording was misleading for event-driven pipelines. mart-aerie-rebl3-sites-refresh has no schedule; it runs whenever aerie-rebl3-raw-sync succeeds. The tab said "Disabled / no upcoming run".
## Problem A: stale verdict
isRealEstateVerdictStale (pure, in real-estate-health.ts) decides whether the Observer's verdict is about an earlier run than the latest one. Nothing is claimed, and nothing is an error, when the run is still in progress (completedAt null), there is no last run, or lastEvaluatedAt is null (already the UNAVAILABLE path in the builder).
1. Which run, when both ids are known (added after Mercy's review): the same run id means current whatever the clocks say. Different ids and an evaluation stored at or after the latest run finished means stale; Surtr evaluates a run after it finishes, so that only happens when an operator re-evaluated an older run, which leaves the latest run unevaluated and looks current by timestamp alone.
2. Otherwise, and whenever an id is missing, the timestamps decide: stale when lastRun.completedAt is more than REAL_ESTATE_VERDICT_STALE_GRACE_MS (30 minutes) after observer.lastEvaluatedAt.
3. An unreadable timestamp is stale, never current (fail closed; see the class audit).
Why 30 minutes (read from Surtr, not modified): Surtr/src/derive/observer/sweep.ts has DEFAULT_LOOKBACK_MIN = 15 and a sweep only evaluates runs that finished inside it (findRecentTerminalRunsRedshift filters on ended_at >= since); infra/lib/surtr-app-stack.ts fires the sweep every 5 minutes. So the last sweep that can still see a run starts about 15 minutes after it finished. Twice that leaves room for evaluation latency, the concurrency cap and a 409 while an earlier sweep runs. A run not evaluated within about 30 minutes of completing never will be automatically.
Semantics: staleness never improves a verdict.
- OK becomes a non-green Stale badge and Observer card, with the note "Verdict is for an earlier run — last evaluated Xd ago; the latest run finished Yh ago and has not been evaluated by the Observer."
- WARN and CRITICAL keep their badge and tone exactly as before and gain the same note (bad news stays visible). UNAVAILABLE, whether a failed fetch or Surtr's own verdict, is unchanged and gets no second caveat anywhere on the tab.
- Surtr's verdict vocabulary and the fail-closed unavailable behaviour are untouched. STALE exists only as a display state (getRealEstateDisplayState), not as a verdict; normalizeRealEstateVerdict("STALE") is still UNAVAILABLE.
- The "Open findings" card no longer shows a green 0 when it is read off a stale evaluation; a non-zero count is always shown as before.
Client-side, not server-side. Everything the rule needs is already in Surtr's response, so it stays one pure helper with no clock, shared by the page and the tests. The payload gains five optional fields, all plain pass-throughs of what Surtr serializes: schedule.expression, lastRun.triggerType, lastRun.triggeredBy, lastRun.runId and observer.lastEvaluatedRunId. No verdict is computed server-side; a server-side flag would only be one more field for the guard to trust.
## Problem B: schedule wording
- Verified in Surtr: presentRun serializes trigger_type and triggered_by (?? null), the detail endpoint serializes schedule.expression, and the fetch schema here already had all three optional or nullable, so no schema change was needed.
- Wording (describeRealEstateSchedule): no expression and not enabled is Not scheduled, and only when the last run's trigger type is EVENT and triggeredBy is a non-empty string it adds "Runs when triggered by <triggered-by>" (never a hard-coded pipeline id). Expression present and not enabled is Schedule disabled (warn). Enabled keeps "Enabled" with the next run.
- Schedule existence is never inferred from the expression alone (Mercy, second round). pipeline-config.ts allows an empty expression and defaults enabled to true, and registry-sync stores the empty string as null, so no expression with enabled: true is a real shape: it reads Enabled with "no schedule expression is set" as a warning, not the neutral Not scheduled. And because the registry records only the primary schedule, not additional_schedules, the plain case says "no schedule reported by Surtr" rather than claiming nothing is configured. All seven combinations of expression (string, null, not reported) and enabled are pinned by a table test.
- The literal UNKNOWN is treated as missing: create-run-record stores it when an event carries no triggered_by.
## Deferred nit from PR 1406's Mercy review
A finding whose open state Surtr's run ids cannot settle now reads Status unknown instead of rendering no status. It fit naturally (three lines in the findings list).
## Class audit
Searched the tab, payload, guard and tests for every health signal shown without freshness or provenance, and every missing or unknown value shown as a definite one. Fixed siblings:
- Item 1: header said "Checked" for Surtr's as_of (the later of the last run and the last evaluation), which is not when the tab looked. It now reads "Surtr data as of X" with a tooltip; only a placeholder payload, which carries our own check time, still says "Checked".
- Item 2: an unavailable source rendered the builder's placeholders as facts: 0 open findings in green, "Disabled", "No run recorded / never run", "No Observer findings". Those cards now read Unknown and neutral, and the findings list is not rendered.
- Item 3: last-run tone was green for any completed status except lowercase failed, so partial, timeout, FAILED and unrecognised statuses were green. Now only a finished success is green; partial is warn, failed and timeout are bad, unknown is neutral.
- Item 4: formatRelativeTime returns "just now" for any future timestamp, so every scheduled pipeline showed "next just now". Added formatTimeUntil ("in 5h", "due now"). An enabled schedule whose next run Surtr could not compute (nextRunAt is null for an unparseable expression) now reads "next run unknown", not "no upcoming run".
- Item 5: a run with no timestamps read "never run"; it now reads "time not reported". A green tone also needs a finished run.
- Item 6: the findings empty state ("No Observer findings...") carries a caveat when the verdict is stale.
- Item 7 (Mercy): freshness must be tied to the evaluated run, not only to timestamps. Audited every consumer of last_evaluated_at (the builder's null check, the stale helper, the Observer card's "evaluated X ago", the fetch schema); only the stale helper made a freshness decision, and it now compares run ids first.
- Item 8 (Mercy): every Date.parse in the freshness and schedule code now fails closed: an unreadable evaluation or completion time is stale, the note says "at an unknown time" rather than printing a dash, and an unreadable next run reads "next run unknown". The guard and fetch schema already reject such values, so this only hardens direct callers.
- Item 9 (Mercy, second round): do not infer schedule existence from the expression alone. Audited every branch combining enabled, expression and nextRunAt, including the not-reported path: no expression with enabled: true is now a warning instead of Not scheduled, and the guard rejects an upcoming run with no expression (Surtr's nextRunAt returns null without one).
## Guard invariants (each traced to Surtr, with a "why" comment as in PR 1406)
- schedule.expression is a string, null or absent: registry-sync writes p.schedule?.expression || null.
- lastRun.triggerType / triggeredBy are strings, null or absent: presentRun serializes them with ?? null.
- lastRun.runId is a string or absent: presentRun serializes run_id: run.id.
- observer.lastEvaluatedRunId is a string, null or absent: observerDetail serializes last_evaluated_run_id: view.latest?.runId ?? null.
- nextRunAt non-null requires enabled: the API computes next_run_at as scheduleEnabled ? ... : null.
- nextRunAt non-null requires an expression: Surtr's nextRunAt(expression) returns null as its first step when there is none.
Deliberately not added: "expression null implies enabled false". pipeline-config.ts allows an empty expression and defaults enabled to true, and registry-sync stores the empty string as null, so no expression with enabled: true is a real shape, and an existing server test already accepts it. Rejecting it would recreate PR 1406's failure mode; the card shows it as a warning instead.
## Known limits
- In the seconds-to-minutes between a run finishing and its evaluation being stored, the previous verdict reads Stale. That is accurate and can only understate health. If it proves noisy for a pipeline that runs more often than the grace, gate on time since completion (needs the clock threaded in).
- Two runs evaluated out of order also read Stale, because Surtr calls the newest evaluation "latest". That only understates health and cannot happen for a daily pipeline.
## Testing
- Live 2026-09-20 shape is the primary fixture (OK, evaluated 2026-09-18T04:08:10Z, run completed 2026-09-20T04:08:09Z, expression null, enabled false, EVENT triggered by pipeline:aerie-rebl3-raw-sync): run through the fetch schema, builder and guard end to end, through the route, and through SyncPage (badge Stale, no OK anywhere, exact note text, "Not scheduled" + "Runs when triggered by pipeline:aerie-rebl3-raw-sync", not green).
- Also covered: evaluation 2 minutes after the run (not stale), the exact 30:00.000 boundary and one millisecond past it, run in progress, no last run, CRITICAL plus stale (stays Critical, gains the note), same run id (current), older run re-evaluated after the latest (stale, also through SyncPage), missing ids (timestamp fallback), unreadable timestamps, Surtr's own UNAVAILABLE verdict over a retained old evaluation (no caveat anywhere), expression present and disabled, enabled schedule (in 5h, in 12m, under a minute, due now, unknown), missing or blank or UNKNOWN trigger fields (no invented text), and the placeholder-as-unknown state.
- Mutation check: I broke each new behaviour in turn (38 mutations across the boundary, grace, run-id identity, out-of-order re-evaluation, fail-closed parsing, display state, in-progress handling, unavailable suppression, schedule and trigger wording including the enabled-with-no-expression branch, every guard invariant, builder pass-through, status classification, and the page tones, banner, header and Status unknown label). Every one made a dedicated test fail; none survived.
- Passing locally: the five affected test files (207 tests), tsc for app and convex, and pnpm lint (boundaries, convex-paths, read-bounds, test-architecture, knowledge, biome). Biome reports two pre-existing warnings in chat/skill/forge-api/scripts/sindri.mjs, outside this diff.
- Not verified in a browser: Clerk auth blocks it from this session, so UI behaviour is covered by the component tests only. Keval will check it visually in a local preview.
Diff is about 1,830 changed lines, roughly two thirds tests.
## Business Value
The Real Estate Data Health tab is how operators decide whether the REBL3 sites mart can be trusted. Since 2026-09-18 the Observer has been silent while the tab showed a green OK, which is worse than no signal because it reassures. This makes that failure visible the moment the Observer falls behind, without ever softening a bad verdict, and it stops a healthy event-driven pipeline from reading as "Disabled". The class fixes remove the other places the tab could look healthy or definite without evidence (green zeros on an unreachable source, green for partial/timeout runs, "next just now", a re-evaluated old run passing as current), so the next silent failure shows up as what it is.
## Manual Effort Estimate
About 10 hours of focused work by hand, no AI: roughly 1.5h tracing Surtr (sweep cadence, presentRun, registry-sync, status and trigger vocabularies, run ids), 1.5h for the payload, guard, builder and schema check, 2h for the stale helper, display state, banner, wording and the class-audit UI fixes, 4h for the lib, guard, builder, end-to-end, route and page tests with a pinned clock, and 1h for the mutation check, lint, typecheck, review round and this write-up. For Keval to confirm/adjust.
Linear: no ticket linked; this session has no Linear access, so Keval to attach one.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
