Remove stash, unstash and monitor keys from default CalcJob builder inputs #7143

Copilot · 2025-12-09T14:49:22Z

Understand the issue: stash, unstash, and monitors keys appear in empty builders
Investigate code to find root causes
Fix 1: Add populate_defaults=False to the monitors input namespace in calcjob.py
Fix 2: Modify ProcessBuilderNamespace.__init__ in builder.py to respect populate_defaults attribute
Add/run tests to verify the fix
Address PR feedback: make tests more general
Run code review
Run CodeQL security check

Summary

This PR removes the stash, unstash, and monitors keys from appearing in empty CalcJob builders.

Before:

>>> ArithmeticAddCalculation.get_builder()
{'metadata': {'options': {'stash': {}, 'unstash': {}}}, 'monitors': {}}

After:

>>> ArithmeticAddCalculation.get_builder()
{'metadata': {'options': {}}}

Changes Made

src/aiida/engine/processes/calcjobs/calcjob.py: Added populate_defaults=False to the monitors input namespace (already set for stash/unstash).
src/aiida/engine/processes/builder.py: Modified ProcessBuilderNamespace.__init__ to respect the populate_defaults attribute. Namespaces with populate_defaults=False are now lazily created only when accessed.
Tests: Added general test for populate_defaults=False behavior using a dedicated test process class.

Original prompt

This section details on the original issue you should resolve

<issue_title>Remove stash, unstash and monitor keys from the default inputs</issue_title>
<issue_description>From #6826 (comment):
@khsrali one question: do we need to always have the stash and unstash keys in the metadata.options? Now for every CalcJob, even an empty builder will have these keys, e.g.:
from aiida.calculations.arithmetic.add import ArithmeticAddCalculation

ArithmeticAddCalculation.get_builder()
returns
Process class: ArithmeticAddCalculation
Inputs:
metadata:
  options:
    stash: {}
    unstash: {}
monitors: {}
(I have a similar question for the monitors, but I'll check that separately.)

My concern it that this makes the often already complicated inputs for processes even more complicated, harder to read, and also will make users wonder what these inputs are and why they are there.
</issue_description>

Comments on the Issue (you are @copilot in this section)

Fixes Remove stash, unstash and monitor keys from the default inputs #6994

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

codecov · 2025-12-09T15:06:38Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.61%. Comparing base (cc0bb48) to head (2ea7e99).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7143      +/-   ##
==========================================
+ Coverage   79.61%   79.61%   +0.01%     
==========================================
  Files         566      566              
  Lines       43572    43575       +3     
==========================================
+ Hits        34684    34687       +3     
  Misses       8888     8888

☔ 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.

Copilot

Pull request overview

This PR removes stash, unstash, and monitors keys from empty CalcJob builders to reduce interface clutter and user confusion. The implementation uses the populate_defaults=False parameter to enable lazy namespace creation.

Key changes:

Added populate_defaults=False to the monitors input namespace in CalcJob definitions
Modified ProcessBuilderNamespace to respect populate_defaults by deferring namespace creation until first access
Added comprehensive tests for lazy namespace behavior and CalcJob builder output

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
`src/aiida/engine/processes/calcjobs/calcjob.py`	Added `populate_defaults=False` to monitors namespace, matching existing stash/unstash configuration
`src/aiida/engine/processes/builder.py`	Implemented lazy namespace creation - namespaces with `populate_defaults=False` are only created on first access
`tests/engine/processes/test_builder.py`	Added test class and comprehensive tests verifying lazy namespace behavior and CalcJob builder cleanliness

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

khsrali · 2025-12-09T16:21:40Z

@mbercx , actually worked 😄 🚀

from aiida.calculations.arithmetic.add import ArithmeticAddCalculation
>>> ArithmeticAddCalculation.get_builder()
{'metadata': {'options': {}}}

khsrali

How come you added populate_defaults=False, only to monitor and somehow it also disappeared from the results of stash and unstash as well. (I mean, what you did is a good solution I just want you to explain it)

khsrali · 2025-12-09T16:32:25Z

tests/engine/processes/test_builder.py

+    def define(cls, spec):
+        super().define(spec)
+        spec.input_namespace('optional_ns', non_db=True, required=False, populate_defaults=False)
+        spec.input('optional_ns.value', valid_type=str, non_db=True, required=False)


why do you have this line?

The line adds a child input (value) to the optional_ns namespace to make it a realistic test case. Without any child ports, the namespace would just be empty and not useful for testing the lazy creation and value-setting functionality.

khsrali · 2025-12-09T16:34:41Z

tests/engine/processes/test_builder.py

+def test_calcjob_builder_no_stash_unstash_monitors():
+    """Test that CalcJob builders don't include stash, unstash, and monitors in empty builder."""
+    ArithmeticAddCalculation = CalculationFactory('core.arithmetic.add')
+    builder = ArithmeticAddCalculation.get_builder()
+
+    # Verify that stash, unstash, and monitors are not in the builder
+    assert 'stash' not in dict(builder.metadata.options)
+    assert 'unstash' not in dict(builder.metadata.options)
+    assert 'monitors' not in dict(builder)
+
+    # But they should be accessible and settable
+    builder.metadata.options.stash.target_base = '/tmp/test'
+    builder.metadata.options.stash.source_list = ['file1']
+    builder.metadata.options.stash.stash_mode = 'copy'
+    assert 'stash' in dict(builder.metadata.options)
+    assert builder.metadata.options.stash.target_base == '/tmp/test'
+
+


Although the original issue specifically was targeting stash, unstash, and monitors; Do this a bit more general. I don't want to have super single use tests in the code base.

Removed the specific test in 7d97fb5. The general test_builder_populate_defaults_false test using PopulateDefaultsProcess now covers the behavior without being tied to specific CalcJob namespaces.

Copilot · 2025-12-09T16:41:32Z

How come you added populate_defaults=False, only to monitor and somehow it also disappeared from the results of stash and unstash as well. (I mean, what you did is a good solution I just want you ...

The change to builder.py makes the ProcessBuilderNamespace respect the populate_defaults attribute for ALL namespaces. The stash and unstash namespaces already had populate_defaults=False set in the original calcjob.py (lines 437 and 456), but the ProcessBuilderNamespace wasn't checking this flag before. My change to builder.py (lines 71-83) now checks port.populate_defaults and only pre-creates the namespace if it's True. For namespaces with populate_defaults=False, they're lazily created on first access instead.

khsrali · 2025-12-09T16:45:25Z

@mbercx IMO, this solution is already good enough. let me know what do you think, cheers!

…uilder Co-authored-by: khsrali <[email protected]>

…of general populate_defaults test Co-authored-by: khsrali <[email protected]>

Copilot AI assigned Copilot and khsrali Dec 9, 2025

Copilot started work on behalf of khsrali December 9, 2025 14:49 View session

Copilot AI changed the title ~~[WIP] Remove stash, unstash and monitor keys from default inputs~~ Remove stash, unstash and monitor keys from default CalcJob builder inputs Dec 9, 2025

Copilot AI requested a review from khsrali December 9, 2025 15:04

Copilot finished work on behalf of khsrali December 9, 2025 15:04

khsrali requested review from Copilot and mbercx December 9, 2025 16:07

Copilot started reviewing on behalf of khsrali December 9, 2025 16:08 View session

Copilot AI reviewed Dec 9, 2025

View reviewed changes

khsrali marked this pull request as ready for review December 9, 2025 16:22

khsrali requested changes Dec 9, 2025

View reviewed changes

Copilot started work on behalf of khsrali December 9, 2025 16:37 View session

Copilot AI requested a review from khsrali December 9, 2025 16:45

Copilot finished work on behalf of khsrali December 9, 2025 16:45

Copilot AI and others added 4 commits December 10, 2025 09:43

Initial plan

5939f71

Fix populate_defaults=False for stash, unstash, monitors in CalcJob b…

a3e62c0

…uilder Co-authored-by: khsrali <[email protected]>

Simplify tests: remove specific stash/unstash/monitors test in favor …

4b382e0

…of general populate_defaults test Co-authored-by: khsrali <[email protected]>

align fgetter function signature

2ea7e99

khsrali force-pushed the copilot/remove-stash-unstash-monitor-keys branch from 9786997 to 2ea7e99 Compare December 10, 2025 08:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Remove stash, unstash and monitor keys from default CalcJob builder inputs #7143

Remove stash, unstash and monitor keys from default CalcJob builder inputs #7143

Copilot AI commented Dec 9, 2025 •

edited

Loading

Uh oh!

codecov bot commented Dec 9, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

khsrali commented Dec 9, 2025

Uh oh!

khsrali left a comment

Uh oh!

khsrali Dec 9, 2025

Uh oh!

Copilot AI Dec 9, 2025

Uh oh!

khsrali Dec 9, 2025

Uh oh!

Copilot AI Dec 9, 2025

Uh oh!

Copilot AI commented Dec 9, 2025

Uh oh!

khsrali commented Dec 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Remove stash, unstash and monitor keys from default CalcJob builder inputs #7143

Are you sure you want to change the base?

Remove stash, unstash and monitor keys from default CalcJob builder inputs #7143

Conversation

Copilot AI commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Before:

After:

Changes Made

Comments on the Issue (you are @copilot in this section)

Uh oh!

codecov bot commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

khsrali commented Dec 9, 2025

Uh oh!

khsrali left a comment

Choose a reason for hiding this comment

Uh oh!

khsrali Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

khsrali Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Dec 9, 2025

Uh oh!

khsrali commented Dec 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Copilot AI commented Dec 9, 2025 •

edited

Loading

codecov bot commented Dec 9, 2025 •

edited

Loading