Skip to content

fix: create runs_dir only on write paths, not in RunManager.__init__#48

Merged
ultimatile merged 1 commit into
mainfrom
fix/44-lazy-runs-dir-mkdir
Jul 10, 2026
Merged

fix: create runs_dir only on write paths, not in RunManager.__init__#48
ultimatile merged 1 commit into
mainfrom
fix/44-lazy-runs-dir-mkdir

Conversation

@ultimatile

@ultimatile ultimatile commented Jul 10, 2026

Copy link
Copy Markdown
Owner

Summary

Metadata-lookup commands (status, list, job-output, wait) invoked against the wrong project root used to create an empty .crewster/runs tree there before printing Run not found, because RunManager.__init__ unconditionally ran mkdir(parents=True, exist_ok=True). Directory creation now happens only on write paths, and failed lookups leave the filesystem untouched. Closes #44

Changes

  • src/crewster/run.py: removed the mkdir from RunManager.__init__; create_run and save_run_meta already create their run directory with parents=True, which covers runs_dir. list_runs treats a missing runs_dir as "no runs" — EAFP around iterdir, so a runs_dir that exists as a regular file still fails loudly.
  • src/crewster/cli.py: new _print_run_not_found helper used by job-output and wait on a failed lookup. When no runs are recorded under the searched root, it names that directory and notes, conditionally, that a run submitted from a different project root must be looked up with the same --project-dir/--config as at submit time. The hint gates on list_runs (the manager's definition of a recorded run), so stray entries such as .DS_Store or a metadata-less directory cannot suppress it.

Impact

  • submit is unaffected (its write path creates the directory).
  • status / list / job-output / wait lose the mkdir side effect; load_run_meta still raises FileNotFoundError for a missing tree, which the id-lookup commands (status, job-output, wait) already catch, and crewster list in a fresh project still prints No runs found.

Test plan

Invariant: a failed metadata lookup never creates .crewster/runs.

  • tests/test_run.py: __init__ creates nothing; list_runs / find_run_by_job_id / load_run_meta on a missing runs_dir return [] / None / raise without touching the filesystem; create_run and save_run_meta create the tree when missing.
  • tests/test_cli.py: wait and status with an unknown id and list in a fresh project leave .crewster uncreated; the hint fires on an empty metadata tree, survives stray non-run entries, and is suppressed when runs are recorded.
  • pytest: 379 passed; ruff check / ruff format / pyright clean.

Notes

  • status still queries the scheduler when an id matches no local run metadata, without reporting the metadata miss; its Run not found branch is unreachable for a non-empty argument. Tracked in status: Run not found branch is unreachable after the job-id fallback #47.
  • On a failed lookup the hint re-lists runs after find_run_by_job_id already did. The double scan on this error path is deliberate: it keeps the emptiness check inside list_runs instead of threading the run list from the lookup into the printing helper.

Summary by CodeRabbit

  • Bug Fixes
    • Improved “Run not found” messages with guidance when no run history is available.
    • Read-only commands no longer create an empty .crewster directory.
    • list, status, and wait now handle projects without recorded runs more cleanly.
    • Preserved clear errors for missing runs and jobs while avoiding unnecessary filesystem changes.

…it__

Metadata lookups (status/list/job-output/wait) invoked against the
wrong project root used to leave an empty .crewster/runs tree behind
before printing "Run not found". RunManager now defers directory
creation to create_run/save_run_meta, and list_runs treats a missing
runs_dir as "no runs" (EAFP, so a runs_dir that exists as a regular
file still fails loudly).

When a lookup fails with no runs recorded under the searched root,
job-output and wait now hint that run metadata lives under the project
root used at submit time; the hint gates on list_runs so stray entries
cannot suppress it, and it phrases the remedy conditionally to avoid
misdirecting fresh-project typos.

Closes #44
@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 83d9e0af-a422-418d-a264-95fd39fc7cf5

📥 Commits

Reviewing files that changed from the base of the PR and between 243eacb and 6cad027.

📒 Files selected for processing (4)
  • src/crewster/cli.py
  • src/crewster/run.py
  • tests/test_cli.py
  • tests/test_run.py

📝 Walkthrough

Walkthrough

Run metadata lookup paths no longer create .crewster/runs directories. Missing-run CLI responses now include a project-root hint when no metadata is recorded, with coverage for read and write lifecycle behavior.

Changes

Run lookup behavior

Layer / File(s) Summary
Lazy runs directory lifecycle
src/crewster/run.py, tests/test_run.py
RunManager defers directory creation, returns no runs when storage is absent, and preserves directory creation for write operations.
CLI missing-run feedback
src/crewster/cli.py, tests/test_cli.py
job-output and wait use shared missing-run messaging, while CLI tests verify hints and filesystem behavior across lookup commands.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Poem

I’m a bunny with a tidy run,
No empty folders in the sun.
Missing paths now softly say,
“Try the project used that day.”
Metadata blooms when writes begin!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: deferring runs_dir creation to write paths instead of RunManager initialization.
Linked Issues check ✅ Passed The PR addresses #44 by making runs_dir lazy, treating missing run directories as empty, and adding lookup hints/tests for metadata commands.
Out of Scope Changes check ✅ Passed The changes stay focused on lazy runs_dir creation, lookup behavior, and related tests; no unrelated scope is evident.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/44-lazy-runs-dir-mkdir

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@ultimatile
ultimatile merged commit 44534aa into main Jul 10, 2026
1 check passed
@ultimatile
ultimatile deleted the fix/44-lazy-runs-dir-mkdir branch July 10, 2026 08:33
ultimatile added a commit that referenced this pull request Jul 10, 2026
)

An empty list_runs result means "no runs recorded", so only a missing
runs_dir may read as empty. The docstring already claims that a
runs_dir existing as a regular file fails loudly, but no test pinned
it: widening the except clause (e.g. to OSError) would silently turn
that corrupted state into an empty run list. The contract test raises
NotADirectoryError on a runs_dir-as-file and fails on the pre-#48
is_dir() guard, which returned [] for the same state.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

run: metadata lookups create an empty .crewster/runs under the wrong project root

1 participant