fix: ensure that queue input jobs wait if the queue is still empty upon job creation by johanneskoester · Pull Request #3866 · snakemake/snakemake

johanneskoester · 2025-12-06T17:04:11Z

fixes #3852

QC

The PR contains a test case for the changes or the changes are already covered by an existing test case.
The documentation (docs/) is updated to reflect the changes or this is not necessary (e.g. if the change does neither modify the language nor the behavior or functionalities of Snakemake).

Summary by CodeRabbit

Bug Fixes
- Relaxed scheduling readiness checks to correctly handle pending queued input jobs and avoid spurious assertion errors.
Refactor
- Simplified tracking of queue inputs for leaner, more reliable processing.
Tests
- Updated queue-input test timing to align with the revised scheduling behavior.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

job creation

coderabbitai · 2025-12-06T17:04:28Z

📝 Walkthrough

Walkthrough

Relaxes a DAG postprocess assertion to allow unfinished queue-input jobs, changes per-queue list storage to a set when collecting from-queue identifiers, and adds a 20-second initial sleep to the queue-input test to reproduce timing-related behavior.

Changes

Cohort / File(s)	Summary
DAG postprocess assertion `src/snakemake/dag.py`	Assertion broadened: the postprocess check now treats the presence of unfinished queue-input jobs as an allowed exception; otherwise it still verifies ready/needrun conditions before emitting the same bug message.
Queue input collection refactor `src/snakemake/jobs.py`	Replaced per-queue `defaultdict(list)` with a `set` of queue identifiers; iteration now uses the rule's input collection and extracts queue IDs via `get_flag_value`, preventing duplicate entries.
Queue input test timing `tests/test_queue_input/Snakefile`	Inserted an initial `time.sleep(20)` at the start of `update_results()` to shift when items are enqueued and reproduce the long-running update_results timing case.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Inspect src/snakemake/jobs.py to ensure semantics preserved when switching from lists to a set (duplicates, any ordering or indexing expectations).
Review src/snakemake/dag.py assertion logic to confirm it correctly models valid states and does not hide other failure modes.
Run and evaluate tests/test_queue_input/Snakefile for flakiness and acceptable CI impact due to the added 20s delay.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The pull request title clearly and specifically describes the primary change: ensuring queue input jobs wait when the queue is empty at job creation, which directly addresses the linked issue #3852.
Description check	✅ Passed	The pull request description references the linked issue (#3852) and includes both QC checklist items checked off, indicating test coverage and documentation status were considered.
Linked Issues check	✅ Passed	The changes in dag.py, jobs.py, and test file address the core issue: enabling queue-input jobs to wait properly when queues are initially empty by relaxing the job-ready assertion, modifying queue tracking, and adding timing delay in tests to reproduce the stall condition.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the queue input stalling issue: assertion logic in DAG processing, queue representation in job handling, and test case timing are all necessary for the fix.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch fix/queue_input_sleep

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 and usage tips.}

github-actions · 2025-12-06T17:10:21Z

Please format your code with pixi run format

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/snakemake/dag.py (1)
1941-1947: New assertion fully disables the “no-ready-jobs” bug check whenever any unfinished queue-input job exists

The updated assertion:
if check_initial:
    assert self.has_unfinished_queue_input_jobs() or (
        not any(self.needrun_jobs()) or any(self.ready_jobs)
    ), "bug: DAG contains jobs that have to be executed but no such job is ready for execution."
treats the mere presence of any unfinished queue-input job as sufficient to bypass the original invariant. In a mixed workflow (queue-input + normal jobs), this could allow a situation where there are non‑queue jobs that need to run, no jobs are ready, yet the assertion does not fire because at least one unrelated queue-input job is unfinished.

To keep the intended fix while preserving the original invariant for non‑queue‑blocked jobs, consider narrowing the exemption to jobs whose “needrun” status is entirely due to unfinished queue input. For example:
if check_initial:
    # Only enforce the invariant on jobs that are not blocked by unfinished queue input
    has_nonqueue_needrun = any(
        job
        for job in self.needrun_jobs()
        if not job.has_unfinished_queue_input()
    )
    assert (not has_nonqueue_needrun) or any(self.ready_jobs), (
        "bug: DAG contains jobs that have to be executed but no such job is "
        "ready for execution."
    )
This still allows the initial state where all pending work is queue‑input‑blocked (your bugfix case), but will continue to flag a DAG where non‑queue jobs need to run and none are ready.

If you’d like, I can also propose a small test variant that mixes queue and non‑queue jobs to exercise this edge case.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 91cc4af and d9e98ca.

📒 Files selected for processing (1)

src/snakemake/dag.py (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.py

⚙️ CodeRabbit configuration file

**/*.py: Do not try to improve formatting.
Do not suggest type annotations for functions that are defined inside of functions or methods.
Do not suggest type annotation of the self argument of methods.
Do not suggest type annotation of the cls argument of classmethods.
Do not suggest return type annotation if a function or method does not contain a return statement.

Files:

src/snakemake/dag.py

🧠 Learnings (2)

📚 Learning: 2024-10-14T09:42:11.571Z

Learnt from: johanneskoester
Repo: snakemake/snakemake PR: 3140
File: snakemake/dag.py:1308-1308
Timestamp: 2024-10-14T09:42:11.571Z
Learning: In `snakemake/dag.py`, avoid flagging missing lines or indentation issues when there is no clear syntax or logical error to prevent false positives.

Applied to files:

src/snakemake/dag.py

📚 Learning: 2024-11-12T12:08:20.342Z

Learnt from: johanneskoester
Repo: snakemake/snakemake PR: 3148
File: snakemake/dag.py:1332-1336
Timestamp: 2024-11-12T12:08:20.342Z
Learning: In `snakemake/dag.py`, when code is outdated and will disappear upon resolving merge conflicts, avoid making code review suggestions on that code.

Applied to files:

src/snakemake/dag.py

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: apidocs

🤖 I have created a release *beep* *boop* --- ## [9.14.3](v9.14.2...v9.14.3) (2025-12-08) ### Bug Fixes * ensure that queue input jobs wait if the queue is still empty upon job creation ([#3866](#3866)) ([555ab6a](555ab6a)) * fix issues with cyclic dependencies when using the update and before_update flag ([#3857](#3857)) ([21cc94e](21cc94e)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

…on job creation (snakemake#3866) fixes snakemake#3852 ### QC  * [x] The PR contains a test case for the changes or the changes are already covered by an existing test case. * [x] The documentation (`docs/`) is updated to reflect the changes or this is not necessary (e.g. if the change does neither modify the language nor the behavior or functionalities of Snakemake).  ## Summary by CodeRabbit * **Bug Fixes** * Relaxed scheduling readiness checks to correctly handle pending queued input jobs and avoid spurious assertion errors. * **Refactor** * Simplified tracking of queue inputs for leaner, more reliable processing. * **Tests** * Updated queue-input test timing to align with the revised scheduling behavior. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub>

🤖 I have created a release *beep* *boop* --- ## [9.14.3](snakemake/snakemake@v9.14.2...v9.14.3) (2025-12-08) ### Bug Fixes * ensure that queue input jobs wait if the queue is still empty upon job creation ([snakemake#3866](snakemake#3866)) ([555ab6a](snakemake@555ab6a)) * fix issues with cyclic dependencies when using the update and before_update flag ([snakemake#3857](snakemake#3857)) ([21cc94e](snakemake@21cc94e)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

johanneskoester added 2 commits December 6, 2025 18:02

fix: ensure that queue input jobs wait if the queue is still empty upon

7ac8417

job creation

fmt

b37cc4c

fix check

91cc4af

fmt

d9e98ca

coderabbitai Bot reviewed Dec 6, 2025

View reviewed changes

johanneskoester merged commit 555ab6a into main Dec 7, 2025
60 checks passed

johanneskoester deleted the fix/queue_input_sleep branch December 7, 2025 14:49

snakemake-bot mentioned this pull request Dec 7, 2025

chore(main): release 9.14.3 #3868

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: ensure that queue input jobs wait if the queue is still empty upon job creation#3866

fix: ensure that queue input jobs wait if the queue is still empty upon job creation#3866
johanneskoester merged 4 commits intomainfrom
fix/queue_input_sleep

johanneskoester commented Dec 6, 2025 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Dec 6, 2025 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Uh oh!

github-actions Bot commented Dec 6, 2025

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

johanneskoester commented Dec 6, 2025 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

QC

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Dec 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Pre-merge checks and finishing touches

Uh oh!

github-actions Bot commented Dec 6, 2025

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

johanneskoester commented Dec 6, 2025 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Dec 6, 2025 •

edited

Loading