UN-4008 [FEAT] Add the unstract CLI to run extractions and API deployments from the terminal - #2
Conversation
Wheel skeleton for the `unstract` console script: Click app with the
whisper / docstudio / config groups, and the three cross-cutting layers
every command will sit on.
- config: named profiles resolved flag > env > profile > default, with
`env:` indirection so the file records where a secret lives rather than
the secret, 0600 writes, deployment aliases, and `config doctor`
reporting where each setting resolved from without echoing a value.
- output: one JSON envelope {ok, data, error, meta} on stdout for success
and failure alike, so parsing never depends on TTY detection; table and
raw are opt-in renderings, diagnostics go to stderr.
- errors: the exit-code table as a stable API, retry policy that never
retries a 4xx, redaction, and undeclared statuses reported verbatim
rather than guessed.
- poll: transport-agnostic --wait loop reading terminal state from the
response body rather than the HTTP status, never sleeping past the
deadline, echoing the job handle on timeout so work resumes instead of
being resubmitted, and persisting a one-shot result before the read is
acknowledged.
No transport yet: the clients own HTTP. Tests are offline and need no
credentials.
Flags for an operation come from the spec the published client is generated from, intersected with what that client's signature actually accepts: a spec parameter the frozen client cannot name would raise TypeError at the call rather than reach the API, so it is not offered. Two rules keep the derivation honest. Every option defaults to None, meaning absent, so an unpassed flag is not sent and the client or server default applies rather than a value pinned here. And only None is treated as absent: 0, false and "" are choices a caller made and travel to the request. Help text has three sources in order: the overlay, the spec, and the client method's own docstring, which is the only one that describes the parameters today. The overlay carries what a generated spec cannot express -- allowed values, short flags, wording -- in TOML read with the stdlib.
Thirteen commands: whisper extract/status/retrieve/detail/highlights/usage and its four webhook commands, plus deployment run and status. Each one holds only what a spec cannot say -- which parameter is the argument, which the CLI owns, and how a result is polled for. The CLI runs the poll loop for both products rather than using the loop one client ships, so --wait, --interval, --timeout and the handle-returned-on- timeout behaviour are the same everywhere. Deployment runs are queued (timeout=0) so a request does not hold a connection open for the length of the job. Line-highlight scaling is arithmetic on a reply rather than a request, so it is folded into the command that fetches the metadata. Failures converge on one envelope: LLMWhisperer raises with a status code, the deployment client returns one, and both become a CLIError with an exit code and a hint. A result that can be read only once is written to disk before it is printed.
--discover answers what --help answers, as JSON, in three tiers: groups names the products, summary adds their commands, full adds every flag with its type, choices and default plus the exit-code table -- enough to construct a call without a second round trip. A caller starts cheap and drills down. Every tier is read back from Click itself, so a described command cannot drift from the one the parser accepts, and discovery reads no configuration: it is how a caller learns what exists, so it has to work before anything is set up. config doctor --probe adds the second diagnostic question -- does the resolved key work -- to the one it already answered offline, where it resolves from. LLMWhisperer is checked against its usage endpoint. A deployment has no side-effect-free endpoint to call, so its entry reports that the settings resolve and says plainly that nothing was verified.
The vendored specs and the pinned clients move independently, so a refreshed spec can declare a parameter the published client has no argument for. Such a parameter is dropped rather than offered and rejected at the call, and dropping it silently is the failure this pins: the gap is written down per operation, so widening it is a decision rather than an accident.
Two failures a live call found and no offline test could. The metadata arrives as a named object carrying the coordinate list under `raw`, while the client's geometry takes the bare list, so no line was ever scaled. And a line the service has no geometry for is reported as all zeros, whose page height is a divisor in that scaling: it raised ZeroDivisionError out of the client, which the entry point does not catch, so the command printed a traceback with an empty stdout. Such a line now gets no box.
Three follow-ups to the command surface. Both client pins move forward, and the six extraction parameters and three status parameters they gained appear as flags with no line written here -- which is what deriving from the specs was for. The contract test's unreachable set shrinks to what the clients own rather than lack: the URL-in-body flag and the execution id read from the endpoint URL. --base-url, --api-key and (for deployments) --org-id sit on the product group and fill the flag tier of flag > env > profile > default, which the loader already supported but nothing populated. A key given on the command line warns: it lands in shell history and in the process list. The 406 hint is scoped to deployments. A whisper result read twice comes back as a 400 whose body says so, and translating on that prose would break the moment the wording changes -- the service's own message already says what happened, and it is passed through verbatim.
`deployment status` derived --include-metadata, --include-metrics and --include-extracted-text from the spec, collected them into **params, and never passed them to the client. The command succeeded and the payload parsed, so a dropped flag was indistinguishable from a working one. The poll loop behind `deployment run --wait` had the same hole, which made a waited run return less than the identical flags returned without --wait. Both now forward what was asked for, and the parameters the status endpoint does not accept are filtered out rather than sent. Tests cover each flag in both polarities, since a flag silently dropped is exactly what the offline suite missed. Alongside: - `config doctor` no longer reports an `org_id` setting for LLMWhisperer, which has none. It always read as unresolved and there was no way to resolve it. - The deployment probe reports `ok: null`, not `ok: true`. Nothing is called, so there is no verdict; `true` beside `checked: false` reads as a live check that passed. `resolved` carries what is actually known. - The 406 hint pointed at --save, which does not exist on the command that emits the hint. It now names the command that has it. - A 400 carries a hint. The service can answer 400 with an empty error body, in which case the message was a synthesised fallback and there was nothing else to go on. Adds RUNBOOK.md: install, moving the client pins, the live-gate checklist, and the release steps. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ
Waiting returns the result and nothing else: the extracted text, or the deployment's structured output. Neither names the job, so a caller who waited had no handle to correlate against the service, quote in a bug report, or use for a follow-up call. Without --wait the handle is the entire payload, so the identity appeared and disappeared depending on a flag. Both waited paths now carry it in `meta` -- the whisper hash and the execution id -- leaving `data` exactly as it was. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ
`--save` exists to protect a read the service serves exactly once, and it
was the flag that lost the data: the write ran after the acknowledging
read, raised `OSError` through an entry point that does not catch it, and
left an empty stdout with the extraction gone. The target is now proven
writable before anything destructive runs, the write goes through a
temporary file so a full disk cannot truncate the previous copy, and a
write that fails anyway raises with the payload attached under its own
exit code -- by that point the envelope carries the only copy left.
Also on the one-shot path: a waited extract read the result with a bare
`.get("extraction")` where the sibling command falls back to the whole
payload, so a response shaped any other way printed `ok: true, data: null`
for a document that had been processed and billed. Both now read it the
same way, and a genuinely empty result is a failure rather than a silent
success.
Redaction was an opt-in keyword argument that only the success path
passed, so every error envelope and every stderr summary went out with
the key in it -- four times on stdout in the reproduced case. Credentials
are now registered where they resolve and scrubbed by every emitter, and
`CLIError.details` is redacted structurally rather than at each call site.
Three more places where a failure was reported as a success: the
standalone status commands ignored a finished-and-failed execution inside
an HTTP 200, the poll loop treated an unreadable body as progress and then
blamed the timeout on a job it never confirmed was running, and any status
outside 4xx/5xx mapped to exit 0 while printing `ok: false`.
Verified by mutation -- moving the save after the print, dropping the
registry, dropping the details redaction and dropping the status check
each fail the suite now, and none of them did before.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ
A CLI whose output shape depends on whether a terminal is attached is a CLI whose scripts break when they move from a shell to CI. This drops the isatty question entirely: the default is a table, in a terminal and in a pipe alike, and anything that parses the output asks for `-o json`. An explicit `-o` is the last word. The environment picks the default and nothing more, so the same `-o json` invocation renders the same bytes wherever it runs -- which is the property a caller is actually relying on. Coding agents are the exception worth making: they set a marker in the environment, and there the default becomes json rather than making every call carry a flag. `--agent yes|no` settles it either way. Every envelope now carries `meta.contract_version`, and `--discover full` publishes what a consumer has to do to hold up its end: ignore unknown fields, refuse a version above the one it was written against. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ
Nothing bounded a stalled connection: the deployment client is untimed and its api_timeout is an execution mode the backend reads, not a socket timeout. --transport-timeout sets one. Unset by default, so a run that would have hung still hangs rather than starting to fail in a way no existing script expects. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ
Ctrl-C came back as exit 1 with nothing on stdout, which reads to a supervisor as a failed command worth retrying -- the one thing that must not happen to a run the user deliberately stopped. It now exits 130, the value every shell already reads that way, and prints the same envelope as any other failure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ
Overlay, spec and client docstring can each describe a flag. No spec parameter carries a description today, so the order between them is unexercised until one does, which is exactly when an inversion would ship unnoticed.
The vendored LLMWhisperer spec was several revisions behind and now declares enums the CLI was hand-listing. The two had already diverged: --mode rejected three modes the service accepts and --output-mode two, and nothing would have reported it. Read the enum off the spec, keep the overlay for narrowing one on purpose, and drop the descriptions' own value lists for the same reason their default sentences are dropped. `highlights` gains a `mode` query parameter that the published client has no argument for, so it joins the parameters the CLI cannot reach.
A sentence-shaped match ends at the first period, so "Defaults to 0.3." was left in the help beside the default rendered from the signature. Strip each restated sentence with its own end-anchored pass instead.
The pinned clients predated the fix that stops an omitted optional parameter being sent as the string "None", so a CLI built on them sent it. The derived surface is byte-identical across the move; neither signature changed.
Each of these restated the line below it, or described a prior state that is no longer there to check against. Keep the reason, drop the narration.
Copies one organization's resources into another by calling the client's orchestrator directly. Two endpoints with a key each, which no single profile describes, so both are flags and both keys come from the environment. Also moves the client pin forward to pick up the status path-prefix fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ
The vendored copy was several iterations behind the one the pinned client is generated from, so the CLI's help, its parameter set and what --discover publishes all described an older service contract. The flag snapshot is the check that makes a resync safe: every other contract assertion reads the spec on both sides of its comparison, so a spec that loses a parameter loses the flag and the expectation with it.
…tatus A transport error was translated into a CLIError outside the poll loop, where the handle no longer exists, so the caller was left to resubmit a document the service had already processed and billed. Translating at the call keeps the loop's own context; the loop attaches the handle itself for anything the caller did not translate. `whisper status` reported a failed extraction as a success, its sibling in the other product having already been fixed: both read the body, not the status code.
Four failures the CLI reported as successes or as something vaguer than it knew: - a server-reported error inside a 2xx got the catch-all exit code, which is the least informative one for the most interesting failure this API has; - `config doctor` printed its own findings and exited 0, so a setup script branching on it read a broken configuration as a working one; - a deployment alias pointing at an unset environment variable fell back to the profile's organisation and key, running against a tenant nobody named; - a webhook's auth token was echoed verbatim. The restated-default stripper was also greedy to the end of the string, so a description whose value list came first lost every sentence after it.
The command that writes into a live organisation had none of its own behaviour pinned. Its table output -- the one a person gets, and the only output path that did not go through the emitter -- scrubbed by hand and was run by no test, while the test that claimed a platform key never reaches stdout passed with the registration deleted. Rendered output now goes out through the same path as every envelope, and a key planted in a report is asserted not to survive it. Also: --on-name-conflict decides what is written into the target and is now asserted to arrive; skipped documents are counted at the top of the payload, because skipping is not fatal and a caller reading the exit code alone would never learn a document did not move; `config doctor` resolves each deployment alias the way a run does, instead of listing names its docstring implies it checked; a failed retrieve is pinned to carry the handle; the restated-default stripper ends at its own sentence rather than at the end of the text; and the groups tier lists leaf commands apart from groups, which a consumer walks differently.
The status endpoint's own query parameters are forwarded now, and a deployment URL that carries no derivable prefix is polled where the service said rather than at a rebuilt path.
The notes carry the console-script collision, the behaviours a script would otherwise discover by being surprised, and the service version a custom page separator needs. The pin moves to a documentation-only commit.
The envelope shape is documented in the README and published by --discover; greeting every --help with it buries the two things a reader is there for.
A .unstract.toml found by upward search comes from whatever checkout the user happens to be standing in. It may still select a profile, set org_id and define deployment aliases; api_key and base_url are withheld, with a warning, and reported as withheld by config doctor. Named explicitly with --config or $UNSTRACT_CONFIG, the same file is honoured in full. Also point a first-time user at where keys are minted, from config init, from doctor and from the README, and ship an on-prem profile shape. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ
The discovered config path is written to as well as read from, so a symlinked .unstract.toml let a repository redirect config set and config init --force onto any file it named. The upward search now skips a symlinked candidate, and the write opens with O_NOFOLLOW so a symlink at the target is a clear error rather than a truncation. Also: the config group reports the file's warnings instead of dropping them, doctor answers for a withheld deployment-alias key the way it does for a product one, trust is derived from the path rather than from how the loader was called, and the README says plainly that routing stays repo-controllable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ
The deployment client sets none of its own, so a stalled connection was waited on forever unless the flag was passed. Default to the 120s the LLMWhisperer client applies; `--transport-timeout 0` keeps the old behaviour for a caller who wants it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ
`create-github-app-token@v3` takes the App's client id, and the org's existing variables are named for it, so this repo can share them rather than needing an App of its own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ
The hash and the file move in the same commit, so the check cannot tell a deliberate edit from a refresh. It catches a copy that was corrupted or half-updated, and a provenance entry left behind by its file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ
provenance.json now records the exact pin each spec was copied for, and tests/test_specs.py compares it with the pin in pyproject.toml. A client bumped without its spec re-synced fails locally, with no network, instead of deriving flags the released client cannot carry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ
The service reports a batch as COMPLETED even when a document inside it failed, with the failure carried per file in extraction_result. The CLI read only the execution status, so a caller branching on the exit code was told the batch succeeded with a document's output missing. Both `deployment run` and `deployment status` now walk the per-file results and fail with the failed files named; the full payload is kept verbatim in error.details, since the status read is one-shot and the successful documents survive nowhere else, and --save still writes it before the error is raised. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ
ritwik-g
left a comment
ritwik-g
left a comment
There was a problem hiding this comment.
Standardized PR Review — FOLLOWUP (rounds 2 and 3, combined)
Scope: f46b650c..1ac518e7 — the 26 commits since my first review. Base unmoved (13cb3f77), branch current. Scope change: NO — no new files, dependencies, persisted fields or network calls.
Verdict: REQUEST CHANGES
Critical: 0 · High: 3 · Medium: 5 · Prior findings resolved: 21 of 22
Down from BLOCK. Every High below is a regression introduced by a fix, or a guard on a fix that cannot fail — not a surviving defect from the original PR. Each has a one-line remedy.
Prior findings — reconciliation
RESOLVED (21): #1-#8, #10-#19, #21, #22, the coverage entry, and all four unanchored items.
Three were re-proved by mutation rather than taken on trust — #7 (clients.py, both original mutations now fail), #14 (test_workflows.py, and its anti-vacuity guard is itself load-bearing), and #9.
#9 deserves a specific note. It was the one High left open pending a live run, and 1ac518e settles it: "The service reports a batch as COMPLETED even when a document inside it failed." So it was real, not void. The fix walks per-file results on both run and status, sets verbatim_details=True so the successful documents survive the error path, names the failed files in extra, and updates the exit-code table. I verified the --save-before-raise ordering on both paths myself (poll.py:266-272 inside wait_for_completion; docstudio_cmd.py:285 before :287). Good fix.
NOT RESOLVED — deliberately, with reasoning I accept (1): #20, tracked as issue #7 — nothing on PyPI to pin to until the release workflow runs once.
Two claims I investigated and rejected
Recorded because passing them on would have wasted your time:
- "
preflightno longer rejects a directory--savetarget." False positive. All three--saveoptions declareclick.Path(dir_okay=False); Click raisesBadParameterbeforepreflightruns. Reproduced by callingpreflight()directly, which bypasses type conversion. - "
PUSH_TO_MAIN_APP_CLIENT_IDmay not be set." Already satisfied — it is available to this repo org-wide, and in use in three other Zipstack repos.
Release-path status, since it came up in Slack
All three prerequisites are now met. The org variable and secret were already shared with this repo, the App was already installed org-wide (repository_selection: "all"), and the main ruleset now carries zipstack-push-to-main (actor_id 1199465, Always) in its bypass list — verified, with all four rules intact. Only step 3 ever actually needed doing.
Lens checklist — 19/19
Unchanged from round 1 except: 3 → #1, #2, #8; 7 → #1; 11 → #4; 13 → #3, #7, #8; 16 → #5, #6. 4 Security — Clean, with the open question below. 15 — Clean; pinned gate passes at ruff 0.16.2 (the lockfile's version) on 1ac518e7. 17 — Clean, branch current. 6, 12, 18, 19 — N/A.
Coverage caveat, stated rather than hidden: lens 3 over commit 1ac518e was assessed by me directly, not by a dispatched specialist — that agent terminated on a rate limit and returned a fragment. I read _failed_files / _raise_for_failed_files and both call sites in full and found the logic sound, but this line had one reader rather than two.
Open question for the author
verbatim_details=True means a credential the run never resolved, appearing under a secret-looking key in a server-authored body, now reaches stdout where it was redacted before. Nobody could trace a concrete route — FileResult.metadata is untyped in the vendored spec, which is the gap. Does include_metadata on a deployment status response ever carry adapter or connector credentials? If it can, poll.py:172/:205 and docstudio_cmd.py:207 need redaction on everything except the result-bearing key rather than being skipped wholesale.
Unanchored observation
_failed_files treats any per-file status that is not casefolded "success" as a failure, while the execution-level poll in the same file accepts two success spellings (terminal_success=("COMPLETED", "SUCCESS")). The spec types FileResult.status as a bare string with no enum. The new tests do cover case variation ("Success", "SUCCESS") and the all-succeeded direction, and you confirmed the shape against the live service — so this is a robustness note, not a defect. Worth a comment naming the spelling you observed, since the spec does not pin it.
Posted as a comment, not REQUEST_CHANGES — the merge gate is the maintainers' call.
Reading an empty string as "no answer yet" was true only of the deployment client, which spells a pending result that way. The LLMWhisperer client returns empty text for a document with nothing on it, so `-o raw` printed the hash in its place, or failed and spent the one-shot read. Empty now means pending for the one field that spells it so, and is the answer everywhere else. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ
The hint is paste-ready, and the status read it names is one-shot: a caller who asked for --save and pasted it as given spent the read with nothing written to disk. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ
The fixture carried no field the redactor matches, so the assertion that the successful documents survive verbatim passed with or without it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ
The rule existed and nothing exercised it: the fixture carried a status on every entry, so dropping the branch left the suite green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ
Publishing last means a failed publish leaves a public release naming a version nobody can install, and the tag guard stops the rerun that would fix it. The release is created as a draft and made public only after the publish succeeds, so a failure at any point leaves nothing public. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ
It named an import relationship that does not exist. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ
The test compares two recorded strings; it cannot tell whether the spec was re-synced for the pin it names. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ
Under a captured sys.stdout the descriptor lookup raises before the redirect runs, and the guard around it swallowed that too, so the test passed with the redirect deleted. It now runs against a real descriptor and checks the redirect targets stdout. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Re the open question in the follow-up review, whether Checked the backend ( |
The suppress existed because Windows cannot open a directory to sync it, but it also hid a real failure on every other platform and reported the write as fully durable. The sync is now skipped only on Windows; elsewhere a failure is warned about, since the config has already been renamed into place and the exit code stays what the write earned. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ
ritwik-g
left a comment
ritwik-g
left a comment
There was a problem hiding this comment.
Approving at 312ae76.
I checked every fix from my last review against the code, not just the replies. For each one I removed the fix and re-ran the suite, and a test failed every time:
-o rawempty-vs-pending (89ed9bf): fails both when empty is always treated as pending and when it never is--savekept in the timeout resume hint (7e976fb)verbatim_detailson the failed-files payload (0b40b3d)- Directory-sync failure reported (
312ae76) - Release only goes public after
uv publish(3d58591)
404 passed, 1 skipped. ruff 0.16.2 lint and format are clean. The main ruleset now has the push-to-main App in its bypass list, so the release push won't be rejected.
Two small non-blocking follow-ups, fine to leave for later:
- The
--save <path>in the resume hint isn't shell-quoted, so a path with a space won't paste as-is. - The new directory-sync warning may fire on filesystems that reject
fsyncon a directory. The config is still written; it's only noise.
The long ones restated what the next line does, carried detail that goes stale (library versions, exact counts) or explained a decision at more length than the code it sits above. Each is now one or two lines saying why, readable without knowing how the code got there. Behaviour is untouched: every changed file parses to the same AST once docstrings are stripped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ
`persist` fsynced the temporary file and renamed it over the target, but never synced the directory the rename wrote to. A crash between the two can leave the bytes on disk with nothing pointing at them -- and a retrieval that can only be read once has no second copy to recover from. The directory is synced after the rename, and a failure to sync it is raised like any other save failure, with the payload on `details`: the file may be there, but nothing can promise it is, and the caller has to be handed the result either way. `save_config` already did this, and warns rather than raises because a config can simply be written again; that difference is deliberate and left as it is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ
A comment earns its place by stopping a plausible edit from being wrong: an ordering requirement, a scrub bypass, a one-shot read, a trap in a library. Comments that only justify a decision the code already makes are removed, and constraint statements padded with argument are cut to the constraint. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ
|
@greptileai elaborate on what's needed to make this PR 5/5 |
|
The current head is close, but I would keep it at 4/5 until the release and durability semantics are tightened. The earlier P1s around atomic temp files, config symlinks, Merge blockers
Strongly recommended before calling it 5/5
Owner-side release checklist
Once the branch restriction, App configuration, rerun behavior, and durability contract are explicit and covered by tests, I would consider this 5/5 release-ready. |
…the release to main The release workflow checks out whatever ref dispatched it. A candidate or a `none` bump reaches PyPI with no commit to review, so the job now refuses to run off main, and the App credentials are checked before the checkout rather than failing as an empty token further down. `click~=8.1.0` installs the newest 8.1.x, which is not the floor the package declares; the floor is now installed exactly. A new job installs the built wheel into a clean environment and exercises both entry points, `--discover full`, the usage-error envelope and its exit code, and `config init` -- none of which a test run inside the source tree can fail on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ
…vive a crash `save_config` warns on stderr when the directory entry could not be synced after the rename; a JSON consumer reading only stdout saw a plain success. `config init` and `config set` now add `meta.durability: "unconfirmed"` in that case. Nothing changes when the sync succeeds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ
3 participants
What
unstract— one command that takes a document to structured JSON: LLMWhisperer extraction, an API deployment run, and the polling in between.unstract config init | list | get | set | doctorunstract whisper extract | status | retrieve | detail | highlights | usage,whisper webhook create | get | update | deleteunstract docstudio deployment run | statusunstract --discover groups | summary | fullWhy
Both products are reachable today only from Python. This makes them scriptable from a shell and drivable by an agent: one output envelope, one exit-code table, and a JSON description of the whole surface so a caller can construct a command without a second round trip.
How
flag > env > profile > default. Discovery is--config→$UNSTRACT_CONFIG→ a project-local.unstract.tomlfound by searching upward (stopping at$HOME) →~/.unstract/config.toml. Values may indirect throughenv:VAR, so a config file can be committed without a key in it. Files are written 0600. Deployments are named aliases inheriting org and key from their profile..unstract.tomlmay not supplyapi_keyorbase_url. A checkout the user did not write would otherwise hand the CLI a key or point it at another host; a path the user names explicitly is a deliberate choice and is honoured. Withheld values are kept and written back to their own file, never carried into another one. Config writes refuse a symlinked target and land throughmkstemp+os.replace, so a planted symlink cannot redirectconfig setonto some other file. Routing (org_id,api_name, profile selection) stays repo-controllable by design.{ok, data, error, meta}on stdout, in JSON by default whether or not stdout is a TTY, so a script gets the same bytes as a terminal.--output tablewraps rather than truncates.--timeout 30returns at 30s — and a timeout carries the handle out so a caller can resume rather than restart.TypeErrorat the call rather than reach the API, so it is not offered, andtests/test_contract.pyrecords which ones those are so the gap widens on purpose or not at all. An unpassed flag is not sent, so the server default applies rather than one pinned here; onlyNonecounts as absent, so0,falseand""travel.--discoveranswers what--helpanswers, as JSON, read back from Click itself, so a described command cannot drift from the one the parser accepts.fulladds every flag with its type, choices and default plus the exit-code table.Can this PR break any existing features
No. New repository, nothing depends on it yet, and it is not published. It reads the two clients through their public APIs only.
Notes on Testing
230 tests, offline by design: no network and no credentials — the clients are replaced at the factory, so what is asserted is which arguments a command hands the client, and what a caller sees on stdout and in the exit code. CI runs ruff and pytest, then the suite a second time against the newest
clickthe pin allows —uv runresolves from the lockfile, and an install in the wild does not. Live round trips are a manual pre-release step.Related Issues or PRs
Pins
Built on
unstract-client==1.6.0andllmwhisperer-client==2.9.0, both from PyPI. The vendored specs are byte-identical copies of the ones those releases were generated from, recorded with their source commit and sha256 insrc/unstract_cli/specs/provenance.jsonand checked bytests/test_specs.py. Neither client installs a console script, sounstractis this CLI's alone;unstract-cliremains as a second name.Release
.github/workflows/release.yml— a manual dispatch, frommainonly, that bumps the version, lints, tests and builds, then commits the bump, tags it and cuts the GitHub release as a draft, publishes to PyPI withuv publishthrough a Trusted Publisher, and un-drafts the release last. PyPI is the one step that cannot be undone, so it goes after everything that can: a failure before it leaves a draft release, a tag and a bump commit to remove, in that order, and the run is dispatched again. A failure after it leaves the version published, so the same version cannot be re-cut -- the tag guard refuses it; promote or bump instead. The version lives only insrc/unstract_cli/__init__.py, read through hatch, so the bump edits one file, and the committed value names the last stable release.pre_releasepublishes a PEP 440 release candidate: the target version is computed as usual, thenrcNis appended, counting up from the rc tags already published for that target, and__version__is left alone because a candidate is not a release. Promoting is the same dispatch withpre_releaseoff.version_bump: nonetargets the version already in the repo, so the first cut isnone + pre_release→0.1.0rc1, andnonealone promotes it to0.1.0.Owner-side setup, none of which exists yet on this repo:
unstract-cli, OwnerZipstack, Repository nameunstract-cli, Workflow namerelease.yml, Environment name blank (the workflow declares no environment).PUSH_TO_MAIN_APP_CLIENT_ID(Actions variable, holding the App's client ID — the workflow authenticates withclient-id, notapp-id) andPUSH_TO_MAIN_APP_PRIVATE_KEY(Actions secret) must reach this repo. Neither exists at repo level here; both are presumably org-level, and an org-level secret still has to listunstract-cliamong the repositories it is visible to.Zipstack/unstract-cli— the workflow requests a token scoped to that repository by name.🤖 Generated with Claude Code
https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ