refactor: standardize dependencies and experiment structure across chapters - #425
Draft
whanyu1212 wants to merge 122 commits into
Draft
refactor: standardize dependencies and experiment structure across chapters#425whanyu1212 wants to merge 122 commits into
whanyu1212 wants to merge 122 commits into
Conversation
The repo declares its Python dependencies across 87 separate
requirements.txt files, repeating a small core (openai: 131
declarations, python-dotenv: 114, requests: 50) almost everywhere while
heavy stacks (torch, vllm, unsloth) sit in chapters 7-9. There is no
single install path, and no way to install one chapter without either
hunting per project or pulling GPU dependencies you do not need.
Add a root pyproject.toml declaring the shared core once, with
capability groups (viz, docs, web, serve, rag, mcp, providers, tokens,
analysis, train, audio, dev) and thin per-chapter aggregates:
pip install -e ".[ch1]" # 49 packages, no GPU stack
pip install -e ".[ch7]" # training deps, opt in explicitly
Groups are derived from an audit of the per-project requirements files.
Vendored third-party trees (AWorld, browser-use, tau_bench) are excluded
-- they carry their own requirements.
This change is purely additive. No existing requirements.txt is
modified, so `pip install -r requirements.txt` keeps working everywhere
and in-flight PRs are unaffected.
All 11 chapter aggregates verified to resolve in a clean venv.
An editable install (`pip install -e .`) generates agentbook.egg-info/ at the repo root. Ignore it along with the usual build/, dist/ and .eggs/ output so it is never committed by accident.
Addresses review feedback on the initial pyproject.toml. The capability groups were derived from the union of per-project requirements.txt files, which missed packages that experiments import directly. Rebuilt them from an audit of actual imports across all non-vendored chapter code. License: - Declare Apache-2.0 to match the repository LICENSE (was MIT). Dependencies: - Keep PyPDF2 alongside pypdf: chapter1/context/agent.py, chapter2/local_llm_serving/tools.py and chapter4/perception-tools still `import PyPDF2`, which pypdf does not provide. - New `media` group (pillow, opencv-python) for chapter 4 perception tools and chapter 5 paper-to-ppt/video. - New `browser` group (browser-use) for chapter 4 collaboration tools and chapter 8 RPA. - New `mem` group (mem0ai, memobase) for chapter 3 memory experiments. - New `torch` group (torch, transformers) for local inference without the full fine-tuning stack, e.g. chapter 2 attention visualization. - New `unsloth` group, split from `train` so a CPU-only or macOS reader can still install `train`. - Add sympy to `analysis`; snac to `audio`; FlagEmbedding, annoy, hnswlib, jieba, networkx and umap-learn to `rag`. Aggregates updated accordingly: ch2 gains torch, ch3 gains mem, ch4/ch5 gain media, ch4/ch8 gain browser, ch8 gains train. All 11 chapter aggregates plus the `unsloth` extra verified to resolve in a clean venv, and every module named in review confirmed reachable from its chapter's install.
Second round of review feedback. Replaced the ad-hoc grep with an AST audit that walks every non-vendored chapter module and resolves import names to distribution names, so transitive imports through local modules are caught too (e.g. perception-tools/src/main.py -> arxiv_enhanced -> arxiv). New groups: - `integrations`: arxiv, wikipedia, waybackpy, yfinance, youtube-transcript-api, yt-dlp, python-chess, notion-client, sendgrid, aiosmtplib, PyGithub, google-api-python-client, google-auth-oauthlib, psutil, langchain-openai -- chapter 4 tool implementations. - `orchestration`: langchain-core, langchain-openai, slack-sdk -- chapter 8 self-evolution experiments. - `solvers`: python-constraint -- chapter 5 code-for-logic. - `vllm`: opt-in, Linux/GPU only. Needed by chapter2/local_llm_serving and the lazy import in chapter8/prompt-distillation/create_data.py. Additions to existing groups: - viz: colorama (chapter 2 context-compression entry points). - analysis: numba (chapter6/elo-leaderboard/optimized_elo.py). - rag: aiofiles, colorlog, loguru, markdown, PyYAML, huggingface-hub (chapter 3 structured-index and document processing). Aggregates: ch4 gains integrations, ch5 gains solvers, ch6 gains providers (anthropic in llm_judge), ch8 gains orchestration. Verified by cross-checking every audited import against a clean-venv resolve of each aggregate: all 10 chapters covered. Platform-specific stacks (vllm, unsloth) stay opt-in so no chapter install fails on macOS or CPU-only machines; test tooling stays in `dev` so running an experiment does not pull a test stack.
Third round of review feedback. - Raise the bitsandbytes floor to >=0.45. The previous >=0.43 allowed resolving to a release with no macOS wheel, so `.[ch7]` and `.[ch8]` could fail on Apple Silicon despite `train` being described as portable. - `ch3` now selects `providers`. chapter3/multimodal-agent/agent.py runs `from google import genai` unconditionally, and google-genai was only reachable transitively, which is not guaranteed on a clean resolve. - Declare google-auth explicitly in `integrations`. chapter 4's execution and perception tools import google.oauth2 / google.auth directly; it previously arrived only via google-auth-oauthlib. Also fixed the audit script to resolve namespace-package imports (`from google import genai` -> google-genai) rather than dropping them, which is why the ch3 gap was missed in the previous round.
Fourth round of review feedback, both concerning chapter 7's MultilingualReasoning experiment. - Add `trackio` to `train`. gpt_oss_20b_sft.py sets SFTConfig(report_to="trackio"), so the backend is requested by name at runtime and never imported directly -- invisible to an import audit, but Transformers fails when it initializes the reporting integration. - Floor `transformers>=4.55` in `train`. The same module imports transformers.Mxfp4Config, added in 4.55. A clean resolve happened to pick a newer release, which masked the gap; installing into an environment that already had transformers 4.40-4.54 would satisfy the `torch` extra's >=4.40 and then fail at import. Also document a constraint this exposed: chapter7/sesame pins transformers==4.52.3, which cannot coexist with the >=4.55 floor. No single aggregate can satisfy both, so that experiment needs its own venv and requirements.txt; noted inline above the ch7 aggregate.
Fifth round of review feedback, plus a sweep for the same class of problem. Review finding: `rich` and `tenacity` are imported by 26 non-vendored modules (25 in chapter 3, 1 in chapter 2) but were declared nowhere. They installed only because another dependency happened to pull them, which a later clean resolve could change. Both added to `viz`. Swept for the rest of the class by diffing every audited import against the expanded extras graph rather than against a resolved environment, which surfaced five more: - pydantic -> core (used by chapters 3, 4, 5 and 8) - httpx, aiohttp, anyio -> `web` - python-dateutil -> `orchestration` Verified by expanding the full extras graph per chapter, so coverage is now checked against what the manifest declares rather than what a particular machine's resolver produced. All 10 chapters covered and all 11 aggregates still resolve.
browser-use declares requires-python >=3.11 (confirmed on PyPI for 0.13.6 and in the vendored copy at chapter8/browser-use-rpa/browser-use/pyproject.toml:7). It is pulled in by the `browser` extra, which ch4, ch8 and `all` select, so those documented install paths could not resolve on the 3.10 interpreter the manifest advertised as supported. Raise the floor to >=3.11 and move the ruff target to py311 to match. Checked the other pinned third-party floors while here (unsloth, vllm, chromadb, mem0ai, trackio, numba); none requires above 3.11, so 3.11 is the correct floor rather than an arbitrary bump.
The unanchored build/ and dist/ patterns matched directories at any depth, including the vendored web bundle at chapter8/gaia-experience/AWorld/aworld/cmd/web/webui/dist/, which has 12 tracked files. A frontend rebuild or vendor update would silently drop newly generated assets from commits. Anchor all four patterns to the root, and narrow the egg-info glob to the package this repo actually builds. Verified: vendored dist/ and build/ paths are no longer matched, the root artifacts are still ignored, and no currently tracked file becomes ignored.
Reverts the blanket requires-python >=3.11 from the previous commit, which over-corrected: it rejected every install on 3.10, including .[ch1], .[ch2], .[ch9] and .[ch10], none of which need a browser. chapter2/local_llm_serving, chapter7/orpheus and chapter8/prompt-distillation all document Python 3.10+. Scope the two genuinely 3.11-only packages with environment markers instead: - browser-use (requires-python >=3.11), used by the `browser` extra - memobase (requires-python >=3.11), used by the `mem` extra On 3.10 those two are skipped and the rest of the aggregate installs; on 3.11+ both are included as before. Checked every pinned package against PyPI for a >3.10 floor. Only these two lack any 3.10-compatible release -- matplotlib, numpy, pandas, scipy, scikit-learn and networkx publish newer versions requiring 3.11+ but pip backtracks to older compatible builds. Verified on real interpreters, not just the 3.12 used previously: all 11 aggregates resolve on both Python 3.10.4 and 3.12.2, with browser-use and memobase present on 3.12 and correctly absent on 3.10.
Add uv.lock and [tool.uv] so readers get reproducible installs, and so
verifying the dependency graph stops being the bottleneck it was during
review: uv resolves all 530 packages across three platforms in ~4s,
where the equivalent pip dry-run sweep took roughly 40 minutes.
Declare the resolution matrix explicitly rather than letting uv infer it
from whichever machine runs `uv lock`:
environments = [
"sys_platform == 'darwin' and platform_machine == 'arm64'",
"sys_platform == 'linux' and platform_machine == 'x86_64'",
"sys_platform == 'win32'",
]
Without this the lock would encode one platform's resolution and break
every other reader -- the specific risk of committing a lockfile at all.
pip is unaffected and remains documented for readers without uv; it
resolves fresh instead of reading the lock. requires-python stays >=3.10
and the browser-use / memobase markers are preserved in the lock.
Verified after locking, on real interpreters:
- uv, Python 3.10.4: all 11 aggregates resolve, browser-use and memobase
correctly excluded by marker
- uv, Python 3.12.2: all 11 resolve with both packages present
- pip, Python 3.10.4: unchanged, still resolves
Review flagged two floors that were satisfiable by releases predating the APIs the code calls: - openai>=1.35 -> >=1.68. Four chapter 8 experiments call client.responses.create, added in 1.68. - trl>=0.9 -> >=0.22.2. chapter7/MultilingualReasoning uses SFTConfig(max_length=...) and SFTTrainer(processing_class=...). Both are the same class as the earlier transformers>=4.55 fix: a floor low enough to install but too low to run, which fails at runtime rather than at install time and only when the environment already holds an older release. Rather than wait for these to surface one at a time, diffed every declared floor against the highest floor any per-project requirements.txt asks for. That found 36 more, now aligned -- including datasets 2.19->3.4.1, peft 0.10->0.17.0, huggingface-hub 0.23->0.34.0, anthropic 0.25->0.40.0 and ollama 0.2->0.5.1. Two deliberate exceptions: transformers stays at >=4.55 (the justified floor; chapter7/orpheus's ==4.55.4 pin is an exact pin, not a floor), and python-chess keeps >=1.10 because the repo's "1.999" is a sentinel rather than a real version. Verified after re-locking: all 11 aggregates resolve under uv on Python 3.10.4 and 3.12.2, and pip still resolves unchanged.
chore(deps): add root pyproject.toml with optional-dependency groups
resolve_llm_backend() was defined three times with near-identical bodies
(chapter1/context/config.py, chapter1/web-search-agent/config.py,
chapter1/learning-from-experience/llm_agent.py), and each project carried
its own provider -> base_url table. Adding one provider meant editing
several files and keeping them consistent by hand.
Move it to agentbook/providers.py: one PROVIDERS registry, one resolver.
Adding a provider is now a single entry.
Fixes drift found while unifying: context/config.py mapped deepseek-* to
deepseek/<id> for OpenRouter, while the other two copies silently fell
through to the catch-all default model. The shared version keeps the
correct mapping.
Adds three providers, so a reader without paid API access can run the
chapters:
- ollama -- fully local, no key, no cost
- openai -- native OpenAI, which the repo previously could not reach
despite using the openai SDK for every provider
- gemini -- OpenAI-compatible endpoint with a free tier
Also adds a root .env.example. python-dotenv searches upward, so one
.env at the root serves every chapter; the template leads with the two
zero-cost setups (OpenRouter ':free' models, or Ollama).
The three call sites import from agentbook with a path fallback, so each
experiment still runs standalone from a checkout where the package is
not installed. resolve_llm_backend is kept as a shim for compatibility.
Verified: 25 new tests covering resolution order, key precedence, the
gpt-5 OpenRouter routing rule and the deepseek regression; the 32
existing web-search-agent tests still pass; all three modules import
from their own directory and resolve via agentbook.providers.
The chapter CLIs write result files next to the script by default (task_result_<mode>.json from `main.py --mode single`, plus the ablation report/plot/JSON). They are per-run artifacts, not source, so running an experiment should not leave the tree dirty.
Review feedback on the shared registry. 1. The new providers were registered but unreachable. chapter1/context kept hardcoded provider lists in main.py (argparse choices and the interactive command) and its own provider -> base_url table in agent.py, so `--provider ollama` was rejected by argparse before the registry was ever consulted. All three now derive from the registry: main.py uses SUPPORTED_PROVIDERS, and agent.py calls resolve_backend instead of rebuilding the table. Adding a provider is now genuinely one entry. 2. Selecting openrouter directly returned the model id unchanged, so a bare override such as gpt-4o or claude-sonnet-4 was sent without its provider namespace. map_model_to_openrouter now applies whenever the resolved backend is OpenRouter, not only on the fallback path. 3. chapter1/web-search-agent never called load_dotenv, so the root .env.example claim that one file serves every chapter was false for that CLI. It now loads it, and all four chapter 1 projects read the root .env. Verified: 30 registry tests (5 new, covering SUPPORTED_PROVIDERS and direct-OpenRouter id mapping) plus the 32 existing web-search-agent tests; `--provider ollama` is accepted and the CLI lists all nine providers; chapter 1's context agent still runs end to end against a free model (2 tool calls, correct conversions); web-search-agent reads the root .env.
Follow-up review found the previous commit only fixed argparse choices,
leaving two hardcoded dispatch chains further down main.py that still
handled the old provider set:
- The CLI key-selection chain exited with "Unknown provider" for ollama,
openai, gemini, ark and google, so those choices were accepted by
argparse and then rejected before reaching the resolver.
- The interactive `provider <name>` handler assigned new_api_key only for
the old set, so switching to ollama/openai/gemini/openrouter/ark/google
raised UnboundLocalError, or after an earlier successful switch reused
that provider's stale key against the new endpoint.
Both now call resolve_backend, which already knows each provider's key
variables, the no-key case (ollama) and the OpenRouter fallback.
Also routed Config.get_api_key and Config.get_default_model through the
registry rather than leaving two more copies of the same table, and
exported canonical_provider so alias resolution has one implementation.
Verified: --provider ollama/openai/gemini/ark/google all reach the
resolver ("Using provider: ..." rather than "Unknown provider");
interactive switching to all four previously-crashing providers reports
success with no UnboundLocalError; 30 registry tests and the 32 existing
web-search-agent tests pass; ruff clean; a live free-model run still
completes with a correct tool call.
Two more review findings. 1. Switching providers in interactive mode passed the resolved backend's api_key to ContextAwareAgent, which resolves again. On the fallback path that value is the OpenRouter key, so re-resolution treated it as the provider's own: switching to gemini built a client against Google's endpoint holding an OpenRouter key, failing authentication on the next request. Pass an empty key when the backend fell back, so the agent re-resolves down the same path. The CLI path already did this. 2. The `providers` command still printed descriptions only for the old provider set, silently omitting openrouter, openai, gemini, ollama, ark and google even though they were selectable. It now renders from the registry, showing each provider's default model and whether its key is configured. Added a regression test asserting a fallback backend's key must not be fed back into resolve_backend, since that is the subtle half of finding 1. Verified: `providers` lists all 12 accepted names with key status; 31 registry tests and the 32 web-search-agent tests pass; ruff clean; a live free-model run still completes with a correct tool call.
Two more places still assumed the old provider set.
- Config.validate() called get_api_key() and treated an empty result as
failure, so validate("ollama") returned False even though ollama needs
no key. It now calls resolve_backend(), which accounts for keyless
providers and the OpenRouter fallback, and whose error already names
the variables to set.
- The interactive `status` command printed no API-key line for openai,
gemini, openrouter, ollama, ark or google. It now renders from the
registry, reports "not required" for local runtimes, and notes when a
missing provider key is being covered by OPENROUTER_API_KEY.
Verified: Config.validate returns True for ollama/openrouter/gemini/kimi;
status reports a key line for every provider; 32 registry tests and the
32 web-search-agent tests pass; ruff clean.
resolve_backend("openrouter", model="gpt-5...", api_key="explicit") took
the gpt-5 reroute, which always read OPENROUTER_API_KEY from the
environment and discarded the explicit credential. That contradicts the
CLI's --api-key precedence and could authenticate against the wrong
OpenRouter account.
via_openrouter now prefers the explicit key when the selected provider is
openrouter itself, and the gpt-5 branch triggers on an explicit key even
with no environment variable set.
Deliberately narrow: for any other provider the explicit key belongs to
that provider, not to OpenRouter, so it is still not forwarded. A doubao
key rerouted to OpenRouter continues to use OPENROUTER_API_KEY.
Verified with three tests covering explicit-wins-over-env,
explicit-without-env, and the non-forwarding case; 35 registry tests
pass; ruff clean; a live free-model run is unaffected.
refactor(agentbook): unify provider resolution into a shared registry
providers.py had grown to 320 lines carrying four independent responsibilities: the provider table, OpenRouter model mapping, the resolution precedence rules, and a back-compat shim. Each changes for a different reason, so each is now its own module: models.py Provider / Backend dataclasses openrouter.py OpenRouter constants and model-id mapping registry.py the PROVIDERS table, aliases, name lookup resolution.py the precedence rules legacy.py the pre-registry resolve_llm_backend shim Modules import only downward, so there are no cycles. The shim previously reimplemented the gpt-5 / primary-key / fallback chain rather than sharing it -- the same drift risk the registry was introduced to remove. It could not share the logic because the OpenRouter construction was a closure over five locals inside resolve_backend. Lifting that closure to build_openrouter_backend() lets both paths call one implementation. __init__.py re-exports the full public surface, so every existing import keeps working untouched; no call site changed. Verified by a differential harness comparing old against new over 19,652 input combinations across providers, models, explicit keys and environment shapes -- the only difference is __all__ gaining the two OpenRouter constants it already exposed as attributes. Docstrings converted to Google style throughout.
Adversarial review of the package split found that registry.py's promise
-- adding a provider means adding one entry and nothing else -- was not
true, and proved it by adding a second aggregator.
Aggregator behaviour was gated on the literal `spec.name == "openrouter"`
in two places, so a second aggregator such as Together.ai silently sent
un-namespaced model ids ("gpt-4o" rather than "openai/gpt-4o") and failed
at request time rather than in configuration. The same literal meant an
explicit key for that aggregator was never recognised as its credential.
Both now key off a `namespaces_models` field on Provider, so the resolver
is closed to modification while the registry stays open to extension.
The aggregator branch had also lost the placeholder-key fallback the
original applied to every provider, so a keyless aggregator resolved to
an empty api_key that the OpenAI client rejects. Restored inside
build_openrouter_backend so both call paths are covered.
Also from the review:
* legacy.py's docstring named two of its three dependent chapters, and
omitted chapter1/context/config.py, which re-exports the shim without
calling it -- deleting the function on that advice would have broken
chapter1/context at import time.
* The zero-cost guidance sentence was duplicated across two error
messages; it is now ZERO_COST_HINT, defined once.
* _PLACEHOLDER_KEY was the string "ollama", making a resolved backend's
api_key equal to its provider name and indistinguishable from a real
user-set key. It is now "not-needed", which is inert.
* SUPPORTED_PROVIDERS is an import-time snapshot; supported_providers()
recomputes for callers that mutate PROVIDERS.
* Backend.__iter__ documented Yields:, but it returns an iterator rather
than being a generator.
Five regression tests cover the extension path the review exercised. The
differential harness reports 19,584 checks with every difference from the
pre-split module accounted for: 834 placeholder-key, 432 unified error
wording, 0 unexplained.
Codex review of #3 found that `namespaces_models` had been given three jobs when it should have had one. Introduced to remove a hardcoded provider name from the resolver, the flag ended up standing in for "is OpenRouter" -- so it dragged OpenRouter's endpoint and credentials along with its model-id format. Two consequences, both reproduced before fixing: * A second aggregator resolved to https://openrouter.ai/api/v1 with `using_openrouter=True`, sending its own credential to the wrong host. * Its key populated `explicit_openrouter_key`, so a gpt-5 request without OPENROUTER_API_KEY rerouted to OpenRouter and authenticated with a credential OpenRouter has never seen. `namespaces_models` now governs model-id mapping and nothing else; the fallback endpoint and credential stay tied to the openrouter entry via `_OPENROUTER`. Its docstring says so explicitly, since the field name alone invites exactly the conflation that produced this bug. The existing regression test asserted only the mapped model and the key, which is why it passed while the endpoint was wrong -- a test written from the same misunderstanding as the code. It now pins base_url and using_openrouter too, with two further cases covering the gpt-5 reroute for a non-OpenRouter aggregator and for OpenRouter itself. Also widens the test workflow, which fired only on chapter1/web-search-agent/** and so ran nothing at all for this PR even though those tests import the code it changes. It now also triggers on agentbook/**, tests/** and pyproject.toml, and a second job runs the registry suite at the repository root. Verified by reproducing the job in a clean venv: pip install -e ".[dev]" then pytest tests, 42 passed. Differential harness re-run: 19,584 checks, 0 unexplained differences.
chapter1/web-search-agent/tests is formatted with black at its default 88 columns and CI checks that, but the directory was not excluded from the root ruff config, which sets 100. Running ruff format over it -- easy to do from the repository root -- silently reflows those files and turns the black check red. extend-exclude alone does not prevent this: ruff applies exclusions when walking a directory but honours paths named explicitly on the command line, which is exactly how the mistake happens (and how pre-commit passes filenames). force-exclude makes the exclusion apply either way. Verified by replaying the failure: `ruff format chapter1/web-search-agent/tests` now reports no files and leaves the tree untouched, while agentbook/ and tests/ stay fully formatted and linted.
ff60a6f put chapter1/web-search-agent/tests in extend-exclude, which is global: it stopped ruff format reflowing those files past black's 88 columns, but it also ended `ruff check` coverage for the directory. That was silent -- nothing turns red when linting simply stops. Moved to [tool.ruff.format].exclude, which applies to the formatter only. The trailing /* is load-bearing: a bare directory name is not matched there, unlike in the global list, so "chapter1/web-search-agent/tests" alone leaves the files formatted after all. Verified all four properties, since the previous attempt satisfied only two of them: format, discovered from the repo root -> skipped format, path named on the command line -> skipped ruff check -> covers the directory again black --check -> unchanged, CI stays green Restoring lint coverage surfaced one real finding (I001, unsorted imports in tests/test_config.py). Fixed here; black accepts the result.
Mutation testing the suite -- reintroducing each fixed bug and checking that something goes red -- found one survivor: deleting the `or _PLACEHOLDER_KEY` fallback inside build_openrouter_backend broke no test, despite the path being reachable through a keyless provider that routes to OpenRouter. test_keyless_aggregator_still_gets_a_placeholder_key was written for that line, but 591b15d moved aggregators out of the OpenRouter builder, so it now exercises the plain-provider branch instead. It still passes, just for a different reason -- the kind of drift a green suite cannot report. Covers the builder directly rather than through a provider, so the assertion no longer depends on which branch happens to route there.
refactor(agentbook): split providers.py into a package
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Author
|
Checkpoint: dependency-resolution stabilization merged into the fork.
Validation:
Current upstream state remains draft with |
…on-tests Move active tool selection tests under tests
Contributor
Author
|
Checkpoint: Phase 6
Validation:
Current upstream state remains draft with |
Move async agent tests under tests
Contributor
Author
|
Checkpoint: Phase 6
Validation:
Current upstream state remains draft with |
Move event trigger tests under tests
Contributor
Author
|
Checkpoint: Phase 6
Validation:
Current upstream state remains draft with |
…-tests Move cost analysis tests under tests
Contributor
Author
|
Checkpoint: Phase 6
Validation:
Current upstream state remains draft with |
Contributor
Author
|
Checkpoint: upstream conflict reconciliation completed.
Validation:
Current upstream PR state: |
Keep Chapter 8 training stack isolated
Contributor
Author
|
Checkpoint: Stabilization item for Chapter 8 dependency split completed.
Validation:
Current upstream PR state: |
Contributor
Author
|
Checkpoint after upstream sync:
|
Move TTS quality eval tests under tests
Move public health eval tests under tests
Owner
|
Thanks @whanyu1212 for the tremendous amount of work on this PR! The progress is impressive—dependency resolution now passes on Python 3.10 and 3.12, the Chapter 8 training stack is isolated, upstream conflicts have been reconciled, and many experiment layouts and tests have been standardized. This is a substantial cleanup that should make the repository much easier to install, understand, and maintain. It looks like the remaining work has narrowed considerably, mainly to resolving the GitGuardian historical finding, syncing with the latest I especially appreciate the incremental checkpoints and detailed documentation throughout this large effort. Great work—this is getting very close! |
Move Intuitor tests under tests
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Status
Tracks #421.
Current fork head:
aea87ef.Current check state:
.env.exampleplaceholder introduced in81161fa; current tracked content has already been cleaned.DIRTY; upstream conflict reconciliation is required before final review.ch3,ch8, andallbecause the lock selects anonnxruntimerelease without a Python 3.10 artifact.The detailed implementation history previously kept in this body is archived in this PR comment.
Goal
Make every chapter experiment:
The chapter experiments remain independent educational projects. Shared plumbing belongs in
agentbook/; teaching logic remains underchapter1/throughchapter10/.Architecture Boundary
agentbook/is intentionally a small shared package, not a framework containing all experiments.Good shared-package candidates:
Experiment-local responsibilities:
Completed Foundation
chapter1/contextexperiment-layout pilot.docs/EXPERIMENT_CONVENTIONS.mdfor entrypoints, tests, fixtures, outputs, and validation.Completed Phase 6 layout migrations:
chapter1/learning-from-experiencechapter1/search-codegenchapter2/agent-skills-pptchapter2/log-sanitizationchapter9/phone-agentchapter9/end-to-end-speechchapter9/controllable-ttschapter10/book-translationchapter10/multi-role-transferchapter10/staged-system-promptCurrent Audit Baseline
The July 2026 audit identified the remaining cleanup scope, excluding vendored third-party trees:
test_*.pyfiles across 50 experiments;src/;main.pyordemo.py;agentbook, despite broad provider/fallback duplication elsewhere.Adjusted Remaining Roadmap
Stabilization Checkpoint
ch3,ch8, andall, or explicitly revise the supported floor.prompt-distillation.ch1throughch10andall.pyproject.tomlanduv.lockchanges.Shared Plumbing Adoption
openrouter_fallback.pymodules withagentbook.providers.agentbook.model_policymodule.Phase 6A: Tests And Manual Scripts
tests/directories.tests/manual/using names outside pytest discovery.test_loader.pyrather than preserving permanent naming exceptions.Phase 6B: Canonical Entrypoints
main.pyanddemo.py.Phase 6C: Fixtures And Outputs
output/or equivalent directories..gitignorerules without suppressing deliberate media fixtures.Phase 7: Dependency Reconciliation
uv.lockafter every dependency-contract change.Phase 8: Final Maintenance And Verification
Compatibility Strategy
During migration:
Non-Goals
This work does not intend to:
agentbook/into a framework containing chapter teaching logic;tests/directories where no real tests exist;Development Checkpoints
Foundation and shared providers:
Chapter install adoption:
Layout conventions and migrations:
Detailed phase-by-phase notes are preserved in the archived implementation-history comment.
Validation Summary
Completed work has been validated incrementally with:
git diff --check;python scripts/check_i18n_consistency.py;uv lock --check;Known validation gap discovered by the latest audit:
uv lock --checkpasses, but Python 3.10 dry-runs fail forch3,ch8, andall; the final CI matrix must exercise actual target resolution rather than lock freshness alone.Review Guidance
At the current draft stage, feedback is most useful on:
Final implementation review should wait until the remaining roadmap is complete or explicitly descoped and the PR is marked ready.
Merge Readiness
This PR will remain draft until:
all.