Add persistent run store with agent API and redesigned Runs panel - #16
Open
abrohamLee wants to merge 34 commits into
Open
Add persistent run store with agent API and redesigned Runs panel#16abrohamLee wants to merge 34 commits into
abrohamLee wants to merge 34 commits into
Conversation
added 30 commits
August 15, 2026 16:59
Codex review dispositions: run_id-scoped abort via per-trainer FIFO (no cross-abort, no head-of-line blocking); load_series() authority rule shared by rebuild/reconcile/API; startup wiring moved into _lifespan; SERIES_COLORS export noted; run_registered parser branch; validate with resume/policy; unreadable rows listable+deletable; bulk delete collects before deleting; removed reference to nonexistent contract test; venv/torch prerequisite.
…grouping Wraps the non-CRL branch of iter_sweep_events with RunWriter/capture_events so each sweep combo persists as its own run (origin="sweep", group_id=<sweep_session_id>). The wrapped iterator is closed explicitly after the consumption loop (guarded by hasattr) so finalization on early break (complete/aborted/paused) is deterministic rather than GC-dependent; the CRL branch stays unwrapped but gets the same close() guard for consistency. Applied the identical wrapping to train_coordinate_descent.py: both the baseline evaluation (_run_train_once) and the per-candidate evaluation inside the round/axis loop now persist inner runs under the coordinate- descent session id, since both consume iter_trainer_events_from_context the same way as the sweep path. Also fixes a pre-existing bug in validate_sweep_request that stringified the NodeKind enum before calling has_capability (str(NodeKind.trainer) == "NodeKind.trainer", never matching), which made POST /api/train/sweep 400 on every request. Every other call site in the codebase passes the enum directly; this brings train_sweep.py in line and was required for the new test (and the endpoint itself) to work at all.
…d, abort semantics)
- Register reference/runs-api.md in the docs governance page/doc-type map (tests/test_docs_source_contracts.py), mirroring training-api.md. - Remove all em/en dashes from runs-api.md, data-contracts.md, and training-api.md, rewritten with commas/colons/parentheses. - While editing runs-api.md: document that CRL trainer runs are never captured into the run store, add a 'Known limitations' section for the trainer-control collision, and fix the /metrics and /abort route rows to note that unreadable runs also 404 there (not just missing ones). - Generate docs/locales/zh_CN/LC_MESSAGES/reference/runs-api.po via the project's gettext workflow (sphinx -b gettext + sphinx-intl update) and fully translate it. - Catch up the zh_CN catalogs that had accumulated untranslated/fuzzy entries across several pages; this debt was previously masked by the runs-api.po catalog-set mismatch short-circuiting the pending-entry check in test_docs_i18n.py.
- comfy_research/main.py: on startup, repair a missing/corrupt/undercounted
data/runs/index.db before serving traffic (run_index.repair_index_if_needed),
and mark every running/queued row crashed regardless of heartbeat age
(run_index.reconcile_all_active_on_startup) instead of the previous
reconcile_stale_running() default-timeout call, which never reconciled a
run whose heartbeat was still fresh at crash time (the only call site).
- run_index.py: add repair_index_if_needed() (rebuild on sqlite3.DatabaseError
or on a row count under the on-disk run-*/run.json count) and
reconcile_all_active_on_startup(); factor the shared crash-marking loop
into _crash_run_ids().
- scripts/rebuild_run_index.py: thin CLI wrapper around rebuild_index(), for
forcing a rebuild outside the startup path.
- run_gc.py: apply the same retention cap/grace/terminal rules to
origin=sweep via a new max_runs_sweep config key (default 2000), looping
over [(agent, max_runs_agent), (sweep, max_runs_sweep)] instead of only
handling agent-origin runs; sweeps are the highest-volume producer.
- schemas/run_record.py: cache _declared_field_keys as a module-level
{type: [field keys]} dict built once from the 233KB node manifest, so
build_run_record is O(nodes) instead of O(nodes x manifest-parse).
- run_worker.py: evict a run's RunWriter and TrainRequest from pool state
once it's terminal (in _execute's finally, and when an abort finalizes a
still-waiting run), since TrainRequest can carry a full resume blob and
neither dict was ever bounded before; the idempotency-hit path now falls
back to run_store.read_run_record for an evicted run instead of a
KeyError, and the idempotency map is capped at 4096 entries (oldest
evicted first).
- api/runs.py: validate run_id up front and catch a malformed GET /api/runs
cursor, returning structured 400 invalid_run_id / invalid_cursor instead
of letting run_store.run_dir's or query_runs' ValueError surface as an
unstructured 500.
- run_store.py: log a warning (with run_id and dropped-line count) when
read_metric_rows silently drops a corrupt trailing line, instead of
dropping it silently.
- api/train.py: log once per stream (not per line, via a local flag) when
the remote-train NDJSON capture hits an unparseable line, instead of a
bare except: pass.
…ture - test_run_index.py: reconcile_all_active_on_startup ignores heartbeat age (the gap reconcile_stale_running()'s default timeout leaves open), and repair_index_if_needed rebuilds on a deleted or corrupt index.db and is a no-op when the index is already current. - test_run_gc.py: sweep-origin runs get the same cap/grace/terminal pruning as agent-origin, and are a no-op under the default cap. - test_runs_api.py: malformed run_id path params (including a path-traversal-shaped one) and a malformed GET /api/runs cursor return structured 400, never an unstructured 500. - test_coordinate_descent_run_capture.py: drives POST /api/train/coordinate-descent (analogous to test_sweep_run_capture.py) with an isolated COMFYRESEARCH_RUNS_DIR, asserting the baseline and per-candidate inner runs persist with origin=sweep and the session's group_id, and the wrapper's NDJSON events still complete. - test_repro_template_manual_rebuild.py: isolate test_post_train_accepts_rebuilt_fig1_graph_body's POST /api/train with COMFYRESEARCH_RUNS_DIR instead of writing into the real data/runs/.
added 4 commits
August 17, 2026 01:02
…ix header/scroll chrome - runFormat.ts: add disambiguateTitles(rows) -> Map<runId, displayTitle>; rows sharing a base title (e.g. all runs of a sweep) get a ' · <shortId>' suffix applied consistently to the visible title, checkbox/button aria-labels, and the compare legend label, so duplicate sweep titles no longer collapse into indistinguishable accessible names. - index.css: add a visible focus ring for the hidden row-select checkbox via .cr-runs-panel__row:has(...input:focus-visible); fall back --cr-surface-2 to --cr-surface-1 on the two new call sites (icon-btn hover, selected-row background) since --cr-surface-2 resolves to initial in the classic theme. - RunsPanel.tsx: wrap the groups/empty-state/compare section in cr-nodes-panel__scroll to match NodesLibraryPanel/SavedGraphLibraryPanel; promote the header title to <h2 class=cr-nodes-panel__title> and add a scoped cr-runs-panel__header rule so the refresh button right-aligns without touching the shared header rule; add aria-expanded to the group toggle; gate the empty state on a loaded flag so it no longer flashes before the first fetch resolves. - runCompareOverlay.ts: guard step_ticks/loss_history/test_loss_history reads with Array.isArray so a wrong-type metrics payload degrades to an empty series instead of throwing during render. - tests: unit tests for disambiguateTitles; RunsPanel tests for the h2/scroll markup and for disambiguated aria-labels/legend entries on a duplicate-title sweep; runCompareOverlay test for the malformed-payload guard.
… abort traversal - run_index.py: wrap the trainer_title ALTER TABLE in try/except sqlite3.OperationalError so two processes racing to open a fresh index for the first time don't crash if both see the column missing and both attempt to add it. - test_runs_api.py: the backslash-form POST abort traversal case now accepts (400, 404), checking the invalid_run_id code only when the status is 400, matching how the percent-encoded abort path is already handled for httpx normalization variance.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a server-side run store: every training run is now persisted with a queryable
run_id, plus an agent-first async API and a redesigned Runs panel in the canvas UI.Why
Training results previously lived only in the browser: the NDJSON stream was consumed client-side and stashed back into node data (the source of the multi-MB base64 checkpoints inside graph-library JSON). Results were lost on disconnect, there was no run history, no cross-run comparison, and no way for automation to submit a run and fetch results later.
What
Storage (
data/runs/{run_id}/, files are the source of truth)run.json(metadata + config-only graph snapshot),metrics.ndjson(append-only per-step deltas),results.json(terminal snapshot); atomic writes; checkpoints/PNGs/embedding blobs are never persistedscripts/rebuild_run_index.py)Capture (engine-level, zero frontend changes required)
/api/trainlocal + remote SSH streams, each sweep and coordinate-descent inner run (ownrun_id,group_id= session), async-submitted runsrun_registeredfirst event; disconnect finalizes partial results asabortedAgent API (
/api/runs)POST /api/runs: 202 +run_id, detached execution on a worker pool (per-trainer FIFO, no head-of-line blocking), single-flightIdempotency-Key{"code", "detail"}errorsRuns panel (canvas left rail)
8 runs · best 3.2e-3), full-row selection with accessible checkbox semantics and keyboard focus ringDocs:
reference/runs-api.md(en + zh), data-contracts and training-api updates.Incidental fixes
POST /api/train/sweep400'd on every request under Python 3.11 (str(NodeKind.x)enum stringification); fixed, with a remaining same-class instance noted incrl_run.py:293for follow-updata/runs/gitignored; train API tests isolated from the real storeTesting
test_information_bottleneck_reproduction::test_single_logit_full_batch...(pre-existing numerical drift under torch 2.12, unrelated)Known limitations (documented in runs-api.md)
trainer_node_id; avoid running the same graph concurrently via bothPOST /api/runsand streaming/api/train