Skip to content

Commit 3509903

Browse files
NikitasT2003claude
andcommitted
fix(setup): honest reporting when examples are already present
Reported bug: user ran `thesis setup`, answered "Yes" to the Step 5 prompt about copying example sources, and saw: ✓ copied 0 example file(s). The Yes answer did nothing visible — all three example files were already in the workspace from a prior setup run. The code was correct (it skipped existing targets) but the message was a lie. Changes - _copy_examples(*, overwrite=False) now returns a counter dict with four fields: copied, overwritten, already_present, source_missing. - Wizard message reflects real state: * fresh run: "examples: copied 3 new" * partial: "examples: copied 1 new, left 2 already-present in place" * unchanged: "examples already in your workspace (3 file(s)) — nothing to copy. Use --overwrite-examples if you want to replace them." * --overwrite-examples: "examples: overwrote 3" * install without examples/ tree: "bundled examples/ tree not found in this install — skipped." - New CLI flag: `--overwrite-examples` for users who want to replace their (possibly edited) example files. Tests (9 new, 262 total passing) - TestCopyExamplesHelper: fresh→copies, repeat→already_present, --overwrite replaces, counter dict shape. - TestWizardExampleReport: end-to-end via CliRunner asserts the exact wording users will see in each scenario. - Regression guard: "copied 0 example" substring must never appear when the user answered Yes to the prompt. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent ffd22ea commit 3509903

5 files changed

Lines changed: 220 additions & 16 deletions

File tree

src/thesis_agent/cli.py

Lines changed: 54 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -96,13 +96,23 @@ def _ensure_workspace(force: bool = False) -> None:
9696
)
9797

9898

99-
def _copy_examples() -> int:
100-
"""Copy `examples/` into the workspace. Returns count of files copied."""
99+
def _copy_examples(*, overwrite: bool = False) -> dict[str, int]:
100+
"""Copy `examples/` into the workspace.
101+
102+
Returns a dict with three counters so the caller can report honestly:
103+
- `copied`: files written to an empty target
104+
- `overwritten`: existing files replaced (only when `overwrite=True`)
105+
- `already_present`: skipped because the target already existed
106+
(only when `overwrite=False`)
107+
A `source_missing` bool indicates the bundled `examples/` tree is absent
108+
— happens in odd installs (e.g. wheel without examples bundled).
109+
"""
110+
result = {"copied": 0, "overwritten": 0, "already_present": 0, "source_missing": 0}
101111
src_root = Path(__file__).resolve().parent.parent.parent / "examples"
102112
if not src_root.exists():
103-
return 0
113+
result["source_missing"] = 1
114+
return result
104115
p = paths()
105-
n = 0
106116
mapping = {
107117
src_root / "research" / "raw": p.raw,
108118
src_root / "style" / "samples": p.style_samples,
@@ -113,12 +123,19 @@ def _copy_examples() -> int:
113123
continue
114124
dst.mkdir(parents=True, exist_ok=True)
115125
for f in src.iterdir():
116-
if f.is_file() and f.name != ".gitkeep":
117-
target = dst / f.name
118-
if not target.exists():
126+
if not (f.is_file() and f.name != ".gitkeep"):
127+
continue
128+
target = dst / f.name
129+
if target.exists():
130+
if overwrite:
119131
target.write_bytes(f.read_bytes())
120-
n += 1
121-
return n
132+
result["overwritten"] += 1
133+
else:
134+
result["already_present"] += 1
135+
else:
136+
target.write_bytes(f.read_bytes())
137+
result["copied"] += 1
138+
return result
122139

123140

124141
# ---------------------------------------------------------------------------
@@ -202,6 +219,10 @@ def setup(
202219
skip_examples: bool = typer.Option(
203220
False, "--skip-examples", help="Don't copy example sources.",
204221
),
222+
overwrite_examples: bool = typer.Option(
223+
False, "--overwrite-examples",
224+
help="Replace example files in the workspace even if they already exist.",
225+
),
205226
quickstart: bool = typer.Option(
206227
False, "--quickstart", help="Accept sensible defaults for every prompt.",
207228
),
@@ -339,8 +360,30 @@ def setup(
339360
except _Cancelled:
340361
copy_ex = False
341362
if copy_ex:
342-
n = _copy_examples()
343-
console.print(f" [green]✓[/] copied {n} example file(s).")
363+
res = _copy_examples(overwrite=overwrite_examples)
364+
if res["source_missing"]:
365+
console.print(
366+
" [yellow]bundled examples/ tree not found in this install — "
367+
"skipped.[/]"
368+
)
369+
elif res["copied"] or res["overwritten"]:
370+
parts = []
371+
if res["copied"]:
372+
parts.append(f"copied {res['copied']} new")
373+
if res["overwritten"]:
374+
parts.append(f"overwrote {res['overwritten']}")
375+
if res["already_present"]:
376+
parts.append(f"left {res['already_present']} already-present in place")
377+
console.print(f" [green]✓[/] examples: {', '.join(parts)}.")
378+
elif res["already_present"]:
379+
# The honest report: nothing to do, not a failure.
380+
console.print(
381+
f" [dim]examples already in your workspace ({res['already_present']} "
382+
f"file(s)) — nothing to copy. Use [cyan]--overwrite-examples[/] "
383+
f"if you want to replace them.[/]"
384+
)
385+
else:
386+
console.print(" [dim]no example files to copy.[/]")
344387
else:
345388
console.print(" [dim]skipped examples.[/]")
346389

tests/test_cli_helpers.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,9 @@ def test_copy_examples_returns_count(self, tmp_path: Path, monkeypatch):
100100
p = paths()
101101
for d in (p.raw, p.style_samples, p.thesis_dir):
102102
d.mkdir(parents=True, exist_ok=True)
103-
n = cli._copy_examples()
104-
assert n >= 1 # at least the example source
103+
res = cli._copy_examples()
104+
assert isinstance(res, dict)
105+
assert res["copied"] >= 1 # at least the example source
105106

106107

107108
# ---------------------------------------------------------------------------

tests/test_copy_examples.py

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
"""Copy-examples honesty.
2+
3+
User reported running `thesis setup` → Step 5 → answered "Yes" → wizard
4+
replied "copied 0 example file(s)". The call was correct (targets already
5+
existed) but the report was misleading. These tests lock in the fix:
6+
report copied / overwritten / already-present separately, and expose
7+
`--overwrite-examples` for when users actually want to replace them.
8+
"""
9+
10+
from __future__ import annotations
11+
12+
from pathlib import Path
13+
14+
import pytest
15+
from typer.testing import CliRunner
16+
17+
from thesis_agent import cli
18+
from thesis_agent.config import paths
19+
20+
runner = CliRunner()
21+
22+
23+
@pytest.fixture
24+
def ws(tmp_path: Path, monkeypatch):
25+
monkeypatch.chdir(tmp_path)
26+
for v in (
27+
"THESIS_PROVIDER", "ANTHROPIC_API_KEY",
28+
"OPENROUTER_API_KEY", "OPENROUTER_BASE_URL",
29+
):
30+
monkeypatch.delenv(v, raising=False)
31+
return tmp_path
32+
33+
34+
# ---------------------------------------------------------------------------
35+
# _copy_examples helper direct tests
36+
# ---------------------------------------------------------------------------
37+
38+
class TestCopyExamplesHelper:
39+
def test_fresh_workspace_copies_files(self, ws: Path):
40+
p = paths()
41+
for d in (p.raw, p.style_samples, p.thesis_dir):
42+
d.mkdir(parents=True, exist_ok=True)
43+
res = cli._copy_examples()
44+
assert res["copied"] >= 1
45+
assert res["already_present"] == 0
46+
assert res["overwritten"] == 0
47+
assert res["source_missing"] == 0
48+
49+
def test_second_call_reports_already_present(self, ws: Path):
50+
p = paths()
51+
for d in (p.raw, p.style_samples, p.thesis_dir):
52+
d.mkdir(parents=True, exist_ok=True)
53+
first = cli._copy_examples()
54+
second = cli._copy_examples()
55+
assert second["copied"] == 0
56+
assert second["already_present"] == first["copied"]
57+
58+
def test_overwrite_flag_rewrites_files(self, ws: Path):
59+
p = paths()
60+
for d in (p.raw, p.style_samples, p.thesis_dir):
61+
d.mkdir(parents=True, exist_ok=True)
62+
cli._copy_examples()
63+
# User edits an example
64+
edited = p.raw / "example-source.md"
65+
edited.write_text("MY EDITS", encoding="utf-8")
66+
67+
# Without --overwrite: left alone
68+
res_no = cli._copy_examples()
69+
assert edited.read_text(encoding="utf-8") == "MY EDITS"
70+
assert res_no["overwritten"] == 0
71+
assert res_no["already_present"] >= 1
72+
73+
# With overwrite=True: replaced
74+
res_yes = cli._copy_examples(overwrite=True)
75+
assert edited.read_text(encoding="utf-8") != "MY EDITS"
76+
assert res_yes["overwritten"] >= 1
77+
assert res_yes["copied"] == 0
78+
79+
80+
# ---------------------------------------------------------------------------
81+
# Wizard output: honest report when Yes but nothing to do
82+
# ---------------------------------------------------------------------------
83+
84+
class TestWizardExampleReport:
85+
def _run_setup(self, args_extra: list[str] | None = None):
86+
args = [
87+
"setup", "--non-interactive",
88+
"--provider", "anthropic", "--api-key", "sk-ant-x",
89+
"--skip-validation",
90+
] + (args_extra or [])
91+
return runner.invoke(cli.app, args)
92+
93+
def test_first_run_says_copied_N(self, ws: Path):
94+
result = self._run_setup()
95+
assert result.exit_code == 0, result.stdout
96+
assert "copied" in result.stdout
97+
# Should name at least one new file
98+
assert "new" in result.stdout
99+
100+
def test_second_run_says_already_present(self, ws: Path):
101+
self._run_setup()
102+
result = self._run_setup()
103+
assert result.exit_code == 0
104+
# Must NOT mislead with "copied 0"
105+
assert "copied 0" not in result.stdout
106+
# Must explain the real state
107+
assert "already in your workspace" in result.stdout
108+
# Must point to the escape hatch
109+
assert "--overwrite-examples" in result.stdout
110+
111+
def test_overwrite_flag_replaces_and_reports(self, ws: Path):
112+
self._run_setup()
113+
# Mutate one example
114+
p = paths()
115+
(p.raw / "example-source.md").write_text("MINE\n", encoding="utf-8")
116+
result = self._run_setup(["--overwrite-examples"])
117+
assert result.exit_code == 0, result.stdout
118+
assert "overwrote" in result.stdout
119+
# And the content was actually replaced
120+
content = (p.raw / "example-source.md").read_text(encoding="utf-8")
121+
assert content != "MINE\n"
122+
123+
def test_skip_examples_still_works(self, ws: Path):
124+
result = self._run_setup(["--skip-examples"])
125+
assert result.exit_code == 0
126+
assert "skipped examples" in result.stdout.lower()
127+
# Did not copy
128+
assert not (ws / "research" / "raw" / "example-source.md").exists()
129+
130+
def test_setup_help_documents_overwrite_flag(self, ws: Path):
131+
result = runner.invoke(cli.app, ["setup", "--help"])
132+
assert result.exit_code == 0
133+
assert "--overwrite-examples" in result.stdout
134+
135+
136+
# ---------------------------------------------------------------------------
137+
# Regression: wizard must never claim "copied 0" when Yes was answered
138+
# ---------------------------------------------------------------------------
139+
140+
def test_yes_with_existing_examples_never_says_copied_zero(ws: Path):
141+
# First run populates the examples
142+
runner.invoke(cli.app, [
143+
"setup", "--non-interactive",
144+
"--provider", "anthropic", "--api-key", "sk-ant-x",
145+
"--skip-validation",
146+
])
147+
# Second run: user answers "Yes" to copy (via non-interactive default)
148+
result = runner.invoke(cli.app, [
149+
"setup", "--non-interactive",
150+
"--provider", "anthropic", "--api-key", "sk-ant-x",
151+
"--skip-validation",
152+
])
153+
assert result.exit_code == 0
154+
assert "copied 0 example" not in result.stdout # the bug that triggered this fix

tests/test_wizard.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,9 @@ def _patch_questionary(monkeypatch, script: list[Any]) -> FakeQuestionary:
107107
def _no_real_calls(monkeypatch):
108108
"""Neutralise validation + example copying so tests run offline and fast."""
109109
monkeypatch.setattr(cli, "_validate_key", lambda *a, **kw: (True, "ok"))
110-
monkeypatch.setattr(cli, "_copy_examples", lambda: 0)
110+
monkeypatch.setattr(cli, "_copy_examples", lambda **kw: {
111+
"copied": 0, "overwritten": 0, "already_present": 0, "source_missing": 0,
112+
})
111113

112114

113115
# ---------------------------------------------------------------------------
@@ -280,7 +282,9 @@ def test_ctrl_c_writes_nothing(workspace: Path, monkeypatch):
280282
def test_validation_failure_then_save_anyway(workspace: Path, monkeypatch):
281283
"""Validation fails — user picks 'save anyway'."""
282284
monkeypatch.setattr(cli, "_validate_key", lambda *a, **kw: (False, "401 invalid key"))
283-
monkeypatch.setattr(cli, "_copy_examples", lambda: 0)
285+
monkeypatch.setattr(cli, "_copy_examples", lambda **kw: {
286+
"copied": 0, "overwritten": 0, "already_present": 0, "source_missing": 0,
287+
})
284288
_patch_questionary(monkeypatch, [
285289
"anthropic",
286290
"sk-ant-bad", # key

tests/test_wizard_key_source.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,9 @@ def ws(tmp_path: Path, monkeypatch):
8282
):
8383
monkeypatch.delenv(v, raising=False)
8484
monkeypatch.setattr(cli, "_validate_key", lambda *a, **kw: (True, "ok"))
85-
monkeypatch.setattr(cli, "_copy_examples", lambda: 0)
85+
monkeypatch.setattr(cli, "_copy_examples", lambda **kw: {
86+
"copied": 0, "overwritten": 0, "already_present": 0, "source_missing": 0,
87+
})
8688
return tmp_path
8789

8890

0 commit comments

Comments
 (0)