scaffold: give every template a tested app factory - #1124
Conversation
run_text_turns() drives a whole scenario against one session and returns one TurnResult per input; run_text_turn() is now a one-input wrapper around it, keeping its signature, dispatch order and exception timing. run_scripted_audio_turn() drives one turn through the real audio pipeline (transport -> VAD -> STT -> agent -> TTS) with scripted stub I/O, so an offline test can cover audio wiring and not just text. stubs.scripted_turn_config() packages the scripted provider set that cli/console.py's voice demo hand-wrote; console now calls it and keeps its own printing, timeout and journal summary. examples/journal_demo.py deliberately keeps its explicit wiring: that wiring is what the example teaches, and tests/examples pins its literals. The inline latency lookup becomes _latency_ms(), which scans by metric name priority (text_turn_latency_ms, then turn_total_latency_ms) rather than record order, so a text turn's reported latency cannot silently change if the text path ever emits the voice metric too. No new top-level export: the names live in easycat.debug.testing.__all__ and easycat.stubs.__all__. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hYv6Zee2HKJpSnuGHiL1s
The default template built its agent at module scope and called
VoiceApp(...).run("local") at import time, so nothing could import it
without opening a microphone, and its generated tests defined a private
StubAgent that never touched agent.py. Deleting the current_time tool or
corrupting the instructions failed nothing.
agent.py now exposes AGENT_NAME/INSTRUCTIONS, make_agent() and
make_app(), and runs only under an `if __name__ == "__main__":` guard.
The SDK-free tool moves to tools.py so the generated tests can call it
directly with no agent SDK installed. tests/test_agent.py imports both,
gates the wiring assertions behind pytest.importorskip("agents"), and
covers two turns on one session, a failing tool dependency, and one
scripted audio turn. The generated pyproject.toml gains
[tool.pytest.ini_options] pythonpath = ["."], without which the
documented `uv run pytest` cannot import agent/tools at all — the repo
smoke test hid this by running `python -m pytest`, which prepends the cwd.
_render_text drops the config sentinel together with the separator that
introduced it, replacing the hard-coded three-expression comma repair,
and matches the longest indent first so the multi-line form leaves no
orphaned spaces behind.
Guards: AST import-safety and static app/test symbol cross-checks that
run with no SDK; a rendered-kwargs check that the scaffolded providers
really resolve (VoiceApp validates field names only); stub-boundary
wording in both the generated test and AGENTS.md; a seeded tool break
that must fail the generated suite; and an offline run with ambient
credentials behind a canaried outbound-network guard. test_packaging.py
now derives its expectations from the scaffold source, which also closes
a silent provider-stt/provider-tts wheel gap.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hYv6Zee2HKJpSnuGHiL1s
test_documented_canonical_voice_quickstart_shape_stays_consistent
matched only ``VoiceApp(...).run("local")`` or a name bound to a
``VoiceApp(...)``. The default scaffold now builds the app in an
importable ``make_app()``, so the receiver is a factory call; recognise
a function that returns ``VoiceApp(...)`` as the same canonical shape.
README and examples/openai_agents_voice.py keep the literal form.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hYv6Zee2HKJpSnuGHiL1s
Review round 1 on the importable default scaffold.
The generated wiring test asserted ``"$" not in AGENT_NAME``, so any
project whose agent name or instructions legitimately contained a dollar
sign ("Quote prices in $USD") failed its own suite on the user's first
``uv run pytest``. It now rejects a constant shaped like an unrendered
``$PLACEHOLDER`` instead, and a smoke case scaffolds with quotes, a
backslash and a dollar sign to keep that honest.
``test_tool_failure_surfaces_instead_of_hanging`` exercised a private
BrokenTool stub that never touched ``tools.py``. ScriptedReasoning now
calls ``tools.current_time`` through the module and the test patches the
real tool, so renaming or deleting it fails here too; the static import
guard learned the ``import tools`` style so it keeps pinning the symbol.
README and docs/testing-and-evals.md claimed shapes this PR made false:
the README now says the default scaffold builds the same VoiceApp behind
``make_app()`` and a ``__main__`` guard (and the quickstart guard pins
that sentence, with the factory relaxation scoped to the scaffold source
so README and the first example still have to teach the literal form),
and the rung-2 prose is scoped to the one migrated template.
Also: ``_render_text`` anchors the sentinel line to a line start and
reuses its captured indent, so a sentinel nested deeper than the two
depths the templates use today leaves no orphaned spaces; the netguard
canary bounds its connect and both subprocesses, so a guard that failed
to load fails the test instead of stalling the lane; and
``run_scripted_audio_turn``'s wall-clock fallback now brackets the turn
alone rather than session start-up, the drain sleep and teardown.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hYv6Zee2HKJpSnuGHiL1s
run_scripted_audio_turn's `timeout_s: float = 10.0` default is scanned by the teardown-budget ratchet, so the branch left the credential-free suite red on an unclassified inventory site. Record it as `not_teardown`: the wait bounds the offline test turn's wait for the agent reply, not any session teardown path. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hYv6Zee2HKJpSnuGHiL1s
pydantic-ai and pydantic-ai-workflow imported no framework SDK behind a
factory: agent.py built its Agent(s) and called run(...) at module scope,
so nothing could import the module without wiring a real model, and the
generated tests exercised a private StubAgent that never touched the
project's own code.
- pydantic-ai: tools.py holds current_time(); agent.py exposes
AGENT_NAME/INSTRUCTIONS constants, make_agent(), and make_config(), and
only runs under `if __name__ == "__main__":`.
- pydantic-ai-workflow: tools.py holds pick_specialist() (the SDK-free
router) and its TECH_TERMS; agent.py exposes make_specialists(),
make_workflow(), and make_config(). The router constant and function
live in tools.py, not agent.py, because agent.py imports pydantic_ai at
module scope — importing it from a test would fail with no SDK
installed, defeating the always-on half of the generated suite.
- Generated tests/test_agent.py: ScriptedReasoning stands in for the
model while the project's real router/tools and EasyCat's real text and
audio pipelines run end to end; the SDK-bound wiring assertions and a
pydantic_ai.models.test.TestModel run sit behind
pytest.importorskip("pydantic_ai"). make_config()/EasyConfig() is
deliberately not called from the offline test: unlike VoiceApp,
EasyConfig validates credentials at construction time, so calling it
with no API key set would fail every user's first `uv run pytest`.
- pyproject.toml gains `[tool.pytest.ini_options] pythonpath = ["."]` so
the documented `uv run pytest` (the console script) can `import agent`.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hYv6Zee2HKJpSnuGHiL1s
agent.py built its Agent at module scope and called asyncio.run(main())
unconditionally, so importing the module started the REPL. agent.py now
exposes AGENT_NAME/INSTRUCTIONS constants and make_agent(); the REPL moves
into chat(), run only under `if __name__ == "__main__":`.
The generated tests/test_agent.py's wiring assertions sit behind
pytest.importorskip("agents"); ScriptedReasoning drives EasyCat's real text
and audio pipelines the rest of the time. pyproject.toml gains
`[tool.pytest.ini_options] pythonpath = ["."]`.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hYv6Zee2HKJpSnuGHiL1s
twilio-phone and telnyx-phone already had a make_agent() factory that
server.py imports, but their generated tests still stubbed around it and
take_message lived inline in agent.py. take_message moves to tools.py as a
plain function (wrapped with function_tool(...) at wiring time), and
agent.py gains AGENT_NAME/INSTRUCTIONS constants so the generated test can
assert real wiring without a rendered `$` placeholder.
server.py is unchanged: it keeps importing and calling make_agent() by the
same name and signature, and it still builds EasyConfig(...)/
EasyConfig.phone(...) itself, not in agent.py — so the generated wiring
test never has to touch either file's credential-eager config construction.
Generated tests/test_agent.py: ScriptedReasoning calls tools.take_message
through the module (so breaking the real tool breaks the test too) while
EasyCat's real text and audio pipelines run end to end; the SDK-bound
wiring assertions sit behind pytest.importorskip("agents"). pyproject.toml
gains `[tool.pytest.ini_options] pythonpath = ["."]`.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hYv6Zee2HKJpSnuGHiL1s
agent.py built its Agent at module scope and called run(EasyConfig.browser(...))
unconditionally, so importing the module opened a WebRTC server. tools.py
now holds connection_help(); agent.py exposes AGENT_NAME/INSTRUCTIONS,
make_agent(), and make_config(), guarded by `if __name__ == "__main__":`.
The generated tests/test_agent.py's wiring assertions sit behind
pytest.importorskip("agents"); ScriptedReasoning drives EasyCat's real text
and audio pipelines with the project's real tool the rest of the time.
make_config() is deliberately not called from the offline test: unlike
VoiceApp, EasyConfig validates credentials at construction time, so
calling it with no API key set would fail the offline test. pyproject.toml
gains `[tool.pytest.ini_options] pythonpath = ["."]`.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hYv6Zee2HKJpSnuGHiL1s
provider, provider-stt, and provider-tts built their demo Agent at module scope and called run(EasyConfig.mic(...)) unconditionally, so importing agent.py opened a live mic session. agent.py now exposes AGENT_NAME/INSTRUCTIONS constants, make_agent(), and make_config(), guarded by `if __name__ == "__main__":`. register() stays a module-level call: it only mutates the provider registry, opens nothing, and the package's `custom_*:register` entry point must keep targeting a module-level callable — the import-safety guard now allows exactly that one call, in exactly these three templates. Generated tests/test_agent.py imports the real custom_vad/custom_stt/ custom_tts module and asserts register() makes the shortcut selectable (mirroring the template's own conformance/contract suite), plus the standard ScriptedReasoning text and audio pipeline tests. These templates have no tools.py — there is no tool to route around — and make_config() is not called from the offline test for the same reason as the other families: EasyConfig validates credentials at construction, and these templates' custom provider shortcuts additionally aren't registered with the base EasyConfig.mic() the repo-side value check uses. pyproject.toml gains a `pythonpath = ["."]` key in its existing pytest table. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hYv6Zee2HKJpSnuGHiL1s
…list Deletes _MIGRATED_TEMPLATES now that every template ships the importable make_agent()/make_app()/make_config() shape: T3-T8 and T11 reparametrize over sorted(_LINE_BUDGETS), and test_template_ships_offline_agent_tests' if/else collapses to the migrated branch (its remaining `else` branch was unreachable and is removed). - _TOOLS_MODULE_TEMPLATES / _PRIMARY_FACTORY: T5's "import tools" check and T4's static mutation guard are no longer openai-agents-specific; the latter now checks each template's actual factory name (make_agent, make_workflow, ...) instead of a hard-coded "make_agent". - test_template_entry_points_are_import_safe allows exactly one named module-scope call (register()) in the three provider* templates. - _uses_run_easyconfig_preset is replaced by _builds_easyconfig_preset_and_runs_it: the old helper required run(EasyConfig.<preset>(...)) as one expression, which no longer matches once the preset is built in make_config() and run separately under the __main__ guard. - test_rendered_app_kwargs_resolve_to_real_providers (T11) generalizes past a single VoiceApp(...) call: it now finds either a VoiceApp(...) or an EasyConfig(...)/EasyConfig.<preset>(...) call, reading server.py instead of agent.py for twilio-phone/telnyx-phone. text-chat (no audio config at all) and the three provider* templates (their custom provider strings aren't registered with the base EasyConfig.mic() this check builds) are excluded — their value-level coverage is proved elsewhere: init's own preflight, and, for provider*, the generated test's own register()-then-select assertion. - test_scaffold_offline_tests_run_without_cwd_on_sys_path (E2) and test_scaffold_offline_tests_pass_with_ambient_credentials_and_no_network (E3) parametrize over every template. A second seeded-break case, test_scaffold_offline_tests_fail_when_routing_behavior_breaks, proves A3 for a non-tool decision (pydantic-ai-workflow's pick_specialist), so a seeded tool break is not the only breakage a generated suite can catch. - test_init.py: three literal `name="..."` assertions on generated agent.py now expect the rendered `AGENT_NAME = "..."` constant instead, following the same templates' move off inline Agent(name=...) literals. - docs/testing-and-evals.md rung 2 no longer scopes the tested-app-factory pattern to the openai-agents scaffold alone. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hYv6Zee2HKJpSnuGHiL1s
Review round 1 on DX3-2.
`Agent("openai:gpt-4.1-mini", ...)` resolves the model string inside the
constructor, so PydanticAI raises `openai.OpenAIError: Missing credentials`
when `OPENAI_API_KEY` is unset or empty. Both PydanticAI templates' generated
wiring tests called `make_agent()` / `make_specialists()` with no argument, so
a user who ran `uv sync` and then the advertised key-free `uv run pytest` got
two failures. No lane in this repo installs `pydantic_ai`, so the generated
test's own `importorskip` hid it everywhere it could be observed.
`make_agent(model=MODEL)`, `make_specialists(model=MODEL)` and
`make_workflow(model=MODEL)` now take the model as an injectable argument
defaulting to a new `MODEL` constant, and the generated tests pass a
`TestModel()` in instead of overriding an already-constructed agent. That also
lets the wiring tests assert what they claim: the pydantic-ai test now pins the
registered tool and the system prompt through one deterministic run, and the
workflow's routing test gives each specialist a distinguishable reply so an
inverted `pick_specialist` or a swapped specialist key fails it.
A new repo-side guard, `test_pydantic_templates_inject_the_model_so_offline_
tests_need_no_key`, parses both templates and fails if a factory loses its
`model` parameter or a generated test calls one with no model — it runs in
`just guard-examples` with no SDK installed, which is where the original defect
was invisible.
Also in this round:
* Three `AGENTS.md` files claimed the generated test asserts `EasyConfig`
fields, which it deliberately never builds; the claim is dropped and the real
boundary (`EasyConfig` validates credentials at construction) is named.
* T11 (`test_rendered_app_kwargs_resolve_to_real_providers`) now rebuilds the
config through the template's own preset instead of a hard-coded
`EasyConfig.mic`, which restores the design's webrtc-browser requirement
that `EasyConfig.browser(...)` builds with the `webrtc` extra absent and adds
the same proof for `EasyConfig.phone(...)`.
* Six generated tests pointed the reader at "the repo-side test", which does
not exist in a scaffolded project; they now name a command the user can run.
* The seeded routing-break smoke test also asserts the pipeline test fails,
mirroring its tool-break sibling.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hYv6Zee2HKJpSnuGHiL1s
There was a problem hiding this comment.
Sorry @yisding, your pull request is larger than the review limit of 150,000 diff characters
|
Warning Review limit reachedNext included review available in 44 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (54)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28f1ffd2ac
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…-templates # Conflicts: # docs/testing-and-evals.md # tests/cli/e2e/test_scaffold_smoke.py # tests/cli/test_templates.py
|
Merged Verify after the merge (dev group + openai-agents extra): |
``run_scripted_audio_turn`` and ``scripted_turn_config`` promised "no API key and no network", but only the AUDIO stages are scripted: *agent* is passed through untouched, so a real framework agent's bridge still needs its model credential and calls the model for real — billing the call when an ambient key happens to be set. The scaffolded templates all pass a keyless ``ScriptedReasoning``, so they are genuinely offline; the docstring is what over-promised. Say which half is scripted instead of narrowing the helper's contract, which legitimately accepts a real agent when a caller wants a live turn. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YQKaDBWFS7efjXaqud8hiU
The stack's squash-merges broke this branch's ancestry, so git picked a pre-#1123 merge base. The two sides are disjoint — main carries #1124's template migration, this branch carries the unbuildable-selection report — and the tree's diff against main is byte-identical to this branch's own diff against the #1125 tip it merged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YQKaDBWFS7efjXaqud8hiU
Problem
After DX3-1, only the default
openai-agentstemplate had an importable app factory and generated tests that actually exercised it. The other nine templates still built their agent (or, forpydantic-ai-workflow, a routing dict) at module scope and calledrun(...)unconditionally, so importing any of them started a live session, and their generatedtests/test_agent.pyfiles all defined the same privateStubAgent, touching none of the project's own code: deleting a tool, corrupting instructions, or breaking a routing decision failed no shipped test.text-chatadditionally ran an unguardedasyncio.run(main())at import time.Change
tools.pyholds the SDK-free logic (current_time, and for the workflow,pick_specialist/TECH_TERMS), kept out ofagent.pybecause that file importspydantic_aiat module scope and would otherwise force the SDK onto every generated test.agent.pyexposesmake_agent()/make_specialists()/make_workflow()/make_config()behind a__main__guard.make_agent()factory; the REPL moves intochat(), run only under__main__.take_messagemoves totools.py;agent.pygainsAGENT_NAME/INSTRUCTIONSconstants.server.pyis untouched — it already importedmake_agent()by name.tools.pyholdsconnection_help;agent.pygains the same factory/guard shape.agent.pygainsmake_agent()/make_config(), guarded by__main__;register()stays a deliberate, now-explicitly-allowed module-level call (it only mutates a registry and the package's entry point must keep targeting it).tests/test_agent.pynow runs its always-on half (aScriptedReasoningstand-in exercising the project's real tools/router plus EasyCat's real text and audio pipelines) with no framework SDK installed, and gates the SDK-bound wiring assertions behindpytest.importorskip. Everypyproject.tomlgainspythonpath = ["."]so the documenteduv run pytest(console script) canimport agent/import tools.tests/cli/test_templates.py,tests/cli/e2e/test_scaffold_smoke.py,tests/cli/test_init.py) drop the_MIGRATED_TEMPLATESallow-list and reparametrize every T3–T11-style check over all ten templates; a second seeded-break test proves A3 for a non-tool decision (pydantic-ai-workflow's router); E2/E3 now run for every template.MODELconstant,make_agent(model=MODEL),make_specialists(model=MODEL),make_workflow(model=MODEL)) so generated wiring tests can pass inTestModel()instead of constructingAgent("openai:gpt-4.1-mini", ...)directly, which raisedopenai.OpenAIError: Missing credentialswith no key present. A new repo-side guard (test_pydantic_templates_inject_the_model_so_offline_tests_need_no_key) checks both templates keep that shape.T11now rebuilds config through the template's own preset (EasyConfig.mic/.browser/.phone) instead of a hard-codedEasyConfig.mic, restoring coverage thatEasyConfig.browser(...)andEasyConfig.phone(...)build with their respective extras absent. SeveralAGENTS.mdfiles and generated test docstrings were corrected to describe what the offline test actually asserts, and to point users at a real command (uv run --env-file .env python agent.py) instead of a nonexistent "repo-side test".Boundaries
This branch was stacked on dx3/importable-default-scaffold (PR #1120), which has since merged to main; this PR now targets main. It is part of the DX milestone from plan/roadmap/2026-09-05-next-level-developer-experience.md, DX3 slice DX3-2.
Out of scope: PR3 (wheel / outside-checkout run, the
generated-app-smokeCI job); any change toeasycat.debug.testingbeyond what PR1 already added; adding framework extras to the dev group or any existinguv sync; changingserver.pyauth/token logic; the provider-authoring contract tests (unchanged).Deviations from the design:
make_config()/EasyConfig(...)/EasyConfig.<preset>(...)for any template exceptopenai-agents.EasyConfig.__post_init__validates credentials eagerly (raisesEASYCAT_E203with no key and no explicit stt/tts) unlikeVoiceApp, which never inspects config at construction (per DX3-1's finding). So pydantic-ai, pydantic-ai-workflow, webrtc-browser, and the three provider* templates' generated tests only build/assert onmake_agent()/make_specialists()/make_workflow(). The "rendered kwargs resolve to a real provider" guarantee for those six templates is proven only by the repo-side test (T11, which sets ambient env vars), not by the generated project's own offline suite.pydantic-ai-workflow'sTECH_TERMSconstant and the word-matching logic moved intotools.pyrather than staying inagent.py, becauseagent.pyimportspydantic_aiat module scope and the generated test's always-on half must import zeropydantic_aito run with no SDK installed.test_rendered_app_kwargs_resolve_to_real_providers) generalizes past a literalVoiceApp(...)-only shape to also match a bareEasyConfig(...)call (needed for twilio-phone'sserver.py), and excludestext-chatand the three provider* templates (documented with a comment)._LINE_BUDGETS/_SUPPORT_FILE_BUDGETSwere set to the measured line counts of the files as written, not the design's rough estimates.Review
Agent("openai:gpt-4.1-mini", ...)directly, which raisesopenai.OpenAIError: Missing credentialswith no key set, so a fresh scaffold's key-freeuv run pytestfailed. Fixed by making the model injectable and passingTestModel()from the generated tests. One minor doc-reference issue (threeAGENTS.mdfiles pointed at a nonexistent "repo-side test") was also fixed.AGENTS.mdfiles claimed the generated test assertsEasyConfigfields it never builds (fixed by rewording), and webrtc-browser's design requirement that the wiring test provemake_config()builds with the webrtc extra absent was dropped (fixed by generalizing T11 to rebuild config through the template's own preset). Two minor plan-compliance issues (stale "repo-side test" pointers, line-budget rounding) were also fixed.tools=[current_time]orsystem_prompt=INSTRUCTIONSwere dropped frommake_agent()(fixed by asserting the tool output and system prompt viaTestModel), and (same root cause as the correctness blocker) the model-construction failure meant these tests couldn't even run without a key. Two more major issues were fixed: the workflow'stest_workflow_uses_a_deterministic_test_modelended in a bareassert result(renamed/rewritten to assert the routed reply per specialist), and one seeded-break test asserted only one failing test name instead of the full expected set. One minor fix proposal — restoringmake_config()in six generated tests viamonkeypatch— was rejected: it would violate the repo's own guard that generated tests contain noOPENAI_API_KEYstring, and it breaks for non-default provider kwargs (e.g.--stt deepgram/flux) where the generated test cannot know which provider key to fake; the design requirement it stood in for is instead met repo-side via the T11 fix above.Test evidence
No pre-existing failures observed in any lane.
🤖 Generated with Claude Code
https://claude.ai/code/session_011hYv6Zee2HKJpSnuGHiL1s
Summary by Sourcery
Make every scaffold template import-safe and ship generated tests that exercise its real application wiring and SDK-free behavior offline.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests: