Skip to content

Contain outputs under an output root (zwill_work/)#70

Merged
johnjosephhorton merged 1 commit into
mainfrom
output-root-guardrail
Jul 9, 2026
Merged

Contain outputs under an output root (zwill_work/)#70
johnjosephhorton merged 1 commit into
mainfrom
output-root-guardrail

Conversation

@johnjosephhorton

Copy link
Copy Markdown
Contributor

What

Reports, bundles, executive summaries, and exported CSVs now land under a single output root (default zwill_work/, beside .zwill/) instead of sprawling across the working directory — the file-sprawl the research-agent run hit. A doc convention is a soft guardrail; this makes the system set the location.

resolve_output_path():

  • relative --path/--out and CWD-relative defaults (<survey>_report/, artifacts/...) → rebased under the root
  • absolute paths → written verbatim (escape hatch) with a one-line warning that they left the root
  • root resolved from ZWILL_OUT env > .zwill/config.json output_dir (set via zwill init --output-dir) > default zwill_work/
  • rebasing creates the parent dir, so callers that assumed a CWD-relative parent still have a writable location

Deliberately not rebased

To avoid breaking read-back chains and misplacing state (found empirically — the example scripts' edsl-export --path job.jsonedsl-run --job job.json chain broke when the write rebased but the read didn't):

  • managed state stays in .zwill/
  • intermediate EDSL plumbing read back by a later command — edsl-export/edsl-run --path (job/results) and --job-path — stays at CWD
  • explicit directory args (--output-dir, --workdir, --artifacts-dir) that hold job plumbing or whole project dirs stay at CWD
  • --input-path reads are never rebased

Rebased (deliverable file outputs): path, out, report-html/json/csv, results-path, prompt-path, context-path, markdown-path, json-path, option-path, zip-path.

This is enabled by the earlier --input-path rename (#69), which made every --path/--out unambiguously an output.

Behavior

survey report --path survey_report.html     -> zwill_work/survey_report.html
report build (no --path)                     -> zwill_work/<survey>_report/
twin-validate --out bundle/                  -> zwill_work/bundle/
--path /abs/x.html                           -> /abs/x.html  (+ warning)
edsl-export --path job.edsl.json             -> job.edsl.json (CWD, plumbing)
ZWILL_OUT=out survey report --path r.html    -> out/r.html

Testing

  • pytest — 248 passed (adds tests/test_output_root.py: rebase, absolute-passthrough-with-single-warning, .zwill never rebased, no double-nesting, env override, init --output-dir persistence).
  • ruff clean.
  • Live-verified all six behaviors above end-to-end.

🤖 Generated with Claude Code

The research-agent run flung report files, bundles, and exports across the
working directory. Guidance in a doc is a weak guardrail; make the system set the
location instead.

Add resolve_output_path(): relative --path/--out and CWD-relative defaults (e.g.
<survey>_report/, artifacts/...) rebase under an output root, default zwill_work/
beside .zwill/. Override with the ZWILL_OUT env var or `zwill init --output-dir`
(persisted in config.json). Absolute paths are written verbatim (an escape hatch)
with a one-line warning that they left the root. When a path is rebased its parent
dir is created, so callers that assumed a CWD-relative parent still have a
writable location.

Deliberately NOT rebased, to avoid breaking read-back chains and misplacing state:
- managed state stays in .zwill/;
- intermediate EDSL plumbing read back by a later command — edsl-export/edsl-run
  --path (job/results) and --job-path — stays at CWD so `edsl-run --job <file>`
  still finds it;
- explicit directory args (--output-dir, --workdir, --artifacts-dir) that hold
  job plumbing or whole project dirs stay at CWD;
- --input-path reads are never rebased.

Rebased flags are the deliverable file outputs: path, out, report-html/json/csv,
results-path, prompt-path, context-path, markdown-path, json-path, option-path,
zip-path. Enabled by the earlier --input-path rename, which made every
--path/--out unambiguously an output.

Adds tests/test_output_root.py and documents the convention in the agent
workflow guide.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@johnjosephhorton johnjosephhorton merged commit af0bb14 into main Jul 9, 2026
1 of 3 checks passed
@johnjosephhorton johnjosephhorton deleted the output-root-guardrail branch July 9, 2026 11:40
@greptile-apps

greptile-apps Bot commented Jul 9, 2026

Copy link
Copy Markdown

Greptile Summary

This PR introduces a centralized output root (zwill_work/ by default) so that reports, bundles, executive summaries, and exported CSVs land under a single directory rather than sprawling across the CWD. The new resolve_output_path() helper in cli.py rebases relative paths under the output root, passes absolute paths through with a one-time warning, and leaves .zwill/ managed state and EDSL plumbing paths untouched.

  • cli.py adds output_root(), _is_under(), and resolve_output_path() with correct precedence (env var > config > default) and a create_parents flag for existence-check callers; cmd_init is extended to persist --output-dir without clobbering existing config.
  • All ~15 command modules are updated to route output --path/--out/--report-* arguments through resolve_output_path; tests/test_output_root.py covers the six key behaviors end-to-end.
  • One misclassification: the positional path argument in workflow explain, workflow dry-run, and workflow run is an INPUT file (workflow YAML/JSON to read), not an output \u2014 passing it through resolve_output_path will break those commands for any CWD-relative workflow file.

Confidence Score: 3/5

The core output-root machinery and the bulk of the command-module updates are correct, but the three workflow commands now look for the workflow config file under zwill_work/ instead of the current directory, making them immediately non-functional for the common case of a CWD-relative workflow file.

The rebase logic in cli.py is well-designed and the test coverage is solid. The majority of the 15 changed modules apply resolve_output_path correctly. However, workflow_commands.py applies it to a positional argument that is a workflow configuration file the command reads via load_workflow_file(path), not a file it writes. Running zwill workflow run my_workflow.yaml will fail with a file-not-found because it searches zwill_work/my_workflow.yaml instead.

zwill/workflow_commands.py needs the resolve_output_path calls removed from cmd_workflow_explain, cmd_workflow_dry_run, and cmd_workflow_run — all three use path to READ an existing workflow file, not to write an output.

Important Files Changed

Filename Overview
zwill/cli.py Adds output_root(), _is_under(), and resolve_output_path() core functions; updates cmd_init to persist --output-dir without clobbering existing config — logic is sound and well-tested
zwill/workflow_commands.py Incorrectly applies resolve_output_path to the positional path argument of cmd_workflow_explain, cmd_workflow_dry_run, and cmd_workflow_run, which is an input file path (workflow YAML/JSON), not an output — breaks all three workflow commands for CWD-relative paths
zwill/cli_parser.py Adds --output-dir argument to the init subcommand with clear help text and correct wiring
tests/test_output_root.py Comprehensive tests covering rebase, absolute passthrough with single warning, .zwill exemption, no double-nesting, env override, and init --output-dir persistence — all cases well-isolated with monkeypatch
zwill/practitioner_reports.py Consistently rebases output paths for reports, exports, and render commands; double-calling resolve_output_path pattern is present in several spots
zwill/report_bundle.py Rebases bundle output dir and catalog paths correctly; report_bundle_default_dir now calls resolve_output_path with default create_parents=True, creating the directory as a side effect of computing the default — harmless given the subsequent explicit mkdir
zwill/twin_validation_commands.py All report/export/render --path and --markdown-path args correctly rebased; double-calling pattern is the most prevalent here with ~7 pairs
zwill/twin_studies.py Rebases --report-html, --report-json, --report-csv, and --results-path while correctly leaving --job-path (plumbing) at CWD — consistent with PR design

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["User supplies --path / --out\n(or command default)"] --> B{Is path None?}
    B -->|yes| Z[return None]
    B -->|no| C{Is path under .zwill/?}
    C -->|yes| D[Return as-is\nmanaged state]
    C -->|no| E{Is path absolute?}
    E -->|yes, inside root| F[Return as-is]
    E -->|yes, outside root| G[Warn once to stderr\nReturn as-is]
    E -->|no| H{Already under output_root?}
    H -->|yes| I[Return as-is\nno double-nest]
    H -->|no| J[Rebase: root / path]
    J -->|create_parents=True| K[mkdir parents\nthen return rebased path]
    J -->|create_parents=False| L[Return rebased path\nno mkdir side-effect]

    M["output_root() resolution"] --> N{ZWILL_OUT env var set?}
    N -->|yes| O[Use ZWILL_OUT]
    N -->|no| P{.zwill/config.json\nhas output_dir?}
    P -->|yes| Q[Use configured output_dir]
    P -->|no| R[Use default: zwill_work/]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["User supplies --path / --out\n(or command default)"] --> B{Is path None?}
    B -->|yes| Z[return None]
    B -->|no| C{Is path under .zwill/?}
    C -->|yes| D[Return as-is\nmanaged state]
    C -->|no| E{Is path absolute?}
    E -->|yes, inside root| F[Return as-is]
    E -->|yes, outside root| G[Warn once to stderr\nReturn as-is]
    E -->|no| H{Already under output_root?}
    H -->|yes| I[Return as-is\nno double-nest]
    H -->|no| J[Rebase: root / path]
    J -->|create_parents=True| K[mkdir parents\nthen return rebased path]
    J -->|create_parents=False| L[Return rebased path\nno mkdir side-effect]

    M["output_root() resolution"] --> N{ZWILL_OUT env var set?}
    N -->|yes| O[Use ZWILL_OUT]
    N -->|no| P{.zwill/config.json\nhas output_dir?}
    P -->|yes| Q[Use configured output_dir]
    P -->|no| R[Use default: zwill_work/]
Loading

Comments Outside Diff (1)

  1. zwill/agent_studies.py, line 74-75 (link)

    P2 resolve_output_path called twice per write

    This pattern calls resolve_output_path(args.path) twice back-to-back — once for the .parent.mkdir() and once for the actual write. Since resolve_output_path reads os.environ (and possibly config.json) to determine the output root on every call, this doubles the I/O and config-read overhead for each write operation. The same pattern appears in roughly two dozen spots across practitioner_reports.py, probability_commands.py, twin_validation_commands.py, twin_studies.py, and others. Storing the result in a local variable (out = resolve_output_path(args.path)) before using it eliminates the redundancy.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Reviews (1): Last reviewed commit: "Contain outputs under an output root (zw..." | Re-trigger Greptile

Comment on lines 121 to 143
@@ -137,7 +137,7 @@ def cmd_workflow_dry_run(args: argparse.Namespace) -> dict[str, Any]:


def cmd_workflow_run(args: argparse.Namespace) -> dict[str, Any]:
path = Path(args.path)
path = resolve_output_path(args.path)
config = load_workflow_file(path)
values = workflow_vars(config, args.var)
steps = rendered_workflow_steps(config, values)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Workflow path is an input, not an output — rebasing it breaks all workflow commands

The path positional argument for workflow explain, workflow dry-run, and workflow run points to an existing workflow YAML/JSON configuration file that is only ever READ (via load_workflow_file(path)). Applying resolve_output_path rebases it under zwill_work/, so zwill workflow run my_workflow.yaml now looks for zwill_work/my_workflow.yaml rather than ./my_workflow.yaml, causing a file-not-found error for any workflow file at a CWD-relative path.

The parser confirms this is a positional input: p.add_argument("path", help="Path to a workflow .json/.yaml/.yml file."). The PR's own design rule states inputs are never rebased; this is exactly the kind of path the --input-path rename in #69 was meant to disambiguate — but workflow's positional path was not renamed and is now incorrectly treated as an output.

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.

1 participant