feat(lib): importable in-process library facade (Client/Files) (0.61.0) - #416
Conversation
Expose a stateless, importable surface (`from keboola_agent_cli import Client`) so in-process consumers (Data Apps, transformations, services) can run Query Service SQL and read/write Storage Files without a CLI subprocess, a `serve` daemon, or a config-dir. Addresses #415 points 1, 2, 4 (point 3 -- structured query results -- shipped in 0.59.0). - Client(url, token): query(workspace_id, sql) -> list[dict] over the fast inline /results path (native JSON types; truncation warned, not silently capped); files namespace with upload(path_or_bytes), read_bytes(id) -> bytes, list() -> list[FileEntry] (uniform shape, read via read_bytes), delete(). - Move Query Service pagination (_collect_inline_results + InlineQueryResult) from workspace_service into client.py (re-exported, no behavior change) so the CLI and library share one implementation. - 23 new tests; README "Use as a library" section; changelog + version 0.61.0.
padak
left a comment
There was a problem hiding this comment.
Review of #416 — feat(lib): importable in-process library facade (Client/Files) (0.61.0)
Generated by
kbagent-pr-reviewersubagent. Verdict and findings below
are advisory; the human author retains every veto. CI-coverable issues
(lint, format, tests) are confirmed viamake check, not duplicated here.
Summary
This PR introduces a stateless, importable Python library facade (from keboola_agent_cli import Client) allowing Data Apps, transformations, and other in-process consumers to run Query Service SQL and read/write Storage Files without spawning the CLI or running kbagent serve. The implementation cleanly moves InlineQueryResult and _collect_inline_results from workspace_service.py into client.py (where they belong as pure HTTP pagination), re-exports _collect_inline_results for backward compatibility, adds lib.py as a new thin facade layer, and exposes Client, Files, FileEntry via __all__. No new CLI commands are introduced, so the Plugin synchronization map items (OPERATION_REGISTRY, AGENT_CONTEXT, CLAUDE.md All CLI Commands, commands-reference.md) are correctly untouched. make check passes with 4001 tests. The verdict is COMMENT: no blocking issues, but several non-blocking gaps are worth addressing before or shortly after merge.
Verdict
- Verdict: COMMENT
- Blocking findings: 0
- Non-blocking findings: 4
- Nits: 1
Blocking findings
(none)
Non-blocking findings
[NB-1] src/keboola_agent_cli/client.py:1 — hard file-size ceiling exceeded, and this PR adds more
client.py is 3,307 lines post-merge against a hard ceiling of 2,000 lines defined in CONTRIBUTING.md § "File-size budgets". The file was already over limit before this PR (3,232 lines on main); this PR adds 75 more lines. Per the convention: "When a file crosses the hard ceiling, splitting is required before merging more functionality into it." The fix does not need to happen in this PR, but a follow-up issue should be opened and the split should precede the next time someone adds to client.py. Suggested split: client/query_service.py (the InlineQueryResult + _collect_inline_results + submit_query + wait_for_query_job + get_query_results cluster), client/files.py (the upload/download cluster), client/base.py (shared KeboolaClient shell + token / URL setup). The library facade in lib.py imports from client.py directly, so the split is a refactor that does not change the public API.
[NB-2] tests/test_lib.py — no E2E test for the Client library facade
CONTRIBUTING.md § "Tests (mandatory!)" states: "Every CLI command MUST have a corresponding E2E test in tests/test_e2e.py". While Client is not a CLI command, it is a public-API surface shipped as keboola_agent_cli.__all__. The PR description notes 23 new unit tests against a mocked KeboolaClient — those cover the column→dict mapping, truncation, context-manager close, etc., but no test exercises Client(url, token) against a real stack. An E2E smoke test TestLibFacade in test_e2e.py should at minimum: construct a Client from E2E_API_TOKEN + E2E_URL, call files.list(), upload a small bytes payload, verify read_bytes roundtrips, and close. Without this, the signed-URL → download path, S3/GCS/Azure multi-cloud upload, and branch-resolution over the real /v2/dev-branches endpoint are never exercised in CI.
[NB-3] plugins/kbagent/agents/keboola-expert.md:§2 Tool Selection Matrix — no row for "use the in-process library from a Data App or transformation"
The Tool Selection Matrix covers the kbagent CLI and tool call (MCP) patterns, but now there is a third valid pattern for in-process consumers: from keboola_agent_cli import Client. When an AI agent is helping a developer write a Data App or a transformation that needs to call the Query Service or Storage Files, the matrix currently has no row for this case — the agent would either recommend kbagent workspace query (only meaningful from a shell) or raw httpx calls (the NEVER column for other rows). A new row (or an extension to the existing "Ad-hoc SQL" and "Storage Files" rows) stating | Consume Query Service or Files in-process from a Data App / transformation | from keboola_agent_cli import Client(0.61.0+) -- no CLI subprocess, no daemon, no config-dir; auth via envKBC_TOKEN| rawhttpxto Query Service directly (bypasses retry, no branch resolution) |subprocess.run(["kbagent", "workspace", "query", ...]) | would close this gap. CONTRIBUTING.md marks missing Tool Selection Matrix rows as NON-BLOCKING for this reviewer; the severity is appropriate.
[NB-4] plugins/kbagent/skills/kbagent/references/gotchas.md — no (since v0.61.0) entry for new library behaviors
Two behaviors introduced by this PR are non-obvious and would cause silent errors if an AI agent recommends the library without knowing them:
Client.query()requires a workspace_id that the caller must create and manage separately (kbagent workspace create/kbagent workspace load). The method namequery()suggests "query the project" not "query a pre-provisioned workspace". Callers who skip workspace provisioning get a400or a404with a confusing message.Client.query()withbranch_id=None(the default) callsGET /v2/dev-brancheson every first call perClientinstance to resolve the default branch. If the project has no dev branch configured, it raisesKeboolaApiError("No default branch found")— not obvious from the docstring since production projects always have a default branch, but it can surprise users who pass a custom token for a project mid-provisioning.
Both should be tagged (since v0.61.0) per the mandatory tagging convention.
Nits
[NIT-1]src/keboola_agent_cli/__init__.py:6—from .lib import Client, FileEntry, Filesnow runs eagerly on everyimport keboola_agent_cli, including every CLI invocation. The import chain pulls inlib.py→client.py→httpx→certifi+h11etc. at module load time. This was already the case forconstants.pypulling inhttpxindirectly, but the lib import also pulls intempfile,logging, and the fullKeboolaClientclass definition. The overhead is negligible in practice (measured at well under 1ms on CPython 3.12), but it is worth noting that the library and the CLI now share the same module entry-point. If this ever becomes a startup regression, the fix is lazy imports in__init__.py(e.g.,from __future__ import annotations+ a__getattr__shim). No action needed now.
Verification log
gh pr view 416 --json title,body,files,state→ 12 files, +675/-80,state: OPEN, conventionalfeat(lib):prefix ✓git rev-parse --abbrev-ref HEAD→claude/sweet-swartz-b0bec8matches<branch>input ✓cat /tmp/kbagent-pr-416.diff | grep "^diff --git"→ 12 changed files; nocontext.py,commands-reference.md,gotchas.md,CLAUDE.md, orpermissions.py(correctly absent — no CLI commands added) ✓- Layer-violation greps (
typer,click,formatter.,console.printinlib.py;httpxincommands/) → empty ✓ grep -n "typer\|click\|formatter\|console.print" src/keboola_agent_cli/lib.py→ empty ✓InlineQueryResultconsumers scan:grep -rn "InlineQueryResult" src/ tests/→ onlyclient.py(definition) andtests/test_lib.py(test) ✓from keboola_agent_cli.services.workspace_service import InlineQueryResult→ no such import anywhere insrc/ortests/(relocation is safe, no external consumers) ✓uv run python -c "import keboola_agent_cli; print(keboola_agent_cli.__all__)"→['Client', 'FileEntry', 'Files', '__version__'], no network calls ✓- Magic numbers, bare
except:, raw error-code strings,print()in prod code → all greps empty ✓ wc -l src/keboola_agent_cli/client.py→ 3307 (hard ceiling is 2000; pre-PR was 3232) → NON-BLOCKING [NB-1]grep -n "from keboola_agent_cli import Client" tests/test_e2e.py→ no match; no E2E test forClient→ NON-BLOCKING [NB-2]grep -n "lib\|importable\|in.process\|Client(" plugins/kbagent/agents/keboola-expert.md→ no match → NON-BLOCKING [NB-3]make check→4001 passed, 8 skipped, 124 deselected, 15 warnings in 82.94s✓- Behavior reproduction: could not exercise
Client(url, token)against a real stack (no credentials; per reviewer playbook, noted here as unverified) — all behaviors are asserted via unit tests against mockedKeboolaClient
Open questions for the author
- The PR description says
query()returns rows of the last statement that produced a result set (for multi-statement SQL). This is documented in thequery()docstring and tested, but it differs from the CLI behavior inworkspace querywhere--jsonshows all statements. Is returning only the last result set the intended stable contract for the library facade? If so, the gotcha entry in [NB-4] should call this out explicitly so AI agents recommendSELECTas the final statement.
…ix row Follow-ups from the kbagent-pr-reviewer pass on PR #416 (all non-blocking): - NB-2: E2E test exercising the Client facade against a live stack (query + Storage Files round-trip), gated on E2E_API_TOKEN like the rest. - NB-4: gotchas.md (since v0.61.0) -- query() needs a provisioned workspace; branch_id=None costs a lazy branch-list call on first use. - NB-3: keboola-expert.md tool-selection row pointing in-process Python consumers at `from keboola_agent_cli import Client`. NB-1 (client.py 3307 > 2000-line ceiling) filed as tech-debt on #417.
…otchas Follow-ups from the kbagent-pr-reviewer pass on PR #418: - B-1: tests/test_lib.py mock data + assertions now use the real string contract ("1" not 1), so the tests no longer teach "the library gives ints". - NB-1: the 0.61.0 changelog entry no longer actively claims "native JSON types" (corrected in-place; 0.61.1 cites it as the prior wrong claim). - NB-2: the new gotchas bullet carries the (updated v0.61.1 -- closes #416) tag. - NIT-1: the gotchas hedge now says "BigQuery behavior not yet verified".
… strings (0.61.1) (#418) * docs(lib): correct query() value-typing contract -- Snowflake returns strings (0.61.1) A live E2E round-trip against a Snowflake workspace showed the Query Service /results endpoint returns scalars as JSON strings (1 -> "1", true -> "true"; NULL -> None), not native types. The 0.61.0 Client.query() docstring and release notes wrongly claimed "native JSON types". The facade is transparent and does not coerce -- callers must cast. - lib.py: query() docstring documents the real, stable contract. - gotchas.md: add the type-serialization gotcha to the facade entry. - v0.61.0 GitHub release notes corrected (gh release edit). - Bump 0.61.1, changelog. No behavior change. * docs(gotchas): "Two" -> "Three" non-obvious behaviors (PR #418 review) * docs: address PR #418 review -- string-typed test mocks + changelog/gotchas Follow-ups from the kbagent-pr-reviewer pass on PR #418: - B-1: tests/test_lib.py mock data + assertions now use the real string contract ("1" not 1), so the tests no longer teach "the library gives ints". - NB-1: the 0.61.0 changelog entry no longer actively claims "native JSON types" (corrected in-place; 0.61.1 cites it as the prior wrong claim). - NB-2: the new gotchas bullet carries the (updated v0.61.1 -- closes #416) tag. - NIT-1: the gotchas hedge now says "BigQuery behavior not yet verified".
Why
The jasnost project (#415) needs to consume kbagent's Query Service + Storage Files in-process — a hosted Data App can't cleanly run kbagent as a CLI subprocess or a
servesidecar. Today the only entry points are the CLI and the HTTP daemon, so consumers hand-roll a Query Service client and depend onkbcstoragefor Files.What
A stateless, importable library facade — committed public API under
keboola_agent_cli.__all__:No daemon, no shell-out, no config-dir.
Client(url, token)wraps the existingKeboolaClient(shared retry/backoff); the facade adds the high-level shapes the CLI used to assemble in its service layer.Maps to issue #415
from keboola_agent_cli import Client— stateless wrapper, no daemon/shell-out/config-dirquery()returnslist[dict], native JSON typesread_bytes(id) -> bytes(sliced + gzip handled),list() -> list[FileEntry]Design note on point 4: instead of carrying a sometimes-absent signed
urlin every list item, the single read path isfiles.read_bytes(id)— callers never branch on "does this item have a URL?", and there's no expiring-URL footgun.FileEntry.rawkeeps the full API dict as an escape hatch.This lets jasnost delete its hand-rolled
query_service.pyand thekbcstoragedependency and consolidate on kbagent in-process, no sidecar.Layers touched
lib.py—Client,Files,FileEntry.client.py—_collect_inline_results+InlineQueryResultmoved here (Query Service pagination is a layer-3 concern, no config/business logic);workspace_servicere-exports them with no behavior change, so the CLI and the library share one implementation.__init__.py—__all__public surface.tests/test_lib.py); README "Use as a library" section; changelog + version0.61.0.Tests
make checkgreen (4001 passed, 132 skipped). The facade is fully unit-tested against a mockedKeboolaClient: query column→dict mapping, multi-statement (last result set), truncation warning, default-branch resolution, files list shape, upload from path and bytes,read_bytessliced/non-sliced, missing-URL error, and context-manager close.Follow-up
A separate issue will track Storage-client gaps surfaced while comparing against the official
sapi-python-client(e.g.where/changed_sinceexport filters, typedadd-column, Storage triggers, retry jitter) — all out of scope for this facade.