Repository navigation
feat(run): record CPU and peak RSS; match scoring ids by text - #165
Conversation
`digest_runs` held only `wall_seconds`, so the hosted-pool estimate that a run mostly waits on the LLM was never measured. `run_digest` now calls an optional `Deps.process_usage` as the run ends and merges `cpu_seconds` (user + system) and `peak_rss_bytes` into the summary, which D1RunLog already stores whole: no new table or column. The read is `adapters/process_usage.py`, wired in `build_deps`. `ru_maxrss` is bytes on macOS and KiB on Linux; it is normalized to bytes. None leaves the summary unchanged, and a failing read is logged and never replaces the run's result. `resource` does not exist on Windows, and bootstrap imports the adapter, so `import cyris.bootstrap` would have raised there. The adapter tolerates its absence, and `build_deps` wires `process_usage=None` when `resource` is unavailable, which leaves the run's summary unchanged. The None case is tested to log no error: without the None guard in run_digest the call raised, was caught, and logged an error while the summary still lacked the keys, so a test asserting only the summary would stay green. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The scoring prompt prints `original_id` bare, so an int and a string look the same to the model, but the reply lookup was exact on type and whitespace: an int 7 never matched an echoed "7", nor a string "7" an echoed 7 or " 7 ". Ids now compare as stripped text, for the score and for the fallback language. A whole-number float reply id such as 7.0 keys as its integer text, so it still matches a stored int 7 as it did when ids compared with ==. The "Scored none of N articles" warning now also names the type of the ids sent, the first five sent and the first five received (ids only, each clipped, never article text), so an occurrence the match fix does not explain names its own cause. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
 |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| raise | ||
| finally: | ||
| summary["wall_seconds"] = round(time.monotonic() - started, 2) | ||
| if deps.process_usage is not None: |
There was a problem hiding this comment.
RUSAGE_SELF is cumulative for the whole process, but wall_seconds next to it is per run. If run_digest can run more than once in one process (a long-lived container serving regenerate reruns or scheduled runs), cpu_seconds and peak_rss_bytes include earlier work. ru_maxrss also never goes down. Those rows would overstate what the run used, and nothing in the stored row says so. Either record a baseline at the start of run_digest and store the CPU delta, or document that the figures are process lifetime and only meaningful for one-shot cyris run. The wall_seconds comparison invites reading them as per-run.
|
Review of #165: I found one thing worth raising, posted inline. |
|
@codex review |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Summary
digest_runs.summarynow recordscpu_secondsandpeak_rss_bytesfor every run, on SQLite and D1 alike, through an optionalDeps.process_usage(no schema change).ru_maxrssis normalized to bytes; a missingresourcemodule (Windows) wiresNoneand leaves the summary unchanged."7"matches a stored7; a whole-number float such as7.0still matches. The "Scored none of N" warning names the id types and the first ids sent and received, so a zero match that this fix does not explain names its own cause.Tickets: run-resource-metrics, scoring-ids-match-none (vault pm/cyris/tasks).
Verification
scripts/check.shpassed outside the sandbox at both commits (fe68ffd: 5329 passed; the tip's tree equals the reviewed 1aab628c, which passed).digest_runsrow read back with both values, and a real run loggingScored N / N.🤖 Generated with Claude Code