Repository navigation
Derive system monitor progress from resolved ActivitySim run plans - #24
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Fix robust log replacement detection and support colon-formatted durations before approval.
Review effort: Lite
Findings: None
What changed in this PR
Updates ActivitySim system monitoring to derive progress from resolved run plans and execution logs instead of hard-coded model lists.
Changes:
- Adds validated run-plan parsing, resume handling, and generic worker tracking.
- Supports incremental log replay, late attachment, truncation, failures, and single-process runs.
- Adds CLI support, fixtures, tests, and monitoring documentation.
Two moderate issues remain in log replacement detection and parsing durations over 60 seconds.
| File | Description |
|---|---|
tests/test_sys_monitor.py |
Adds comprehensive monitor behavior tests. |
tests/fixtures/sys_monitor/staggered.log.txt |
Provides staggered-worker log data. |
tests/fixtures/sys_monitor/single_process.txt |
Provides a single-process plan fixture. |
tests/fixtures/sys_monitor/single_process.log.txt |
Provides single-process log data. |
tests/fixtures/sys_monitor/run_list.txt |
Provides a multiprocess run-plan fixture. |
tests/fixtures/sys_monitor/resumed.log.txt |
Provides resumed execution log data. |
tests/fixtures/sys_monitor/resume_named.txt |
Provides a named-checkpoint resume fixture. |
tests/fixtures/sys_monitor/resume_latest.txt |
Provides a last-checkpoint resume fixture. |
src/lighthouse/sys_monitor.py |
Implements plan parsing, tracking, log reading, and CLI updates. |
README.md |
Links to monitoring documentation. |
docs/sys-monitor.md |
Documents usage, statuses, resumes, and limitations. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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
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.
Adding a model component or renaming a multiprocess phase currently requires editing
sys_monitor.pyas well as the model configuration. Otherwise the monitor can miss steps or stall. This change discovers the ordered models, phases, and expected worker counts from ActivitySim's resolvedrun_list.txt, then tracks execution using the main log.Changes
--run-list.--tail-bytesnow controls read chunk size instead of discarding earlier history. Distinguish unknown/waiting/resuming/running/finalizing/finished/failed states; parent exit alone never implies success.Validation
git diff --checkand CLI help smoke check passed.Limits
run_list.txtis an informational text format, so parsing is isolated and rejects ambiguous plans. Files must belong to the same invocation; the current artifacts lack a shared run identifier. Some single-process entry points do not emit a run list, so tracking requires an explicitly supplied resolved plan in those cases. Unsupported/missing artifacts leave progress unknown while resource sampling continues. Identical repeated model names are unsupported.The proposed stable replacement is tracked in ActivitySim/activitysim#1120. This PR does not assume a future manifest schema or change ActivitySim.
Related: #21 (comment). This is a separate implementation PR based on upstream
main; it does not include agent-instruction changes.