Skip to content

Add root flag to verdi process list#7270

Open
r0hansaxena wants to merge 6 commits into
aiidateam:mainfrom
r0hansaxena:issue-7170-process-list-root
Open

Add root flag to verdi process list#7270
r0hansaxena wants to merge 6 commits into
aiidateam:mainfrom
r0hansaxena:issue-7170-process-list-root

Conversation

@r0hansaxena
Copy link
Copy Markdown

Summary

Adds a --root flag to verdi process list to filter for root processes — processes that have no caller and were submitted directly by the user rather than spawned by another process.

Closes #7170

Motivation

When working with large workflow histories, it can be difficult to identify which processes are top-level entries vs. sub-processes spawned inside a workflow. This flag makes it easy to list only the "entry points".

Changes

  • src/aiida/cmdline/params/options/main.py: Added a new ROOT overridable CLI option.
  • src/aiida/cmdline/commands/cmd_process.py: Integrated --root into the process_list command.
  • src/aiida/tools/query/calculation.py: Updated CalculationQueryBuilder to identify parentless processes using an efficient outer join on incoming CALL_CALC and CALL_WORK links, filtering for nodes where no caller exists.
  • tests/cmdline/commands/test_process_list_root.py: Added tests covering simple parent-child hierarchies, deeply nested chains, and interaction with --all.

Usage

# List only root (top-level) active processes
verdi process list --root

# List all root processes including finished ones
verdi process list --root --all

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 8, 2026

Codecov Report

❌ Patch coverage is 13.51351% with 32 lines in your changes missing coverage. Please review.
✅ Project coverage is 29.97%. Comparing base (b8b77cc) to head (b1d0de1).

Files with missing lines Patch % Lines
src/aiida/orm/querybuilder.py 12.00% 22 Missing ⚠️
src/aiida/tools/query/calculation.py 0.00% 8 Missing ⚠️
src/aiida/cmdline/commands/cmd_process.py 50.00% 1 Missing ⚠️
.../aiida/storage/psql_dos/orm/querybuilder/joiner.py 0.00% 1 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (b8b77cc) and HEAD (b1d0de1). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (b8b77cc) HEAD (b1d0de1)
2 1
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #7270       +/-   ##
===========================================
- Coverage   79.75%   29.97%   -49.78%     
===========================================
  Files         566      566               
  Lines       43897    43871       -26     
===========================================
- Hits        35006    13144    -21862     
- Misses       8891    30727    +21836     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@edan-bainglass
Copy link
Copy Markdown
Member

edan-bainglass commented Mar 8, 2026

Thanks @r0hansaxena. Looks good so far 💪

One note regarding your tests - we currently consolidate all verdi process testing under cmdline/commands/test_process.py. Specifically for verdi process list, have a look here - tests/cmdline/commands/test_process.py#L328. I'd ask you to please integrate your tests into this location 🙏 It will help with maintenance down the road.

Update

Your tests failed (also the pre-commit) 🥲 Please try to resolve this. You can of course ping me if you need assistance 🙂

Comment thread tests/cmdline/commands/test_process.py Outdated
Comment thread tests/cmdline/commands/test_process.py Outdated
Copy link
Copy Markdown
Member

@edan-bainglass edan-bainglass left a comment

Choose a reason for hiding this comment

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

Thanks @r0hansaxena. Left a couple of comments. Also, are you running tests locally to make sure everything passes on your end?

@danielhollas
Copy link
Copy Markdown
Collaborator

Heya, just a quick question, should we bikeshed the name of the option a bit? Is --root descriptive enough? Should it perhaps be --only-roots? Or something else?

@r0hansaxena
Copy link
Copy Markdown
Author

Thanks @r0hansaxena. Left a couple of comments. Also, are you running tests locally to make sure everything passes on your end?

thanks! the core listing and sql tests are passing fine on my end.

@r0hansaxena
Copy link
Copy Markdown
Author

I have also fixed the linting issues, I'll make changes if necessary after the checks are done running once again accordingly.

@r0hansaxena
Copy link
Copy Markdown
Author

r0hansaxena commented Mar 9, 2026

> Heya, just a quick question, should we bikeshed the name of the option a bit? Is --root descriptive enough? Should it perhaps be --only-roots? Or something else?

I do agree, --only-roots is more descriptive, I have made changes for the same :)

@r0hansaxena r0hansaxena force-pushed the issue-7170-process-list-root branch 2 times, most recently from 001effd to b1d0de1 Compare March 9, 2026 17:42
@r0hansaxena
Copy link
Copy Markdown
Author

the ci checks are failing, I'll address the issues and make the necessary changes

@r0hansaxena r0hansaxena force-pushed the issue-7170-process-list-root branch 2 times, most recently from f0bd0b1 to 51cea1c Compare March 9, 2026 21:13
@r0hansaxena r0hansaxena force-pushed the issue-7170-process-list-root branch from 51cea1c to 1cad40c Compare March 9, 2026 21:18
Add edge_filter_dict parameter to recursive join method.
@edan-bainglass
Copy link
Copy Markdown
Member

Heya, just a quick question, should we bikeshed the name of the option a bit? Is --root descriptive enough? Should it perhaps be --only-roots? Or something else?

Thanks @danielhollas. Though I agree that --root is somewhat ambiguous, --only-roots is somehow already too long to type. We have --all, not --all-processes, or --show-all. How about just --roots?

@r0hansaxena please hold off on updating this part until we reach a consensus 🙂

@edan-bainglass
Copy link
Copy Markdown
Member

Still failing 🥲 Have a look.

@r0hansaxena
Copy link
Copy Markdown
Author

Still failing 🥲 Have a look.

yes, I have made changes, it should work fine this time

@r0hansaxena
Copy link
Copy Markdown
Author

r0hansaxena commented Mar 10, 2026

Still failing 🥲 Have a look.

yes, I have made changes, it should work fine this time

i will address the failing ci checks again

@danielhollas
Copy link
Copy Markdown
Collaborator

Please first verify the tests locally, you can run them as

uv run pytest -m presto -n auto

- Remove -r shorthand for --roots to avoid conflict with --raw
- Fix LinkType import in calculation.py to avoid AttributeError in tests
- Add subquery in CalculationQueryBuilder to correctly filter out non-root processes without interfering with generate_joins() logic
- Fix test_list counts to match new expectations
- Add -r flag to all --roots tests in test_list to verify just data lines
- Moved link.add_incoming before node.store() in tests to respect AiiDA checks
@r0hansaxena
Copy link
Copy Markdown
Author

Please first verify the tests locally, you can run them as

uv run pytest -m presto -n auto

yes, I have been working on the same, I'll update it here

@danielhollas
Copy link
Copy Markdown
Collaborator

Thanks @danielhollas. Though I agree that --root is somewhat ambiguous, --only-roots is somehow already too long to type. We have --all, not --all-processes, or --show-all. How about just --roots?

Looking at the current options (verdi process list -h) I agree that "only" seems superfluous. Yes, --roots seems better than --root. Paging @mbercx as the local neighborhood spiderman UX bikeshedding expert. :-)

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.

verdi process list for root processes

3 participants