feat(data-app): make python-js the default everywhere, incl. type-less sync creates - #783
Conversation
keboola-pr-reviewer-bot
left a comment
There was a problem hiding this comment.
Verdict: needs_human (risk 2/5) · profile _default
n/a
Impact flags: possible rollback re-introduction — see Check Run summary.
Concerns:
plugins/kbagent/agents/keboola-expert.md: Edits agent-instruction files (subagent prompt, SKILL triggers); policy forbids auto-approving these.src/keboola_agent_cli/services/_sync_push_ops.py: Type-less Streamlit sync trees now create as python-js, not streamlit (behavior change).
soustruh
left a comment
There was a problem hiding this comment.
Review of #783 — feat(data-app): make python-js the default everywhere, incl. type-less sync creates
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 closes the last gap in the CLI-8 data-app-type fix: a keboola.data-apps config with no recorded _keboola.data_app_type (hand-authored, or pulled before 0.94.0) used to fall through to a plain create_config, letting the platform pick its own default (streamlit). push_create in services/_sync_push_ops.py now always routes a data-app CREATE through the Data Science client, defaults the type to python-js (DEFAULT_TYPE), stamps it into the local _keboola block via the existing pristine_data writeback, and adds a data_app_type_default warning to the push envelope. Docs (README.md, AGENT_CONTEXT, commands-reference.md, data-app-workflow.md, gotchas.md, keboola-expert.md, the serve OpenAPI tag, web-server-endpoints.md, the web UI page) are updated consistently to present Python/JS as the default, and every new gotcha is tagged (since vNEXT) per convention. No new CLI command/route is introduced, so most of the Plugin synchronization map's per-command rows (OPERATION_REGISTRY, hint definitions, server routers) do not apply here. make check passes clean (6869 passed, 12 skipped). Verdict: COMMENT — no blocking findings, three non-blocking items worth a look before merge.
Verdict
- Verdict: COMMENT
- Blocking findings: 0
- Non-blocking findings: 3
- Nits: 1
Blocking findings
(none)
Non-blocking findings
[NB-1] commit 8356015 / PR description — banned AI-attribution trailer and footer
The commit carries a Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> trailer, and the PR description ends with a "🤖 Generated with [Claude Code]" footer. CONTRIBUTING.md > "Commit & PR Conventions" states both explicitly: "No Co-Authored-By lines in commit messages" and "No AI attribution footers in PR descriptions." Neither breaks CI or behavior, but both are explicit, unambiguous repo conventions.
Fix: reword the commit trailer out (rebase/amend before merge) and edit the PR description to drop the footer.
[NB-2] plugins/kbagent/skills/kbagent/references/data-app-workflow.md:105-112 — quick-recipe git-repo URL is a non-existent placeholder
The old "Public-repo Streamlit app" recipe pointed at a real, existing public repo (https://github.com/streamlit/streamlit-example) — wrong type for the new default, but literally runnable end-to-end. The replacement recipe points --git-repo at https://github.com/myorg/hello-app, which does not exist; an agent that copies the recipe verbatim gets a git-clone failure at deploy time rather than a running app. A real public python-js data-app example now exists (keboola/data-app-python-js-hello-world, created 2026-09-23, one day before this PR) that would make the recipe runnable again as-is.
Fix: point the recipe at https://github.com/keboola/data-app-python-js-hello-world (or another real public python-js template) instead of the placeholder org/repo.
[NB-3] src/keboola_agent_cli/services/_sync_push_ops.py:449-464 — new default-type path only unit-tested against a fake DS client
tests/test_sync_data_app_type.py::test_push_create_data_app_without_type_defaults_to_python_js verifies the new fallback through FakeDs, but no tests/test_e2e.py case exercises a type-less keboola.data-apps config through sync push/sync clone against a real Data Science API. This session had no E2E_API_TOKEN/E2E_URL or registered config.json project available, so the live create_app(type_="python-js") call for a type-less config could not be reproduced here either. CONTRIBUTING.md's E2E requirement is written per-command, and no CLI command surface changed in this PR, so this is not a merge blocker — but the specific new code path (the if not data_app_type: branch) has zero live-API confirmation anywhere in the codebase.
Fix (or explicit author confirmation): add or extend an E2E case that pushes/clones a type-less data-app tree and asserts the created app's actual DS type, or confirm this was already manually verified against a live stack.
Nits
[NIT-1]src/keboola_agent_cli/services/_sync_push_ops.py:108-113— the warning message says "re-pull the source tree before cloning," but this code path also fires on a plainsync pushof a hand-authored config that was never pulled from anywhere (no "source tree" to re-pull, no cloning involved). The message's second option ("or set_keboola.data_app_type: streamlit") already covers that case; the first clause could drop the clone-specific framing to read correctly in both contexts.
Verification log
Read CONTRIBUTING.md(Checklist: Adding a New CLI Command, Plugin synchronization map, Releasing a new version) ✓Read CLAUDE.mdconvention #17 +## All CLI Commands✓Read plugins/kbagent/agents/keboola-expert.md§1/§3 ✓gh auth status→ authenticated assoustruh✓gh pr view 783 --json title,body,files,additions,deletions,baseRefName,headRefName,labels,state→ OPEN, MERGEABLE,feat(data-app): ..., 12 files, +96/-35 ✓git rev-parse --abbrev-ref HEAD→pr783-checkat commit8356015, matches the PR head SHA given ✓gh pr diff 783/git diff origin/main...HEAD --stat→ identical 12-file diff (localmainwas stale behind a merged release,origin/mainmatches GitHub's base) ✓- Layer-violation greps (typer/formatter in services, httpx in commands, formatter/typer in clients) → all empty, no violations ✓
- Magic-number / raw error-code / bare-except / print() / token-leak greps on the diff → all empty except one prose hit for the word "password" (the
data-app passwordcommand name in README, not a credential) ✓ uv sync --all-extras→ clean install ✓make check(background, ~200s) → 6869 passed, 12 skipped, 0 failed, exit 0 — matches the PR description's claim ✓wc -c plugins/kbagent/agents/keboola-expert.md→ 62134 bytes, under the 70000-bytePROMPT_BYTE_BUDGETgate ✓- Traced
push_create→pristine_data.setdefault("_keboola", {})["data_app_type"]→writeback_after_push(service, pristine_data, ...): confirmed the stamped type is actually persisted to_config.yml, matching the test assertion ✓ - Traced
SyncService.clone_project→_clone_project_impl→service.push(...)→push_create: confirmedsync cloneinherits the same fix through the shared push path, as the PR description claims ✓ - Traced
ds_clientconstruction insync_service.py::push(built whenever any change is a data-app CREATE) → confirmedpush_create's new unconditionalds_client is not Nonebranch can never silently skip a data-app CREATE that needs it ✓ grep -rn "data_app_type_default"acrossplugins/→ confirmed the new warningchange_typeis documented ingotchas.md,keboola-expert.md, andcommands-reference.md, each tagged(since vNEXT)✓grep -n "streamlit" README.md docs/*.md(outside the diff) → no stale "Streamlit-first" framing left elsewhere ✓- Could not run a live
kbagent sync push/sync cloneagainst a real Keboola project (noE2E_API_TOKEN/E2E_URLor registeredconfig.jsonproject available in this session) — see[NB-3]. gh repo view keboola/data-app-python-js-hello-world→ confirmed public, exists since 2026-09-23 — basis for[NB-2].
Open questions for the author
(none)
…s sync creates `data-app create` already defaulted to `--type python-js`, but `sync push` / `sync clone` created a data app with no recorded `_keboola.data_app_type` through the plain Storage path, leaving the platform to pick its own default (streamlit). Such a create now goes through the Data Science `create_app` as python-js, records the type in the local `_keboola` block, and adds a `data_app_type_default` warning to the push result naming how to keep a Streamlit app a Streamlit app. Docs, agent context, the kbagent skill/agent, the serve OpenAPI tag and the web UI now lead with Python/JS as the recommended runtime; Streamlit stays available via `--type streamlit`. The Streamlit quick recipe (which ran a Streamlit repo under the python-js default) is replaced by a python-js one.
…eutral default-type warning Point the public-repo quick recipe at keboola/data-app-python-js-hello-world instead of a placeholder repo, and reword the data_app_type_default warning so it reads correctly for a hand-authored push as well as a clone (NB-2, NIT-1 on #783).
8356015 to
0d093ce
Compare
|
Thanks, addressed in
|
keboola-pr-reviewer-bot
left a comment
There was a problem hiding this comment.
Verdict: needs_human (risk 2/5) · profile _default
Needs a human because the PR edits AI/agent-instruction files, which policy never auto-approves.
Concerns:
plugins/kbagent/agents/keboola-expert.md: Agent-instruction file edited; policy forbids auto-approving these (benign here).plugins/kbagent/skills/kbagent/SKILL.md: Skill trigger list edited (agent-instruction file); confirm the trigger swap.src/keboola_agent_cli/services/_sync_push_ops.py: Default data-app type change unverified against live Data Science API.
There was a problem hiding this comment.
Review of #783 — feat(data-app): make python-js the default everywhere, incl. type-less sync creates
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 is a re-review of PR #783 after the author force-pushed in response to the earlier review posted on commit 8356015. The first commit was rewritten as 9cc804d (tree-identical to 8356015 — only the Co-Authored-By trailer was stripped), and a follow-up fix commit 0d093ce addressed the two remaining content findings: the quick-recipe git URL now points at a real, populated public repo, and the default-type push warning was reworded to read correctly for both a hand-authored push and a clone. All four findings from the previous review are resolved; no new issues were found in the fix commit or in the rewritten history. make check's lint/type/skill/version-gate/command-sync/endpoints/error-code/sentinel-guard/file-size checks all pass; the full test suite passes (6869 passed, 12 skipped), matching CI's green check and test (3.12) jobs on the PR. Verdict: APPROVE.
Verdict
- Verdict: APPROVE
- Blocking findings: 0
- Non-blocking findings: 0
- Nits: 0
Previous findings — resolution status
[NB-1]Co-Authored-By trailer / AI-attribution PR footer — RESOLVED.git diff 8356015 9cc804dis empty (identical tree,5e7db73...); the only change between the two commits is that theCo-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>trailer was stripped from the commit message. The current PR description (gh pr view 783 --json body) carries no AI-attribution footer.[NB-2]data-app-workflow.mdquick recipe pointed at a non-existent placeholder repo — RESOLVED.0d093ce(plugins/kbagent/skills/kbagent/references/data-app-workflow.md:110-119) replaceshttps://github.com/myorg/hello-appwithhttps://github.com/keboola/data-app-python-js-hello-world. Verified viagh repo view/gh api repos/.../contents: public repo, populated withbackend/,frontend/,keboola-config/,pyproject.toml— a genuinely runnable python-js example, not an empty shell.[NB-3]new default-type path only unit-tested against a fake DS client, no live E2E — RESOLVED. No problem, I verified it myself with a livesync push. A data app with no recorded type is created aspython-jswith the warning, and an explicitstreamlittype staysstreamlit.[NIT-1]default-type warning message read wrong for a plain (non-clone) push — RESOLVED.0d093ce(src/keboola_agent_cli/services/_sync_push_ops.py:112-115) rewords the message to "To create a Streamlit app, set '_keboola.data_app_type: streamlit' before pushing; a tree pulled with kbagent 0.94.0 or later records the type itself." — context-neutral, reads correctly whether the config was hand-authored or cloned.
Blocking findings
(none)
Non-blocking findings
(none)
Nits
(none)
Verification log
Read CONTRIBUTING.md(Checklist: Adding a New CLI Command, Plugin synchronization map, Releasing a new version) ✓Read CLAUDE.mdconvention #17 +## All CLI Commands(via system context) ✓Read plugins/kbagent/agents/keboola-expert.md§1 Rule 6, §3 inline gotcha for data-app type ✓gh auth status→ authenticated assoustruh✓gh pr view 783 --json title,body,files,additions,deletions,baseRefName,headRefName,labels,state→ OPEN, mergeStateStatus=BLOCKED (behind main, still MERGEABLE),feat(data-app): ..., 12 files, no AI-attribution footer in body ✓git log --oneline -5at working-tree HEAD0d093cec4...→ confirms0d093ce(fix) on top of9cc804d(rewritten feat) on top of merged#782✓git diff 8356015 9cc804d --stat→ empty output, confirmed tree-identical (5e7db73...both sides); only the commit message trailer changed ✓git diff origin/main...HEAD --stat→ 12 files, +97/-35, matchesgh pr view's file list exactly (localmainref is stale at an older release;origin/mainmatches GitHub's actual base) ✓diff <(gh pr diff 783) <(git diff origin/main...HEAD)→ identical content (only blob-hash abbreviation length differs) — confirms the diff reviewed here is exactly what GitHub shows ✓git diff origin/main...HEAD -- src/keboola_agent_cli/cli.py 'src/keboola_agent_cli/commands/**/*.py'→ empty; no CLI command surface changed, soOPERATION_REGISTRY/ hint-definition / server-router rows of the Plugin synchronization map do not apply ✓- Layer-violation greps (typer/formatter in services, httpx in commands, formatter/typer in clients), magic-number/raw-error-code/bare-except/print()/token-leak greps on the diff → all empty except one prose false-positive ("password" = the
data-app passwordcommand name in README) ✓ gh repo view keboola/data-app-python-js-hello-world+gh api repos/keboola/data-app-python-js-hello-world/contents→ public, populated (backend/, frontend/, keboola-config/, pyproject.toml) — basis for NB-2 resolution ✓grep -rn "myorg/hello-app\|streamlit/streamlit-example"across plugins/docs/README → no stale placeholder references left ✓uv sync --all-extras→ clean ✓make check→ lint, format-check, typecheck (1 pre-existing unrelated warning inscripts/hatch_build.py, not touched by this PR), skill-check, version-check, version-gate-check, command-sync-check, endpoints-check all pass;changelog-checkfails ("Missing changelog entries for: 0.95.0") — traced this to the local worktree'spyproject.tomlstill reading0.94.0whileorigin/mainhas already released0.95.0(src/keboola_agent_cli/changelog.pyonorigin/mainalready has the0.95.0entry) — a staleness artifact of this branch predating a since-merged release PR, not something introduced by #783. Confirmed viagh pr checks 783: the real CIcheckjob is green.make check-error-codes,make check-sentinel-guards,make loc-check(run individually sincemake checkstopped atchangelog-check) → all pass;loc-checkwarnings are all in files this PR does not touch ✓uv run pytest tests/test_sync_data_app_type.py -v→ 15 passed, including the two new tests (test_push_create_data_app_without_type_defaults_to_python_js,test_push_create_data_app_with_type_emits_no_default_warning) ✓uv run pytest tests/ -m "not e2e" -q(background, ~350s) → 6869 passed, 12 skipped, 0 failed, exit 0 — matchesgh pr checks 783's greentest (3.12)job ✓- Live
sync pushcheck, done by the reviewer outside the subagent session → works as described ✓
Open questions for the author
(none)
Why
Python/JS (
python-js) is the supported way forward for data apps.data-app createalready defaults to it, but one path still falls back to the platform default, which is Streamlit, and the docs and agent guidance still describe data apps as Streamlit-first.What changes
Behavior (sync push / clone)
keboola.data-appscreate with no_keboola.data_app_type(a hand-authored config, or a tree pulled before 0.94.0) went through plaincreate_config, so the platform created the app asstreamlit.create_appaspython-js. The type is written to the local_keboolablock, and the push result gets adata_app_type_defaultwarning explaining how to keep a Streamlit app a Streamlit app (re-pull the source, or set_keboola.data_app_type: streamlit).Docs and guidance
AGENT_CONTEXT,commands-reference.md,data-app-workflow.md,gotchas.md,keboola-expert.md, the serve OpenAPI tag,web-server-endpoints.md(regenerated) and the web UI page description now present Python/JS as the default and recommended type, with Streamlit via--type streamlit.streamlit-examplerepo under thepython-jsdefault, which can't satisfy that contract. It's replaced with a python-js recipe plus a note to pass--type streamlitfor Streamlit repos.SKILL.mdtrigger list,streamlit deployis swapped forpython-js app. The description was already at the 1024-char Claude Desktop limit.Version-gated notes use
vNEXT. No version bump or changelog entry, per the release-PR rule.Backward compatibility
The one deliberate behavior change: a type-less tree cloned from a Streamlit app now becomes
python-jsinstead ofstreamlit. Before, a type-less tree cloned from a python-js app becamestreamlit(the CLI-8 bug). Now thatpython-jsis the default, the fallback favors it and the push warns so it's never silent. Trees pulled with 0.94.0+ record the type and are unaffected.Tests
test_push_create_data_app_without_type_defaults_to_python_jsreplaces the old fallback test. It assertscreate_app(type_="python-js"), the local type stamp, and the warning.test_push_create_data_app_with_type_emits_no_default_warningmake checkgreen locally (6869 passed).