-
Notifications
You must be signed in to change notification settings - Fork 9
[DOCS] Add a skill for the spec sync and regeneration pipeline #37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,118 @@ | ||
| --- | ||
| name: spec-upgrade | ||
| description: Sync the OpenAPI spec from the LLMWhisperer service repo into this repo and regenerate the transport layer. Use whenever the service adds or changes an endpoint, when the `sdk-drift` or `api-surface` CI job goes red, when `specs/llmwhisperer.json` is behind upstream, or when someone asks to "regenerate the SDK", "refresh the spec", "pick up the new endpoint", or "release llmwhisperer-client". Reach for this even when the request only mentions the generated code or a new parameter — the upgrade is an ordered pipeline, and skipping a step leaves a gate red or the spec silently stale. | ||
| --- | ||
|
|
||
| # Upgrading the spec and regenerating the client | ||
|
|
||
| Nothing tells this repo when the service's spec moves. Someone notices, and then | ||
| runs this pipeline. The steps are ordered because each one's output is the next | ||
| one's input, and because two of them are the only thing standing between a stale | ||
| copy and a client that looks generated but isn't. | ||
|
|
||
| ## The pieces | ||
|
|
||
| | Thing | Where | | ||
| |---|---| | ||
| | Vendored spec | `specs/llmwhisperer.json` | | ||
| | Generator wrapper | `tools/gen_sdk.sh` (pins the generator, records `SPEC_SOURCE_*`) | | ||
| | Generated transport | `src/unstract/llmwhisperer/_sdk_llmwhisperer/` — never hand-edited | | ||
| | Hand-written facade | `src/unstract/llmwhisperer/client_v2.py` | | ||
| | Gates | `sdk-drift` and `api-surface` jobs in `.github/workflows/ci_test.yaml` | | ||
| | Release | `.github/workflows/main.yml`, `workflow_dispatch` | | ||
|
|
||
| Upstream, the spec is produced by the service that serves these endpoints | ||
| (`Zipstack/unstract-llm-whisperer`, generated by its `tools/gen_spec.py`, | ||
| committed at `specs/llmwhisperer.json`, with its own drift workflow). It is a | ||
| copy here, not a fork. | ||
|
|
||
| ## The sequence | ||
|
|
||
| 1. **Copy the spec byte-for-byte** from the upstream commit you are upgrading to. | ||
| Do not reformat it and do not edit it here — the drift gate regenerates from | ||
| whatever is committed, so an edit here becomes a client no service serves. | ||
|
|
||
| 2. **Move `SPEC_SOURCE_REV` in `tools/gen_sdk.sh` in the same commit** (alongside | ||
| `SPEC_SOURCE_REPO` / `SPEC_SOURCE_PATH` if upstream moved the file). Without | ||
| that update nothing distinguishes a current copy from one upstream has moved | ||
| past — the script and the drift gate both report clean either way, which is | ||
| exactly the failure this pipeline exists to prevent. | ||
|
|
||
| 3. **Regenerate:** | ||
|
|
||
| ```bash | ||
| ./tools/gen_sdk.sh | ||
| ``` | ||
|
|
||
| It rebuilds the tree from scratch with a pinned generator. If it exits | ||
| non-zero on a generator warning, believe it: the generator downgrades a | ||
| schema it cannot parse to a warning, drops the endpoint or model, writes the | ||
| rest and exits 0 — so the warning is the only signal that an operation is | ||
| missing. | ||
|
|
||
| 4. **Review the generated surface, with new files included:** | ||
|
|
||
| ```bash | ||
| git add -N -- src/unstract/llmwhisperer/_sdk_llmwhisperer | ||
| git diff --stat -- src/unstract/llmwhisperer/_sdk_llmwhisperer | ||
| ``` | ||
|
|
||
| `git add -N` first because a plain diff cannot see a file the generator has | ||
| newly created — which is precisely what a spec that grew an endpoint | ||
| produces. Read the diff as the API change it represents. | ||
|
|
||
| 5. **Make the new surface reachable, in `client_v2.py`.** Regeneration only moves | ||
| the transport. If the spec added an endpoint or parameter callers should be | ||
| able to use, the facade is where it becomes public API. Fixes belong here or | ||
| upstream in the spec, never in the generated tree — regeneration overwrites | ||
| that wholesale. | ||
|
|
||
| 6. **Run the tests:** `tox`, which collects all of `tests/` — the integration | ||
| tier included, so it needs `LLMWHISPERER_API_KEY` in `.env`. `uv run pytest | ||
| tests/unit` is the offline subset for a fast loop, not a substitute: it | ||
| cannot catch an integration regression. `tests/unit/compat_test.py` | ||
| compares this client against the last released one, vendored under | ||
| `tests/baseline/`. Refresh that baseline only when you mean to move the parity | ||
| reference point, with `tools/refresh_baseline.sh <released-version>`; a spec | ||
| upgrade on its own is not a reason to move it. | ||
|
|
||
| ## The two CI gates | ||
|
|
||
| **`sdk-drift`** regenerates from the committed spec and fails on any diff, so a | ||
| hand-edit of the generated tree and a spec change nobody regenerated over both | ||
| fail the same way. If it is red, run step 3 and commit the result. | ||
|
|
||
| **`api-surface`** runs `griffe check` against the latest release tag — signatures | ||
| only, so it catches a facade change that breaks callers without changing the | ||
| wire. Two things about it are worth knowing before you fight it: | ||
|
|
||
| - It checks **named modules**, not the whole package, because the generated | ||
| subpackage is public to griffe: a spec change that drops a field would | ||
| otherwise report as a breakage of this client's own surface, while the drift | ||
| gate simultaneously demands the generated tree follow the spec. So if you add, | ||
| delete or rename a public module, update the module list in the `griffe check` | ||
| command in `ci_test.yaml` in the same PR — otherwise the gate silently stops | ||
| watching a module, or fails on one that no longer exists. | ||
| - It is not redundant with `compat_test.py`. Griffe cannot see what goes out on | ||
| the wire; that suite owns that half. | ||
|
|
||
| ## Versioning and release | ||
|
|
||
| Choose the bump by what changed for callers: **major** when the spec removed or | ||
| renamed something callers depend on, **minor** for new endpoints or new | ||
| behaviour, **patch** for fixes that keep the surface identical. A red | ||
| `api-surface` gate is the signal for major. | ||
|
|
||
| Do not touch `__version__` in `src/unstract/llmwhisperer/__init__.py` in your PR. | ||
| The in-repo value is the *last released* version; `main.yml` reads it, applies the | ||
| bump you pick at dispatch time, and commits the result itself. Bumping it in the | ||
| PR makes the release skip a version. | ||
|
|
||
| Release by dispatching **Release Tag and Publish Package** on `main` and choosing | ||
| the bump. | ||
|
|
||
| ## Downstream | ||
|
|
||
| `unstract-cli` pins this client exactly and vendors a copy of the same spec. | ||
| After a release lands on PyPI, that pin needs bumping there; see the | ||
| `bump-client-pins` skill in that repo. | ||
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.