docs: lead the integration story with the engine library - #157
Merged
Conversation
The docs presented five integration surfaces as peers, which understated the one that is actually the contract. `amplifier_agent_lib` is the engine; the CLI, the HTTP face, and both wrapper SDKs are adapters over it. Weighting them equally led Python hosts to spawn a subprocess when they could have imported the library, and left the in-process path documented in five lines with no code. INTEGRATION.md and skills/amplifier-agent/SKILL.md now open with a complete, runnable embedding and reframe the wrappers as what you reach for when your host cannot import Python in-process. Install becomes one shared section, since a single distribution ships the library and the binary together, and the binary is presented as the setup and diagnostics surface (doctor, auth set, models list) rather than the runtime path. Checklists and error codes are split by surface so an embedder is not reading binary_not_found and exit codes as their own concern. spec/engine-api.md was not followable end to end: it documented `Engine` but never named `make_turn_handler`, so a reader following it exactly could not construct one. It now carries the turn assembly sequence and names the two symbols on that path that sit outside the lib's public surface. Three claims in it were also stale and are corrected against observed behavior: - `TurnSubmitResult` carries eight keys, not three. The five usage fields landed with per-turn usage accounting. - `bundle_override` does not decide which bundle serves the turn; the `make_turn_handler` closure does. Passing it avoids a redundant re-prepare inside `boot()`, which is why the CLI passes it in production. - Child-to-parent cost bridging is implemented and no longer a limitation. The "no second session-factory path" non-goal is narrowed to embedders, since amplifier_agent_http reaches the runtime through a private path of its own. Also documents behavior that embedders hit and had nowhere to read about: continuity is a function of session id and workspace rather than object lifetime (a second submit_turn on one booted Engine succeeds but does not see the first turn), clearing mount_plan["providers"] before injection is load-bearing, CliApprovalSystem() with no arguments declines, and importing the library overwrites AMPLIFIER_HOME in os.environ. README, ARCHITECTURE, LAYERS_AND_RELEASES, SPEC, and ECOSYSTEM are updated only where they contradicted the above. Both published code samples were executed verbatim against a fresh install from git and returned a real agent reply. Scope of impact: docs-only. No engine, wrapper, or protocol change. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
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
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.
Why
The docs presented five integration surfaces as peers, which understated the one that is actually the contract.
amplifier_agent_libis the engine; the CLI, the HTTP face, and both wrapper SDKs are adapters over it.Weighting them equally has a measurable consequence: a Python host reading these docs reaches for a subprocess when it could have imported the library. The in-process path was documented in five lines with no code, while the wire protocol got 47 lines with full argv and envelope tables, and all seven checklist items were subprocess-only.
What changed
docs/INTEGRATION.mdandskills/amplifier-agent/SKILL.mdnow open with a complete, runnable embedding, then reframe the wrappers as what you reach for when your host cannot import Python in-process.doctor,auth set,models list) rather than the runtime path.binary_not_foundand exit codes as their own concern.enumerate_resolvable_providers()rather than left implicit.docs/spec/engine-api.mdwas not followable end to end. It documentedEnginebut never namedmake_turn_handler, so a reader following it exactly could not construct one. It now carries the turn assembly sequence and names the two symbols on that path that sit outside the lib's public surface.Three claims in it were stale and are corrected against observed behavior:
TurnSubmitResultcarries exactlyreply,turnId,sessionId"bundle_overrideexists for tests; production callers leave itNone"make_turn_handlerclosure does. Passing it avoids a redundant re-prepare insideboot(), which is why the CLI passes it in production.The "no second session-factory path" non-goal is narrowed to embedders, since
amplifier_agent_httpreaches the runtime through a private path of its own.Newly documented behavior that embedders hit and had nowhere to read about:
submit_turnon one bootedEnginesucceeds but does not see the first turn.mount_plan["providers"]before injection is load-bearing.inject_provideris a no-op when a provider is already mounted, so skipping it silently discards the injection.CliApprovalSystem()with no arguments declines everything.AMPLIFIER_HOMEinos.environ.costUsdis aDecimal, sojson.dumpson the result needsdefault=str.README.md,docs/ARCHITECTURE.md,docs/LAYERS_AND_RELEASES.md,docs/SPEC.md,docs/ECOSYSTEM.mdare updated only where they contradicted the above.SPEC.mdmovesengine-api.mdfrom "The protocol" into "Surfaces callers drive".Verification
Both published code samples were extracted back out of the finished docs and executed verbatim against a fresh install from git, each returning a real agent reply:
The multi-turn and resume claims were tested rather than assumed: two
submit_turncalls on oneEngineboth return, but turn two does not recall turn one; a freshEngineper turn withis_resumed=Trueand the same session id does.make verifypasses clean (verify: ALL GATES PASSED).Scope of impact
Docs-only. No engine, wrapper, or protocol change. No CHANGELOG entry, matching the convention for docs-only changes in this repo.
Follow-ups, not addressed here
Two defects surfaced while verifying and are left for separate changes:
foundation_home.bind()setsos.environ["AMPLIFIER_HOME"]unconditionally, discarding a value the host already set.setdefaultwould fix it. Documented as a footgun in the meantime.geminiprovider passes credential detection and reaches the LLM, then crashes insession_store.py:52withTypeError: Object of type SimpleNamespace is not JSON serializable. This is why the docs advise choosing fromenumerate_resolvable_providers()against your own preference order rather than taking the first entry.