Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #393 +/- ##
==========================================
- Coverage 91.69% 90.56% -1.14%
==========================================
Files 15 15
Lines 1120 1123 +3
Branches 139 139
==========================================
- Hits 1027 1017 -10
- Misses 70 84 +14
+ Partials 23 22 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR adds an --output-prefix (-p) option to con-duct ls and relocates CLI default values into cli.py to remove a circular import and ensure defaults are read after .env files are loaded.
Changes:
- Move CLI default constants from
duct_main.pytocli.py(DEFAULT_OUTPUT_PREFIX,DEFAULT_SUMMARY_FORMAT). - Add
--output-prefix/-ptocon-duct lsand refactorls.pyto derive the glob search prefix from args instead of importing fromduct_main. - Update tests to use the new constants and cover custom
ls --output-prefixbehavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/con_duct/cli.py | Introduces new default constants and uses env-evaluated defaults at parser creation; adds ls --output-prefix. |
| src/con_duct/duct_main.py | Removes CLI-default constants from the execution module. |
| src/con_duct/ls.py | Stops importing defaults and uses args.output_prefix to build the glob pattern when no paths are provided. |
| test/utils.py | Updates test helper to use new default constants from cli.py. |
| test/test_ls.py | Adds coverage for ls with default and custom output-prefix globbing. |
| test/duct_main/test_report.py | Updates imports to new summary format constant location. |
| test/duct_main/test_aggregation.py | Updates imports to new summary format constant location. |
Comments suppressed due to low confidence (1)
src/con_duct/cli.py:287
- The help text for --output-prefix lists {datetime} as a supported format variable, but LogPaths.create() only provides pid and datetime_filesafe when calling output_prefix.format(...). Using {datetime} will raise a KeyError at runtime. Either add a datetime value to LogPaths.create() formatting, or remove {datetime} from the help/ docs to match actual behavior.
help="File string format to be used as a prefix for the files -- the captured "
"stdout and stderr and the resource usage logs. The understood variables are "
"{datetime}, {datetime_filesafe}, and {pid}. "
"Leading directories will be created if they do not exist. "
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Move DUCT_OUTPUT_PREFIX and EXECUTION_SUMMARY_FORMAT out of duct_main.py and into cli.py as DEFAULT_OUTPUT_PREFIX and DEFAULT_SUMMARY_FORMAT. These are CLI defaults, not execution logic. Break the circular import (cli -> ls -> duct_main -> cli) by adding --output-prefix / -p to con-duct ls, passing the value through args so ls.py no longer imports from duct_main. Also fix a latent bug: DEFAULT_OUTPUT_PREFIX is now a plain string constant, with os.getenv() evaluated at parser creation time (after .env files are loaded) rather than at import time. Closes con#386 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Previously `con-duct ls` with no paths globbed for `{prefix}*`, which
could match unrelated files. Now globs for `{prefix}*info.json` using
the SUFFIXES constant, so only duct info files are matched.
Addresses review feedback about overly broad globbing when the output
prefix is short or empty.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
src/con_duct/cli.py:287
- The help text says the understood variables are
{datetime}and{pid}, butLogPaths.create()still supports{datetime_filesafe}for backwards compatibility (marked deprecated). Consider mentioning{datetime_filesafe}is supported-but-deprecated to avoid confusing users migrating older configs.
default=os.getenv("DUCT_OUTPUT_PREFIX", DEFAULT_OUTPUT_PREFIX),
help="File string format to be used as a prefix for the files -- the captured "
"stdout and stderr and the resource usage logs. The understood variables are "
"{datetime} and {pid}. "
"Leading directories will be created if they do not exist. "
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Internal changes:
DUCT_OUTPUT_PREFIXandEXECUTION_SUMMARY_FORMATfromduct_main.pytocli.py, renamed toDEFAULT_OUTPUT_PREFIXandDEFAULT_SUMMARY_FORMAT--output-prefix/-ptocon-duct ls, sols.pyreceives the prefix via args instead of importing it fromduct_mainFix latent bug:
.envfiles were loaded); nowos.getenv()runs at parser creation time insidemain()Closes #386
🤖 Generated with Claude Code