fix: resolve two ValueError bugs in config_loader._get_explicit_cli_args

[thc1006]

TLDR

Fixes two bugs in experiments/config_loader.py that crash all experiment scripts on startup.

Closes #1

What changed

_get_explicit_cli_args uses a sentinel-based probe parser to detect which CLI arguments were explicitly provided. Two argparse API misuses caused immediate ValueError on any experiment script invocation:

Bug 1 -- positional arg with duplicate dest

# Before: dest passed both as positional name AND kwarg
probe.add_argument(action.dest, dest=action.dest, ...)
# ValueError: dest supplied twice for positional argument

Fix: only add dest to kwargs for optional arguments (those with option_strings). Positional arguments infer dest from their name.

Bug 2 -- store_true with nargs=0

# Before: store_true has action.nargs=0, which passed the None check
if action.nargs is not None:
    kwargs["nargs"] = action.nargs  # nargs=0 is invalid
# ValueError: nargs for store actions must be != 0

Fix: detect store_true, store_false, store_const, and count actions via isinstance and preserve them with action= kwarg instead of passing nargs=0.

Test plan

• 13 new tests in tests/test_config_loader.py
  • Positional arg detection (present / absent)
  • store_true flag detection (present / absent)
  • store_false flag detection (present / absent)
  • Optional arg with value detection
  • Combined positional + store_true regression (exact issue Bug: config_loader._get_explicit_cli_args fails with positional and store_true arguments #1 scenario)
  • load_config merge precedence: CLI > YAML > defaults
  • Merged config value verification for store_true
• Full test suite: 219 passed, 0 failed
• ruff check + ruff format clean

[Copilot]

Pull request overview

Fixes experiment startup crashes by correcting argparse “probe parser” reconstruction in experiments/config_loader.py, and adds regression tests to ensure explicit-CLI detection and config-merge precedence behave as intended.

Changes:

• Reworks _get_explicit_cli_args to avoid argparse ValueErrors for positional args (duplicate dest) and store_* flags (nargs=0 misuse).
• Adds a new tests/test_config_loader.py test suite covering the reported regressions and merge precedence (CLI > YAML > defaults).

Reviewed changes

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

File	Description
experiments/config_loader.py	Fixes argparse probe-parser construction to prevent startup crashes and correctly detect explicit CLI args.
tests/test_config_loader.py	Adds regression + precedence tests for _get_explicit_cli_args and load_config.

---

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

[Copilot]

action='count' cannot use a non-None sentinel default: argparse increments the existing value, so when the flag is provided it will attempt sentinel + 1 and raise TypeError. This means _get_explicit_cli_args will currently break for any parser that includes a count flag. Consider handling _CountAction separately by using a safe default (e.g. None or 0) and updating the explicit-detection logic accordingly (e.g. treat None/0 as “not provided” for count flags).

[Copilot]

This test module imports config_loader, which imports yaml (PyYAML). CI installs deps via pip install -e ".[dev,pyscf]", and pyyaml isn’t included in those extras, so these tests will raise ModuleNotFoundError: yaml and fail. Either add PyYAML to the dev/test dependencies, or guard these tests with an early pytest.importorskip("yaml") (before importing config_loader) if YAML support is intentionally optional.