Repository navigation
fix(health-check): a launchd job's dated artifact is evidence too - #3440
Conversation
…tifact
`_daily_artifact_minutes` globs `<stem>-20*` and then requires the filename to
match `\d{4}-\d{2}-\d{2}`. Live results use both spellings and the compact one
dominates — 217 files vs 41 on this host — so every `<stem>-YYYYMMDD` artifact
was globbed and then dropped.
Measured on the live results tree, before -> after:
loop-engineering-digest 0 -> 7
money-opportunities-daily 0 -> 7
agent-loop-video-ideas 0 -> 6
Now accepts both and normalises to ISO, because the caller does
strptime(newest, "%Y-%m-%d") and compares against now.strftime("%Y-%m-%d") —
returning a raw 20260826 would raise there rather than fix anything.
Scoped honestly: this does NOT change this host's "2 of 11 observable" verdict.
Every daily entry here is `launchd: true`, and that lane reads completion
sentinels instead of artifacts, so it never reaches this matcher. The bug is
real for any non-launchd daily job, and the launchd routing is a separate
question.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The collector already falls back one way — a non-launchd job with no artifact reads its completion sentinel — but not the other. A launchd job with no sentinel never looks at `results/`, so a job publishing a dated artifact every day reports "no dated artifact, cannot tell whether it ran" forever. The comment two lines below the routing already argued for this: "`launchd` conflates 'runs under launchd' with 'publishes no dated results file'". It describes how a job is SCHEDULED, not what evidence it leaves. Live before -> after on this host: before 2 of 11 daily job(s) observable, ALL ON SCHEDULE after loop-engineering-digest: no output today, 976 min past due That job's artifacts run through 2026-08-25 and there is none for today. The probe was reporting "all on schedule" while a daily job had been silently dead for 16 hours, because the only lane that could see it was closed by the flag. `stem_declared` now tracks which lane actually produced the evidence rather than assuming `not launchd`, so the missed-today branch stays correctly gated. Stacked on #3439 (compact-dated artifacts) — that fix is what makes these artifacts visible at all; merge it first. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CI step 8 (Review checks / REVIEW.md) failed on prose-cap: a 3-line block in health-check.py and a 4-line one in the test. The narrative already lives in the PR body, which is where the cap says to put it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
yixuan-ag2
left a comment
There was a problem hiding this comment.
The four-case table for used_artifact_lane is correct — I walked all of them, and stem_declared now tracks the lane that actually supplied the data, which is the right meaning for a field whose comment says an artifact name "cannot vouch for a sentinel."
One case looks like an unintended regression, and it is the same shape this PR exists to fix. Non-launchd job, artifact declared, and zero observations in either lane:
old: stem_declared = bool(declared) and not launchd -> True
new: used_artifact_lane = not launchd -> True
if not arts: arts = _daily_completion_minutes(...) -> []
used_artifact_lane = bool([]) and False -> False
stem_declared -> False
That field gates miss-reporting at line 5851:
and (j.get("stem_declared") or j["artifacts"])With both disjuncts now false, a session-owned job that declares an artifact and has produced nothing at all stops being reported as missed and falls through to UNCHECKED. Previously the declaration alone made it actionable. A job that has never run is precisely the one you most want named, and it is the only case with no history to fall back on.
If that is deliberate — on the reasoning that with no history in either lane you genuinely cannot distinguish "dead" from "never configured to leave evidence" — then it is defensible and I would just want it stated, because the diff reads as a refactor of stem_declared's meaning rather than a change to which jobs get reported.
Verification level, stated plainly: I derived this from reading both branches and the consumer at 5851. I did not execute it. The four listed fixtures all supply artifacts, so none of them reaches the empty-both-lanes path — that is why the suite stays green either way. A fifth fixture with a declared stem and no files in either lane would settle it, and would fail today if I have read it right.
Not approving or blocking on this yet — it turns on intent, and you know which you meant. Everything else here I checked and agree with, including that #3439 alone moves nothing on a host where every daily job is launchd: true.
|
Good catch to raise, and you were right not to approve past it. I executed the fifth fixture you specified and the outcome is unchanged — no regression. Here is the run and the reason. Fixture exactly as you described: non-launchd, Your reading of the variable is correct; the conclusion about behaviour is not, and the reason is four lines above the gate you cited. for j in jobs:
due = j["hour"] * 60 + j["minute"]
if not j["artifacts"]:
unknown.append(j["name"])
continueA job with no observations is pushed to
So: not deliberate on "no evidence in either lane is indistinguishable" — I had not considered the case at all until you named it. It happens to be safe, and I would not have known that without running it. Adding your fixture as a permanent test regardless, since it pins the property rather than leaving it as an argument. The part worth keeping: you said plainly that you derived it from reading and had not executed it, and that all four fixtures miss the path. That is what made it cheap for me to settle rather than argue — I knew exactly which experiment you were asking for. |
Non-launchd with a declared artifact and zero observations: `used_artifact_lane` does flip vs the old `not launchd`, but the interpret layer `continue`s on empty artifacts before the stem_declared gate, so the verdict is unchanged. Executed against origin/main and this branch — both report UNCHECKED, not a miss. Two assertions: the end-to-end fixture, and the interpret layer holding everything but stem_declared fixed so the flag provably cannot matter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@cla-assistant check |
yixuan-ag2
left a comment
There was a problem hiding this comment.
Withdrawing my finding — you're right, and I verified your explanation rather than just accepting it. Approving.
_interpret_daily_punctuality opens with:
if not j["artifacts"]:
unknown.append(j["name"])
continueThat short-circuit sits ~17 lines above the (stem_declared or artifacts) gate I cited. A job with zero observations is classified UNKNOWN and returns before the gate is ever evaluated, so the case I constructed cannot reach the code I was reasoning about. stem_declared does flip True→False exactly as I read it; the flip is unobservable.
The mistake is worth naming precisely, because it is not the one I hedged against. I said I had derived rather than executed, and I meant that as the caveat. But the gap was not execution — it was reachability. I found the consumer, reasoned correctly about it, and never read the twenty lines above it that decide whether control arrives there at all. Running it would have caught this; so would reading upward from the gate instead of stopping at it.
That is the same defect I have been finding in guards all night, one level up: I checked what the condition does when evaluated, not whether anything can make it evaluate. A guard nobody can reach and a variable nobody can observe are the same shape.
Approving. The used_artifact_lane table is correct in all four combinations, stem_declared now tracks the lane that actually supplied the data, and the mirrored fallback is the half that does the work — #3439 alone moves nothing on a host where every daily job is launchd: true, which is exactly the case on mine.
|
Correcting my own body, and flagging it here because you already approved. Nothing about the change itself moved — the diff, the tests and the mutation result are unchanged. What moved is a claim in the evidence paragraph. I wrote that The job fired and completed this morning, on a The measurement that caught it also points at a better lane, which I am deliberately keeping out of this PR. Every cron job leaves That is a bigger change than this one and rests on evidence I gathered after you approved, so it gets its own PR and its own review rather than being appended here. |
Coverage Gate✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 85%. Diff CoverageDiff: origin/fix/daily-punctuality-compact-dates...HEAD, staged and unstaged changes
Summary
|
|
Verified the combined 2→5 claim end-to-end rather than leaving it inferred. Both this PR and its parent #3439 applied, run against the live workspace: 9 → 6 unverifiable, so 2 → 5 observable. The three that cross over are One thing a reviewer will notice and should not be confused by: the "N of 11 observable" phrasing disappears from the detail line after the change, because there is now a named miss and the miss-report format takes over. The count did not stop being computed; it stopped being printed. That is pre-existing formatting, not something this PR does. Neither PR body carried this measurement — each shows only its own half — so posting it here where the stack is complete. |
qingyun-wu
left a comment
There was a problem hiding this comment.
Exact-head review at b533e80: no blocking findings. The two evidence lanes now fall back symmetrically, stem_declared follows the lane that actually supplied evidence, and the added empty-both-lanes control correctly remains UNCHECKED rather than manufacturing a miss. Both focused punctuality suites pass locally and diff checks are clean.
Reviewed by Qingyun's Personal Codex.
The sentinel lane needs a script that stamps one; the artifact lane needs a job that publishes something dated. A job doing neither is UNCHECKED forever, which is 6 of 11 on this host even after #3439 and #3440. But every cron job leaves `results/task-cron-<name>-<epoch>.txt` when its result is written, whatever else it publishes — the one completion record that needs no per-job config and no new writer. mtime is the finish, the same choice _daily_completion_minutes already makes for a sentinel with no body timestamp. Live before -> after on this host: before loop-engineering-digest late; unverifiable (6): agentloop-growth-daily, agentloop-site-metrics, learned-skills-scan, money-autopilot-scan, obsidian-dream, posthog-usage-daily after loop-engineering-digest: no output today, 1022 min past due money-autopilot-scan: no output today, 861 min past due (no unverifiable clause at all) 2 of 11 observable on main, 5 after the two parents, 11 with this. And it makes the live money-autopilot-scan outage visible: that job has not fired since 2026-08-24 and nothing reported it, because UNCHECKED reads as healthy. The glob is anchored on the epoch. `task-cron-{job}-*` also matches `task-cron-{job}-extra-...`, so a neighbouring job's records would have vouched for this one — my own control caught that before the first commit. Stacked on #3440 -> #3439. Merge order is 3439, 3440, then this. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…3443) * feat(health-check): task-cron records as the third punctuality lane The sentinel lane needs a script that stamps one; the artifact lane needs a job that publishes something dated. A job doing neither is UNCHECKED forever, which is 6 of 11 on this host even after #3439 and #3440. But every cron job leaves `results/task-cron-<name>-<epoch>.txt` when its result is written, whatever else it publishes — the one completion record that needs no per-job config and no new writer. mtime is the finish, the same choice _daily_completion_minutes already makes for a sentinel with no body timestamp. Live before -> after on this host: before loop-engineering-digest late; unverifiable (6): agentloop-growth-daily, agentloop-site-metrics, learned-skills-scan, money-autopilot-scan, obsidian-dream, posthog-usage-daily after loop-engineering-digest: no output today, 1022 min past due money-autopilot-scan: no output today, 861 min past due (no unverifiable clause at all) 2 of 11 observable on main, 5 after the two parents, 11 with this. And it makes the live money-autopilot-scan outage visible: that job has not fired since 2026-08-24 and nothing reported it, because UNCHECKED reads as healthy. The glob is anchored on the epoch. `task-cron-{job}-*` also matches `task-cron-{job}-extra-...`, so a neighbouring job's records would have vouched for this one — my own control caught that before the first commit. Stacked on #3440 -> #3439. Merge order is 3439, 3440, then this. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Bind the task-cron reader to the writer's slugging contract The writer slugifies a configured cron name before building `task-cron-<slug>-<stamp>`; the new punctuality lane globbed the RAW name, so every job whose name needs slugification wrote under one string and was read under another and stayed UNCHECKED forever. Interpolating the raw name into a glob also injected path separators and wildcards: `a/b` searched a subdirectory, `mo*ney` matched arbitrary neighbours. Measured on the pre-fix reader, one record present per job: 'Money scan' -> Money-scan found 0 'a/b' -> a-b found 0 '...' -> unnamed found 0 'money.autopilot.scan' -> money-autopilot-scan found 0 'money--scan' -> money-scan found 0 'money-autopilot-scan' -> money-autopilot-scan found 1 7 of 8 names missed. The duplication was the defect, so rather than copying the regex into the reader this adds `src/cron_task_id.py` as the single owner of the naming contract — slug, task id, discovery glob, and an end-anchored matcher — and points both `cron-runner` and `health-check` at it. Discovery now globs a literal `task-cron-*` and filters with the compiled matcher, so no job name ever reaches a glob. Controls: the reviewer's cases (`Money scan`, `a/b`, all-punctuation) plus `daily.report.job` and `mo*ney` are asserted end to end, configured under the raw name and recorded under the slug. The prefix-neighbour controls are kept and extended — `Money-scan-extra` and `Money-scanner` are both refused. Reverting the reader to the raw-name glob fails 5 of the new assertions, so they catch the regression rather than passing by construction. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Regenerate docs/src-map.md for the new naming-contract module `src/cron_task_id.py` is a new module, and the src-map staleness check refuses a diff that adds one without its index entry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(cron-task-id): require the stamp to end the slug `record_matcher` accepted any non-digit tail after the stamp, so a numeric neighbour still vouched for its prefix: record_matcher("ghost-job").match( "task-cron-ghost-job-2-1787000000000.txt") # True `ghost-job-2` is a separate configured job; `2` was read as the stamp and `-1787000000000.txt` as a suffix. The boundary now accepts `.` or `-` followed by a non-digit, so `-<digits>` marks the digits as the neighbour's slug tail. Kept deliberately wider than "dot or end": measured against 2050 live task-cron-* files, 8 carry a `-late-duplicate.txt` marker directly after the stamp and 1 carries `.no-task.<stamp>.txt`. Requiring a dot would have rejected records this matcher must still claim. Regression controls use the reviewer's exact filename, plus its -late-duplicate variant, plus a control that `ghost-job-2` still matches its own record. Restoring the old pattern fails both neighbour checks. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(cron-task-id): constrain the post-stamp grammar to real suffixes The `-<non-digit>` boundary was still too loose, as reviewed. A neighbour whose slug ends in digits-plus-text passed its digits off as this job's stamp: task_id("ghost-job-2-late", 1787000000000) -> task-cron-ghost-job-2-late-1787000000000 record_matcher("ghost-job").match(... + ".txt") # True Same for `ghost-job-2-text` and `ghost-job-42-backfill`. Consequence is the one this probe exists to prevent: a genuinely missing job reported punctual on an unrelated job's record. `RECORD_SUFFIX` now spells the three suffixes records actually carry, measured over 2056 live and archived records: 2047 `.txt`, 8 `-late-duplicate.txt`, 1 `.no-task.<stamp>.txt`. An unknown suffix fails CLOSED — the record is not counted, so the job reads unverifiable rather than falsely punctual. Regressions are built with `task_id()` rather than hand-typed names, so writer and matcher are exercised against each other, and each neighbour carries a control that it still claims its own record. Restoring the prior pattern fails exactly the three neighbour checks. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(health-check): the task-cron lane observes cron-runner jobs, not all crons Per @yixuan-ag2's review of #3443: the docstring claimed "every cron job leaves it". Only `cron-runner.py:380` writes that filename shape, so session crons registered via CronCreate never produce one and the lane cannot observe them. Their host measures 41 records, all from launchd:true slugs, while its three UNCHECKED jobs are session crons with 0 records each. Docs-only; the guard already fails closed on an unknown suffix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The asymmetry
check_daily_cron_punctualityalready falls back one way:A non-launchd job with no artifact reads its sentinel. A launchd job with no sentinel never looks at
results/at all — so a launchd job that publishes a dated artifact every single day reports "no dated artifact, cannot tell whether it ran" forever.The comment two lines below that routing already argues for the fix: "
launchdconflates 'runs under launchd' with 'publishes no dated results file'". It says how a job is scheduled; it does not determine what dated evidence the job leaves behind.Live before → after, and it is not a cosmetic delta
loop-engineering-digesthas artifacts through2026-08-25and none for today. It is due at 06:02; 976 minutes is ~16 hours. The probe was reporting "all on schedule" while that job's output had been missing all day — because the only lane that could see it was closed by a flag about scheduling.Correction to an earlier version of this paragraph, made before anyone reviewed it. I first wrote "a daily job silently dead for 16 hours". That over-claims what the evidence shows. Checking further:
results/archive/2026-08/task-cron-loop-engineering-digest-1787749355939.txthas mtime Aug 26 06:26, so the job did fire and complete this morning — it produced no dated artifact. Missing output is the real finding and is worth flagging; "the cron is dead" is a different claim my evidence does not support.That is the finding this PR exists for. The 2→5 coverage improvement is the smaller half.
Follow-up this measurement surfaced, deliberately not in this PR. Every cron job leaves a
results/task-cron-<name>-<epoch>.txtrecord whose mtime is its completion time. Measured across this host's daily jobs: 13 of 14 have one, most with 8 distinct days in the last 7, including all six that stay UNCHECKED after this PR. That is a universal per-job completion record neither lane reads, and it would take coverage from 2/11 to essentially 11/11 with no per-job config and no scripts writing sentinels. Separate change, separate evidence.What it does
Both directions fall back, so neither lane's absence is read as the job's silence.
stem_declarednow tracks which lane actually produced the evidence instead of assumingnot launchd, which keeps the missed-today branch correctly gated.Tests
Four cases added to
tests/daily-punctuality-completion-sentinels.test.py:Mutation-verified: removing the launchd arm of the fallback takes the suite from OK / 0 failures to 3 failures, and the control case correctly stays green through the mutation (it is testing the opposite property). Restoring returns 0 failures. The sibling
health-check-daily-punctualitysuite is unaffected: 46/46.The pre-existing fixture case "launchd but never stamps: must stay UNCHECKED" also still passes — it declares an artifact stem but writes no artifacts, so the fallback correctly finds nothing.
🤖 Generated with Claude Code