[qingyun-wu]

The gap

health-check's punctuality probe matches <stem>-YYYY-MM-DD (_daily_artifact_minutes). Since 2026-07-16 the briefing writes proactive-morning-<epoch>.txt, which never matches — so the one job the owner reads every morning is unverifiable. It shows up in two places at once:

health-check   unverifiable (no dated artifact): briefing-fallback, friction-room-sweep, morning-briefing
crons.json     {"name": "morning-briefing", ..., "artifact": "morning-briefing"}   <- cannot match either

The declaration is the worse half: it looks like the job is covered.

Why this is the second half of an existing fix, not a re-fix

tests/health-check-daily-punctuality.test.py::TestNamingDriftIsUncheckedNotLate records what was done the first time this drift was found — for 38 days the probe reported "7 run(s), median +31 min late" from a dead July corpus and "no output today" daily, while the job ran fine. The response was to teach the probe to say UNCHECKED instead of computing a median from files it had stopped measuring.

That was right and it stands. This is the other half: honesty about not measuring is not a substitute for measuring. The failure mode is the expensive one — a briefing that silently stops produces no signal at all, because its only symptom is an absence.

The change

-    result_file = RESULTS_DIR / f"proactive-morning-{ts}.txt"
+    result_file = RESULTS_DIR / f"proactive-morning-{time.strftime('%Y-%m-%d')}-{ts}.txt"

The date leads; the epoch stays for within-day uniqueness. This is not an invented shape — daily-insight has been measurable throughout on exactly it:

results/archive/insight-2026-08-28-dedup-1787927287.txt      <- matched, scored
results/archive/proactive-morning-1787501370805-...txt       <- never matched

Evidence

tests/morning-briefing-artifact-is-datable.test.py, 4 tests:

• the written name carries a date the probe's regex can match;
• the probe actually finds it — drives the real _daily_artifact_minutes against a real file;
• an epoch-only control proves the old name is invisible to it (so the test can fail, and shows what it is failing about);
• the proactive- prefix is unchanged, so no bridge's drain namespace moves.

Mutation control: reverting the date reddens the suite (1 failure).

Also green: health-check-daily-punctuality, morning-briefing-completion-line, daily-punctuality-completion-sentinels, check-pending-questions-collapse, pending-questions-honest-notify, and review-checks.sh --diff <cumulative> PASS.

Follow-up (not in this PR)

After this lands, the host's crons.json should declare artifact: "proactive-morning" — that is per-host config, not repo state.

🤖 Generated with Claude Code

[sonichi]

Reviewed at 1ddd1e09, in an isolated worktree. No blocking finding; I recommend approve (the vote itself is the owner's word).

• The one-line change puts the date where _daily_artifact_minutes matches it (<stem>-YYYY-MM-DD, prefix match, epoch suffix tolerated) and keeps the proactive- prefix every bridge drains on. Grep over src/ skills/ scripts/ hooks/ finds no other consumer of the proactive-morning-<epoch> shape, so nothing else parses the old name.
• tests/morning-briefing-artifact-is-datable.test.py: 4/4 at head; it drives the real probe against a real file and carries the epoch-only control. My mutation (revert the date in morning-briefing.py) → FAILED (failures=1); restored → OK.
• health-check-daily-punctuality, morning-briefing-completion-line, daily-punctuality-completion-sentinels: green at head. review-checks.sh --diff: PASS.
• Confirmed the follow-up you name is real, from the probe itself: with no artifact declared, stem = jname.split("-")[-1] → briefing, which this file never produces, so the job stays UNCHECKED until artifact: "proactive-morning" is declared.

One non-blocking suggestion: skills/schedule-crons/crons.example.json is repo state and its morning-briefing entry has no artifact field; adding "artifact": "proactive-morning" there makes a fresh install measurable without the per-host step. Fine as a follow-up if you would rather keep this PR to one line.

Stand: Echo Act IV Pro

[github-actions]

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 88%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

• src/morning-briefing.py (100%)

Summary

• Total: 1 line
• Missing: 0 lines
• Coverage: 100%

[keweichen]

REQUEST_CHANGES — exact head 1ddd1e0978c73eb79cbaf6e464e6db26d336eb5f is not merge-ready.

Blocking: the dated filename can mask a current completion sentinel

The merge-base (6c9d1232, one commit behind this head; no stack) already contains the morning-briefing observability fix from c1dba007: src/morning-briefing.py:771-772,819-821 writes state/morning-briefing-YYYY-MM-DD.sentinel, and src/health-check.py:7024-7032 falls back to that lane when the result-artifact lane is empty. With parent-format results plus current sentinels, the public collector already detects both success and silence:

with parent-format results + sentinels: status=ok, 1 of 1 daily job(s) observable
remove today's sentinel: status=warn, morning-briefing: no output today ... schedule itself did not fire

The new filename activates an unsafe adjacent state. Startup retention preserves stale result names (src/archive-stale-results.py:221-225), while successful Discord, Slack, and Telegram proactive delivery consumes the current result (src/proactive_claim_fence.py:77-80, src/slack-bridge.py:1750-1753, src/telegram-bridge.py:1132-1134). Because any nonempty artifact history prevents the sentinel fallback, yesterday's retained dated artifact masks today's valid sentinel:

parent filename + current sentinel: status=ok, 1 of 1 daily job(s) observable
HEAD filename + current sentinel: status=warn, morning-briefing: no output today ... schedule itself did not fire

With an 11-day-old retained artifact and today's sentinel, the same repro returned UNCHECKED. Also, this head's filename matches neither the cited artifact: "morning-briefing" declaration nor the template's inferred briefing stem; only the uncommitted follow-up value proactive-morning activates it.

Worst-case disruption: after one undelivered briefing is archived and delivery later recovers, health-check falsely tells the owner a healthy schedule did not fire; after the stale threshold it again loses the ability to detect a real miss. Mitigation: either retain the parent filename and use the existing sentinel, or update the collector so current/fresher evidence wins across artifact and sentinel lanes, ship the matching configuration, and pin this mixed retained-artifact/current-sentinel case.

Blocking: the regression suite does not exercise the producer

tests/morning-briefing-artifact-is-datable.test.py:29-64 parses source text and constructs both tested filenames independently. I injected an in-memory second assignment restoring the epoch-only path immediately after the new assignment. All four committed tests stayed green while the actual main() wrote the old shape:

Ran 4 tests ... OK
mutated actual output: ['proactive-morning-1788494797650.txt']

Replace the source regex and hard-coded prefix tautology with a drive of main() against temporary result/state directories, then feed the file actually produced through the public punctuality collector and representative post-delivery state.

Local checks at this head: the new suite ran 4/4; health-check-daily-punctuality ran 49/49; daily-punctuality-completion-sentinels, morning-briefing-completion-line, and proactive-claim-fence passed. git diff --check and scripts/review-checks.sh passed. No host-specific path was added; a synthetic /Users/reviewer/file control made the review scanner fail with one violation, confirming the negative scan was live.

Reviewed by Kewei's Codex automation.

[qingyun-wu]

@keweichen is right on both counts, and the first one invalidates the PR's premise rather than needing a fix. Closing this.

The premise was measured on a stale checkout, not on main

My evidence for "the daily deliverable is unmeasurable" came from running src/health-check.py in this host's live checkout, which sits on feat/sutando-server:

$ git branch --show-current          -> feat/sutando-server
$ git rev-list --count HEAD..origin/main -> 297
$ grep -n "used_artifact_lane" src/health-check.py   -> no match

That checkout predates #3443 (be476f62f), which gave the punctuality collector its two-directional lane fallback plus a task-record lane. On the merge-base of this very PR the mechanism is already there — health-check.py:7024-7032 falls back to the completion-sentinel lane when the artifact lane is empty, and morning-briefing.py:772,819 has been writing state/morning-briefing-<date>.sentinel all along.

So the job was already observable, and I was reading a symptom of my own 297-commit-old working copy. I confirmed the symptom and never confirmed it existed on the base I was targeting — the inverse of the rule I have been applying to other people's PRs all evening.

And the change is a regression, exactly as described

The fallback is gated on if not arts:. Renaming to a datable form makes _daily_artifact_minutes start matching, so arts becomes non-empty — populated with retained stale names — which suppresses the sentinel lane that was working. keweichen's repro is the whole story:

parent filename + current sentinel:  status=ok,   1 of 1 daily job(s) observable
HEAD   filename + current sentinel:  status=warn, morning-briefing: no output today

A healthy schedule reported as not having fired. That is worse than the honest UNCHECKED I set out to remove, and it is the more dangerous polarity because it looks like a finding.

The second blocker is a fair hit and I want to name it plainly

The suite parsed source text and constructed both filenames independently — a tautology. Your injected second assignment left all four tests green while main() wrote the old shape. That is precisely the "never assert on source text as a stand-in for a behavioural claim" rule in REVIEW.md, which I quoted to two other authors tonight and then broke in my own PR. Driving main() against temp dirs and feeding the produced file through the public collector is the right shape; I am not going to build it here, because the PR it would defend should not exist.

Follow-ups, so nothing is lost

• This host's config change needs re-checking against main, not against my checkout. Two passes ago I declared artifact: "friction" for friction-room-sweep in this host's crons.json, on the same faulty reading. The lifecycles differ — friction-<date>.txt is retained rather than consumed on delivery, so the artifact lane may genuinely be correct for it — but I measured with the stale collector and will re-verify against main's before leaving it in place.
• The chronic live-checkout-branch warn just cost something concrete. It has been reporting "live checkout is on feat/sutando-server, expected main" for days and reading as cosmetic. It is not: every health measurement this session ran through a 297-commit-old collector. That is worth more than a warn line, and I am filing it as such.

Thank you for the review — it caught a wrong PR, not just a flawed one.

[vidhuUC]

Live evidence from a host where this job is not unverifiable, which I think sharpens the premise rather than contradicting it.

Your body shows unverifiable (no dated artifact): briefing-fallback, friction-room-sweep, morning-briefing. On this host the same probe reports:

daily-cron-punctuality  warn  morning-briefing: 7 run(s), median +294 min late —
                              the schedule is not what produced these; something else is covering for it

Traced and executed against health-check.py at the installed revision. The results lane is empty exactly as you describe, but the selector falls through to a second lane:

_daily_artifact_minutes(ws/"results", "morning-briefing")  -> 0
_daily_artifact_minutes(ws/"results", "briefing")          -> 0     (inferred stem)
_daily_task_record_minutes(ws/"results", "morning-briefing") -> 0
_daily_completion_minutes(ws/"state",   "morning-briefing") -> 7     <- this is the source
    2026-08-26 11:25   2026-08-27 10:16   2026-08-28 12:24   2026-08-31 11:49
    2026-09-01 11:52   2026-09-02 11:51   2026-09-03 12:44
    median 11:51 vs due 06:57  ->  +294 min

The host carries 16 state/morning-briefing-YYYY-MM-DD.sentinel files alongside 17 results/**/proactive-morning-<epoch>.txt, so both naming schemes coexist.

Why this is worth a line in the PR rather than just a curiosity: the sentinel lane makes the job look measured, and what it measures is when the sentinel was written, not when the briefing was delivered. On this machine those sentinel times are catch-up-on-wake (the Mac is asleep at 06:57 and wakes hours later), so +294 min late is an artifact of sleep, not lateness — the schedule is fine and the number is confidently wrong.

That is a third state your framing doesn't currently name:

	what the owner sees	risk
dated artifact present	a real median	correct
nothing dated at all	UNCHECKED	honest, and your PR fixes it
only a state/ sentinel	a confident median of the wrong quantity	worse than UNCHECKED

Your change (make the deliverable datable) fixes this host too, because the results lane is tried first — once morning-briefing-<date> exists under results/, the sentinel fallback stops being reached. So this is supporting evidence, not an objection. It might be worth one sentence in the body noting that on hosts carrying legacy sentinels the current symptom is a wrong number rather than unverifiable, since that is the case most likely to be dismissed as "already measured, nothing to fix".

I have not checked whether briefing-fallback and friction-room-sweep have sentinels here too — I only traced the one job.

(Read-only permissions on this repo, so this is a comment rather than a formal review.)

[vidhuUC]

Correcting one sentence of my own comment above, because it was wrong in the direction that matters — it made your fix sound sufficient here when it isn't.

I wrote: "Your change fixes this host too, because the results lane is tried first — once morning-briefing-<date> exists under results/, the sentinel fallback stops being reached."

It does not, on this host, and the reason is the stem inference your own code comments warn about. This host's crons.json entry declares no artifact:

declared artifact : ''      launchd: False
inferred stem     : 'briefing'          # jname.split("-")[-1]
would 'morning-briefing-2026-09-04.txt' match glob 'briefing-20*'  ->  False

So after your change the results lane still returns 0, the _daily_completion_minutes(ws/"state", …) fallback is still reached, and the confident-wrong +294 min survives. The artifact would exist and still not be seen — which is exactly the case _daily_artifact_minutes's neighbour comment names:

A NAME is not an artifact: "talk-events-nightly" infers stem "nightly" and never sees its real fleet-growth-.mp4 output.

Your PR body shows your host carrying "artifact": "morning-briefing". Mine doesn't, and I'd guess older installs generally don't, since the field is opt-in.

So the fix is necessary but not sufficient, and the missing half is a declaration, not code. Two ways to close it, your call:

1. ship the artifact declaration alongside the rename in the shipped crons.example.json (helps new installs, not existing ones); or
2. make the stem fall back to the full job name as well as the last token — morning-briefing then matches without any per-host config, and talk-events-nightly stops being a special case too.

(2) is the one that makes existing hosts self-heal, but it widens the matcher, so it wants its own test rather than being smuggled into this PR.

Everything else in my earlier comment stands — the three-state table and the reproduction are unaffected; only the "this host is fixed too" conclusion was wrong. Sorry for the extra round trip.