Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions .github/workflows/spec-sync.yml
Original file line number Diff line number Diff line change
Expand Up @@ -553,6 +553,22 @@ jobs:
ROUTES are `/v1/*` is not "nothing to do": still wire every in-scope change listed
below (a new request field on /v2/parse, say) and leave the `/v1/*` routes alone.

SCOPE, one level down — exactly which FIELDS back a /v2 request: the TOP-LEVEL
properties of that operation's `requestBody` schema in this spec, and nothing else. A
field the spec declares inside a nested object is not a top-level field — the
encrypted-PDF password is declared at `options.password` on /v2/parse and
/v2/parse/jobs and NOWHERE else, so it is sent there and only there. Two traps, both
of which have bitten these SDKs: (a) the SAME spec declares a top-level `password` on
/v1/ade/parse and /v1/ade/parse/jobs — same name, different level, different route
family — and lifting a `/v1/*` top-level field onto a /v2 operation is the field-level
version of translating a /v1 route into a /v2 URL; (b) inventing a top-level
convenience parameter that folds one wire field into another. Exactly two exist
(`password` on parse, `strict` on extract); they are cross-SDK contract decisions
shared with ade-python and written down in CONTRIBUTING.md, and changing one is a
maintainer's call — do NOT add, remove or re-target an alias. Wire the spec's own
shape, and if a spec change looks like it needs a new one, leave that field unwired
and say so in your summary.

OUT OF SCOPE — do NOT wire: /v2/workflow, /v2/workflow/jobs, and
/v2/workflow/jobs/{job_id}. That surface IS shipped (client.v2.workflow / workflowJobs)
but is hand-maintained and NOT auto-wired by spec-sync; a maintainer reconciles workflow
Expand Down
28 changes: 28 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,34 @@ any wired path the snapshot does not have, so a `/v1/*` route can no longer be "
`/v2/*` one. A new `/v1/*` route in the V2 spec is therefore expected to produce a PR that wires
only the in-scope `/v2/*` changes (if any) and mentions the unwired routes in its description.

**V2 request fields — no gate, compare by hand.** The same rule applies one level down: `client.v2`
Comment thread
tian-lan-landing marked this conversation as resolved.
sends the **top-level** properties of an operation's `requestBody`, a field the spec declares inside
a nested object is not a top-level field, and a top-level field on a `/v1/*` operation is not a
`/v2` field however familiar its name. The encrypted-PDF password is the live example — declared at
`options.password` on `/v2/parse*` and nowhere else, while `/v1/ade/parse*` in the _same_ spec
declares a top-level `password`. Both SDKs nevertheless expose two hand-written top-level
conveniences that fold into nested fields, and these must stay identical in `ade-typescript` and
`ade-python`:

- `password` on `/v2/parse` and `/v2/parse/jobs` → `options.password`. When a caller supplies both,
an explicit `options.password` **wins**, an explicit `null` included (`null` means "no password").
Test the _value_, not key presence: `{password: undefined}` is how JS spells "absent" and
`JSON.stringify` drops the key, so a presence test would suppress the shorthand **and** send no
password at all. `ade-python` carries the mirror-image trap — there `dict(json.loads(x))` accepts
any pair-sequence, so `'[["password", "sneaky"]]'` silently becomes the options dict; coerce
`options` through a helper that requires an object on both the string and the object branch.
- `strict` on `/v2/extract` and `/v2/extract/jobs` → `options.strict`. No conflict is possible —
neither SDK exposes `options` on extract, so the shorthand is the only way to reach the field. If
either SDK ever exposes it, that becomes a real tie-break and needs a rule here first.

**Nothing in CI checks this** — `check-v2-paths` covers routes only, and a hand-written alias is not
spec-derived, so neither repo's CI can see the other's choice. A PR that adds or touches a top-level
param which is _not_ in the spec's top-level `requestBody` properties has to be diffed against the
other repo by hand: precedence, `undefined`/`None` handling, and whether either SDK sends the value
twice. That comparison is exactly what did not happen before #121 / `ade-python#160` — the two SDKs
picked opposite tie-breaks and one call sent a different password per language, which surfaced only
as a 422 `encrypted_pdf_wrong_password` naming no cause.

**Protected environment:** the `contract-tests` gate runs AI-drafted test code with
`LANDINGAI_ADE_STAGING_APIKEY` in env. Configure a **`spec-sync-staging`** Environment (repo Settings
→ Environments) with a **required reviewer** so a maintainer approves before the staging key is
Expand Down