Skip to content

fix: show pending jobs before first completion#64

Open
yfarjoun wants to merge 2 commits intofg-labs:mainfrom
yfarjoun:yf_fix-pending-before-first-completion
Open

fix: show pending jobs before first completion#64
yfarjoun wants to merge 2 commits intofg-labs:mainfrom
yfarjoun:yf_fix-pending-before-first-completion

Conversation

@yfarjoun
Copy link
Copy Markdown
Contributor

@yfarjoun yfarjoun commented Apr 13, 2026

Summary

  • Infer total_jobs from the Job stats table (expected_job_counts) when the Snakemake progress line hasn't appeared yet (it only appears after the first completion)
  • This lets the pending panel show correct counts and per-rule breakdown immediately, updating as jobs start running
  • Falls back gracefully if the Job stats table isn't available yet

Closes #(none — discovered during usage)

Test plan

  • 4 new tests in TestPendingJobsBeforeCompletion
  • total_jobs inferred from expected counts when progress line absent
  • Pending count accounts for running jobs before first completion
  • No inference without estimator
  • Progress line value takes precedence when available
  • Full test suite passes (1086 tests)
  • Manual: start snakesee on a fresh workflow, verify pending panel populates immediately

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Pending job panel now displays inferred job counts earlier during workflow execution, improving visibility before initial progress lines appear.
  • Tests

    • Added test coverage for pending job panel behavior when progress data is not yet available.

yfarjoun and others added 2 commits April 13, 2026 16:14
The pending panel showed "No pending jobs" until the first job completed
because total_jobs was derived from the progress line ("X of Y steps
done"), which Snakemake only emits on completion.

Now infers total_jobs from the Job stats table (already parsed into
expected_job_counts) when the progress line hasn't appeared yet. This
lets the pending panel show correct counts and per-rule breakdown
immediately, updating as jobs start running.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@yfarjoun yfarjoun requested a review from nh13 as a code owner April 13, 2026 20:40
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 13, 2026

📝 Walkthrough

Walkthrough

The changes modify _poll_state() in the TUI monitor to infer total_jobs from a TimeEstimator when initial progress parsing shows zero jobs, enabling the UI to display accurate pending job counts earlier. New tests verify this behavior.

Changes

Cohort / File(s) Summary
Job Count Inference Logic
snakesee/tui/monitor.py
Modified _poll_state() to infer total_jobs from TimeEstimator.expected_job_counts when progress shows total_jobs == 0, with optional estimator rule initialization. Updates parsed progress object with inferred total.
Test Coverage
tests/test_tui.py
Added TestPendingJobsBeforeCompletion test class verifying: inferred total_jobs calculation from expected counts, pending job computation, behavior when estimator is absent, and preservation of existing progress values.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Suggested reviewers

  • nh13

Poem

🐰 ✨ A sniffly nose for jobs we count,
Before the first line starts to mount,
The estimator whispers true—
"Here's the work that waits for you!"
Now pending panels know their sum,
Before the progress lines have come! 🎯

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: enabling the pending jobs panel to display before the first workflow completion.
Description check ✅ Passed The description covers the key changes, testing strategy, and includes the required Type of Change section (bug fix) and testing details; most template sections are addressed.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@snakesee/tui/monitor.py`:
- Around line 3099-3106: The current code unconditionally uses
self._estimator.expected_job_counts to infer progress.total_jobs and can apply
stale counts from a previous log; update the guard so you only use
expected_job_counts when they were populated for the same log/context as the
current progress. In practice modify the logic around
_init_current_rules_from_log and the inference block to (a) ensure
_init_current_rules_from_log is called and succeeds for the current progress/log
before using expected_job_counts, or (b) add a lightweight context check (e.g.
compare a current_log_id/timestamp field on self._estimator or progress) and
reject/clear expected_job_counts if they don’t match; only then compute
inferred_total and replace(progress, total_jobs=inferred_total). Ensure you
reference and update self._estimator.expected_job_counts,
_init_current_rules_from_log, and progress.total_jobs in the change.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f5863a7e-f7e1-4772-9e33-c0148beb70e1

📥 Commits

Reviewing files that changed from the base of the PR and between cb94991 and 9013a7d.

📒 Files selected for processing (2)
  • snakesee/tui/monitor.py
  • tests/test_tui.py

Comment on lines +3099 to +3106
if not self._estimator.expected_job_counts:
self._init_current_rules_from_log()
if self._estimator.expected_job_counts:
from dataclasses import replace

inferred_total = sum(self._estimator.expected_job_counts.values())
if inferred_total > 0:
progress = replace(progress, total_jobs=inferred_total)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Guard total_jobs inference against stale expected_job_counts.

At Line [3099], inference uses any existing non-empty self._estimator.expected_job_counts without log scoping. That can apply stale/latest-run counts at Line [3104] and overwrite progress.total_jobs for the wrong run (especially in historical view).

💡 Suggested fix
diff --git a/snakesee/tui/monitor.py b/snakesee/tui/monitor.py
@@
-        if progress.total_jobs == 0 and self._estimator is not None:
+        if (
+            progress.total_jobs == 0
+            and self._estimator is not None
+            and self._current_log_index == 0  # infer only for latest/live run
+        ):
             if not self._estimator.expected_job_counts:
                 self._init_current_rules_from_log()
             if self._estimator.expected_job_counts:
                 from dataclasses import replace
@@
                 if inferred_total > 0:
                     progress = replace(progress, total_jobs=inferred_total)
diff --git a/snakesee/tui/monitor.py b/snakesee/tui/monitor.py
@@
-        job_counts = parse_job_stats_counts_from_log(log_path)
-        if job_counts:
-            self._estimator.expected_job_counts = job_counts
+        job_counts = parse_job_stats_counts_from_log(log_path)
+        # Always replace to avoid carrying stale counts across runs/log switches
+        self._estimator.expected_job_counts = job_counts or None
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@snakesee/tui/monitor.py` around lines 3099 - 3106, The current code
unconditionally uses self._estimator.expected_job_counts to infer
progress.total_jobs and can apply stale counts from a previous log; update the
guard so you only use expected_job_counts when they were populated for the same
log/context as the current progress. In practice modify the logic around
_init_current_rules_from_log and the inference block to (a) ensure
_init_current_rules_from_log is called and succeeds for the current progress/log
before using expected_job_counts, or (b) add a lightweight context check (e.g.
compare a current_log_id/timestamp field on self._estimator or progress) and
reject/clear expected_job_counts if they don’t match; only then compute
inferred_total and replace(progress, total_jobs=inferred_total). Ensure you
reference and update self._estimator.expected_job_counts,
_init_current_rules_from_log, and progress.total_jobs in the change.

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.

1 participant