Skip to content

feat(data-app): make python-js the default everywhere, incl. type-less sync creates - #783

Merged
soustruh merged 2 commits into
mainfrom
feat/data-app-python-js-default
Sep 25, 2026
Merged

soustruh merged 2 commits into
mainfrom
feat/data-app-python-js-default

Conversation

@matyas-jirat-keboola

@matyas-jirat-keboola matyas-jirat-keboola commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Why

Python/JS (python-js) is the supported way forward for data apps. data-app create already 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)

  • Before: a keboola.data-apps create with no _keboola.data_app_type (a hand-authored config, or a tree pulled before 0.94.0) went through plain create_config, so the platform created the app as streamlit.
  • Now: it goes through the Data Science create_app as python-js. The type is written to the local _keboola block, and the push result gets a data_app_type_default warning explaining how to keep a Streamlit app a Streamlit app (re-pull the source, or set _keboola.data_app_type: streamlit).
  • Configs that already record a type are unchanged.

Docs and guidance

  • README, 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.
  • The "Public-repo Streamlit app" quick recipe ran the streamlit-example repo under the python-js default, which can't satisfy that contract. It's replaced with a python-js recipe plus a note to pass --type streamlit for Streamlit repos.
  • In the SKILL.md trigger list, streamlit deploy is swapped for python-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-js instead of streamlit. Before, a type-less tree cloned from a python-js app became streamlit (the CLI-8 bug). Now that python-js is 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_js replaces the old fallback test. It asserts create_app(type_="python-js"), the local type stamp, and the warning.
  • test_push_create_data_app_with_type_emits_no_default_warning
  • make check green locally (6869 passed).
  • Not verified live against a Data Science API yet (see review NB-3).

@keboola-pr-reviewer-bot keboola-pr-reviewer-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 soustruh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review of #783 — feat(data-app): make python-js the default everywhere, incl. type-less sync creates

Generated by kbagent-pr-reviewer subagent. Verdict and findings below
are advisory; the human author retains every veto. CI-coverable issues
(lint, format, tests) are confirmed via make 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 plain sync push of 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.md convention #17 + ## All CLI Commands ✓
  • Read plugins/kbagent/agents/keboola-expert.md §1/§3 ✓
  • gh auth status → authenticated as soustruh ✓
  • 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-check at commit 8356015, matches the PR head SHA given ✓
  • gh pr diff 783 / git diff origin/main...HEAD --stat → identical 12-file diff (local main was stale behind a merged release, origin/main matches 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 password command 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-byte PROMPT_BYTE_BUDGET gate ✓
  • 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: confirmed sync clone inherits the same fix through the shared push path, as the PR description claims ✓
  • Traced ds_client construction in sync_service.py::push (built whenever any change is a data-app CREATE) → confirmed push_create's new unconditional ds_client is not None branch can never silently skip a data-app CREATE that needs it ✓
  • grep -rn "data_app_type_default" across plugins/ → confirmed the new warning change_type is documented in gotchas.md, keboola-expert.md, and commands-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 clone against a real Keboola project (no E2E_API_TOKEN/E2E_URL or registered config.json project 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).
@matyas-jirat-keboola
matyas-jirat-keboola force-pushed the feat/data-app-python-js-default branch from 8356015 to 0d093ce Compare September 24, 2026 11:03
@matyas-jirat-keboola

Copy link
Copy Markdown
Contributor Author

Thanks, addressed in 0d093ce (branch force-pushed):

  • NB-1: removed the Co-Authored-By trailer from the commit (amended) and the footer from the description.
  • NB-2: the recipe now points at keboola/data-app-python-js-hello-world.
  • NB-3: not verified live yet. No E2E credentials in this session, and I didn't want to add an E2E case I can't run. I'll either do a live push of a type-less data-app tree on a test project and post the result here, or add the E2E case. Either way it's done before merge.
  • NIT-1: reworded the warning so it reads correctly for a plain push too: "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."

make check green (6869 passed).

@keboola-pr-reviewer-bot keboola-pr-reviewer-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@soustruh soustruh left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review of #783 — feat(data-app): make python-js the default everywhere, incl. type-less sync creates

Generated by kbagent-pr-reviewer subagent. Verdict and findings below
are advisory; the human author retains every veto. CI-coverable issues
(lint, format, tests) are confirmed via make 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 9cc804d is empty (identical tree, 5e7db73...); the only change between the two commits is that the Co-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.md quick recipe pointed at a non-existent placeholder repo — RESOLVED. 0d093ce (plugins/kbagent/skills/kbagent/references/data-app-workflow.md:110-119) replaces https://github.com/myorg/hello-app with https://github.com/keboola/data-app-python-js-hello-world. Verified via gh repo view / gh api repos/.../contents: public repo, populated with backend/, 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 live sync push. A data app with no recorded type is created as python-js with the warning, and an explicit streamlit type stays streamlit.
  • [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.md convention #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 as soustruh ✓
  • 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 -5 at working-tree HEAD 0d093cec4... → confirms 0d093ce (fix) on top of 9cc804d (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, matches gh pr view's file list exactly (local main ref is stale at an older release; origin/main matches 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, so OPERATION_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 password command 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 in scripts/hatch_build.py, not touched by this PR), skill-check, version-check, version-gate-check, command-sync-check, endpoints-check all pass; changelog-check fails ("Missing changelog entries for: 0.95.0") — traced this to the local worktree's pyproject.toml still reading 0.94.0 while origin/main has already released 0.95.0 (src/keboola_agent_cli/changelog.py on origin/main already has the 0.95.0 entry) — a staleness artifact of this branch predating a since-merged release PR, not something introduced by #783. Confirmed via gh pr checks 783: the real CI check job is green.
  • make check-error-codes, make check-sentinel-guards, make loc-check (run individually since make check stopped at changelog-check) → all pass; loc-check warnings 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 — matches gh pr checks 783's green test (3.12) job ✓
  • Live sync push check, done by the reviewer outside the subagent session → works as described ✓

Open questions for the author

(none)

@soustruh
soustruh self-requested a review September 24, 2026 16:21
@soustruh
soustruh merged commit 6d764b6 into main Sep 25, 2026
6 checks passed
@soustruh
soustruh deleted the feat/data-app-python-js-default branch September 25, 2026 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants