[asmacdo]

A malformed --summary-format is now rejected before the command runs (exit 2, one-line error, no traceback) instead of crashing after the run and replacing the command's exit code. The str fallback that silently discarded a bad format spec is removed.

• Closes Error out if user provides errorneous format specification #192

Changes

• Validate at parse time. --summary-format gets an argparse type= that renders the template against typed placeholder values (VALIDATE_SUMMARY_PLACEHOLDERS), once with colors off and once on. argparse applies type= to string defaults too, so a bad DUCT_SUMMARY_FORMAT (env or .env) is caught the same way; the error names all three sources.
• Drop the fallback in SummaryFormatter.format_field: {peak_rss:.2d} used to print the raw 123456 plus a warning; it now fails validation.
• Guard the post-run render in execute(): if rendering still fails (e.g. execute() called directly), log an error and return the command's exit code.
• Hint for Python's {field!conv:spec} order: duct's conversions return colored strings, so {wall_clock_time!X:.3f} fails with colors; the error suggests {wall_clock_time:.3f!X}.
• CLAUDE.md: fail fast before the run, loud fallbacks after it.

Example:

$ con-duct run --summary-format '{nonexistent}' sleep 5
con-duct run: error: argument --summary-format: '{nonexistent}': KeyError: 'nonexistent' (set via --summary-format, DUCT_SUMMARY_FORMAT, or a .env file)
$ echo $?
2

Behavior changes to note

• Templates the fallback used to tolerate are now rejected: a spec that does not fit the value's type ({wall_clock_time:d}, {start_time:%Y}, {average_rss:,d}), and {field!X:spec} when it only works without colors.
• Attribute and index access in field names ({command[0]}) are rejected, even where the value supports them.
• test_execution_summary_formatted_wall_clock_time_nowvalid pinned the fallback for {wall_clock_time!X:.3f} with colors; it now expects the ValueError.
• The default format is unaffected (a test asserts it validates).

Verification

• tox -e py313: 519 passed after rebasing on main; tox -e lint,typing clean; CI green on all platforms.
• New tests: parse-time rejection of 14 bad templates (unknown key, unmatched brace, unknown conversion, positional fields, type-mismatched specs, attribute/index access even where the value supports it, conversion order); env-var rejection; the conversion-order hint; placeholder keys and types equal a real Report's execution_summary after two samples (mutation-checked: an int average_rss placeholder fails it); execute() with a bad template returns the command's exit code and keeps the logs.

Why placeholders, and why the type test

A spec error depends only on the value's type (None renders as - before the spec is applied), so rendering against one correctly typed value per key reproduces exactly what the post-run render would do, no stricter and no looser. Python accepts float specs on ints ({peak_rss:.2f}) but not d on floats, and the validator inherits that exactly. The type test keeps the placeholders honest as summary fields change.

Attribute and index access in a field name ({command[0]}, {peak_rss.real}) are rejected before rendering: whether they work can depend on the value, not just its type ({command[20]} fails on the placeholder but may work on a real command), so a placeholder cannot vouch for them. They can be added explicitly later if someone needs them.

except Exception in the argparse type: any failure to render known-good placeholders means the template is bad, whatever the exception type; catching a fixed list left some as a traceback or argparse's generic "invalid value" message.

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.80%. Comparing base (58b1848) to head (fcca2a1).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #454      +/-   ##
==========================================
+ Coverage   94.60%   94.80%   +0.20%     
==========================================
  Files          15       15              
  Lines        1389     1406      +17     
  Branches      186      190       +4     
==========================================
+ Hits         1314     1333      +19     
- Misses         46       47       +1     
+ Partials       29       26       -3     

☔ View full report in Codecov by Harness.
📢 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]

Copilot review overview

🔵 Needs a closer look

It is a user-facing behavior change to CLI argument handling and exit-code semantics (previously-tolerated formats and env defaults now hard-fail at parse time), which warrants a human maintainer's compatibility sign-off despite being well-tested.

Review effort: Balanced
Findings: None

What changed in this PR

This PR makes con-duct run reject a malformed --summary-format at argparse parse time (exit 2, one-line error, no traceback) instead of crashing after the command runs and clobbering its exit code. It removes the silent str fallback in SummaryFormatter.format_field that used to mask bad specs, and adds a defensive guard around the post-run render so a render failure never costs the user a completed (potentially expensive) run. This implements the "fail fast, but never punish a finished run" philosophy now codified in CLAUDE.md, closing #192.

Changes:

• Add validate_summary_format + VALIDATE_SUMMARY_PLACEHOLDERS and wire them into a new argparse type=_summary_format_type, which also validates the DUCT_SUMMARY_FORMAT/.env default and emits a conversion-order hint for {field!conv:spec}.
• Drop the ValueError→str fallback in format_field; guard the post-run render in execute() to log an error and preserve the command's exit code and logs.
• Add tests for parse-time rejection, env-var rejection, the conversion-order hint, placeholder/type parity with a real run, and exit-code preservation on post-run render failure.

File	Description
src/​con_duct/​_formatter.py	Removes silent str fallback; adds VALIDATE_SUMMARY_PLACEHOLDERS and validate_summary_format to render a template against typed placeholders.
src/​con_duct/​cli.py	Adds _summary_format_type argparse type (with conversion-order hint) and applies it to --summary-format.
src/​con_duct/​_duct_main.py	Wraps the post-run summary render in try/except, logging an error while preserving the command's exit code and logs.
test/​test_formatter.py	Adds spec-mismatch, default-validation, and placeholder-parity tests; updates the conversion-order expectation to ValueError.
test/​test_cli.py	Adds parse-time rejection, env-var rejection, pass-through, and conversion-order-hint tests.
test/​duct_main/​test_execution.py	Adds a test that a post-run render failure keeps the command's exit code and logs.
CLAUDE.md	Documents the fail-fast-before-run vs. loud-fallback-after-run convention.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Note

Copilot was unable to complete this review at the selected tier due to model policy or plan restrictions.

🟡 Changes recommended

A moderate-severity issue remains in validate_summary_format() where placeholder-based rendering fails to reliably reject templates using attribute or index access (e.g., {command[0]}, {working_directory.__class__}), allowing unintended formats to pass validation.

1 open issue

• Placeholder-based validation misses attribute and index access · New

_{Review effort: Balanced}

[asmacdo]

Good catch, though for a slightly different reason: the goal was for validation to match the post-run render, and {command[0]} renders fine there too. The real gap is that index access can depend on the value, not just its type ({command[20]} fails on the 7-character placeholder but may work on a real command), so a placeholder can't vouch for it. fcca2a1 now rejects attribute and index access in field names outright; they can be added explicitly later if someone needs them.

Generated by Claude Code

[asmacdo]

@yarikoptic I've had a look at this and I think its good to go. Requested review if you'd like, otherwise I'll plan to merge in a couple days if you dont have the time.

[yarikoptic]

I do not get what this structure is for, in particular why it duplicates some configurable values like this?

[yarikoptic]

in particular how that relates to the PR subject line of "Reject a bad --summary-format before running the command" -- how any particular value here is helpful to validate??

[asmacdo]

It's a dummy. At the start we render this through the summary format so we catch any issues with the user provided summary format before we start executing the command