Skip to content

feat(app): Adding Impulse Databricks App - #132

Merged
MaximHammer merged 8 commits into
mainfrom
feature/adhoc_reporting_app
Oct 27, 2026
Merged

MaximHammer merged 8 commits into
mainfrom
feature/adhoc_reporting_app

Conversation

@MaximHammer

Copy link
Copy Markdown
Contributor

Summary

Adds the Impulse App to this repo. It's a Databricks App for building Impulse reports step by step or with an assistant: it generates the report notebook, runs it as a serverless job and charts the results. The app is optional and self-contained. The databricks-impulse package, the framework tests and the framework CI don't change.

Changes

  • app/: the app, moved here from its own repository (without its history). It pins Impulse 0.10.0
  • .github/workflows/app.yml: a separate, non-required check for the app. It runs on PRs that change the app, and on main after framework changes, so framework PRs and their Codecov status aren't affected.
  • Skills: app/skills/impulse/ is a copy of skills/ from the pinned release. Framework skills are still edited only in skills/, and the app's CI fails if the copy differs from that release.
  • Docs: a new "Impulse App" section under /app (overview, requirements, install, architecture) and a button on the landing page. Existing /docs links stay the same.
  • README and CONTRIBUTING: short pointers to the app.

Testing

  • Unit tests added/updated: the app's unit tests and contract tests pass. The contract tests check every generated notebook against both the 0.10.0 release and the framework code in this repo.
  • Manual testing completed: deployed to a dev workspace. The app starts and serves its UI and API, and a sample report runs successfully on serverless.
  • Documentation updated: the docs build passes with strict link checks.

MaximHammer and others added 7 commits October 23, 2026 10:53
Adds the Impulse App, a Databricks App for building Impulse reports step by
step or with an assistant, as a self-contained project under
apps/adhoc-reporting. It is optional and not part of the databricks-impulse
package: the wheel and sdist contain the same files and code as before, only
the README and NOTICE they include gain an app section, and the root make
targets and CI don't include it.

Imported from the standalone impulse-app repository at commit 85a71a7 (main),
without history, then adapted to its new home:

- Lockfiles point to public PyPI and npm instead of internal mirrors;
  `make lock` keeps them there.
- Own [tool.uv] table and .python-version (3.11), so uv doesn't apply the root
  project's settings; project installs use --frozen, so a local package mirror
  can't rewrite uv.lock.
- `make deploy` only checks the app folder for uncommitted changes.
- The bundle and the default app name are impulse-adhoc-reporting instead of
  impulse-v4.
- Generated notebooks call report.run() and run(persist_results=False) instead
  of the deprecated determine_report() and persist_results(); the contract test
  binds every call against the real Report.run signature.
- Pins Impulse 0.10.0, with the vendored framework skills re-synced.
- `make test FRAMEWORK=../..` builds the contract tests against the framework
  source in this repo.
- The install guide moves to the docs site; the README keeps the development
  notes.

The root README and CONTRIBUTING point to the app, and NOTICE lists its
dependencies.

Co-authored-by: Isaac <no-reply@databricks.com>
Runs the app's unit tests, its contract tests against the pinned framework
release and against the framework source of the same commit, and the frontend
build. It runs on PRs that change the app and, on main, also after framework
changes (src/), so a framework change that breaks the app's generated
notebooks shows up right after it lands. Framework PRs don't run it, because
Codecov holds back its status while any other check fails; to check a
framework branch before merging, start the workflow by hand. It isn't a
required check and uploads no coverage, so the framework's CI and Codecov
status stay as they are.

Co-authored-by: Isaac <no-reply@databricks.com>
Adds a second docs instance under /app with Overview, Requirements, Install
and Architecture pages for the Impulse App. The navbar entry "Docs" becomes
"Impulse Analytics Framework"; it still opens Motivation, and all existing
/docs URLs stay the same. The landing page gets an "Impulse App" button.

Co-authored-by: Isaac <no-reply@databricks.com>
apps/adhoc-reporting/skills/impulse is a copy of the root skills/ at the
release the app pins. The app's tests compared it only with its own checksum
manifest, which the sync script writes, so an edit that also updated the
manifest went unnoticed. `sync_framework.py --check` copies skills/ from the
pinned tag again and fails if the result differs; the app workflow runs it as
`make check-skills`.

CONTRIBUTING and the app README say that framework skills are edited only in
skills/ and reach the app with its next pin bump.

Co-authored-by: Isaac <no-reply@databricks.com>
The root NOTICE.md is back to the framework's version: setuptools packages it
into the databricks-impulse wheel and sdist, which don't contain the app. The
app gets its own NOTICE.md that lists only its direct dependencies, as the
root NOTICE does for the docs site. It has no versions, so dependency updates
don't need an edit; the dependencies aren't distributed with the repository,
and the app's lockfiles pin the full graph.

Co-authored-by: Isaac <no-reply@databricks.com>
Drops "adhoc-reporting" from the app's folder, bundle and default app name.
Everywhere else the product is the Impulse App, and the framework uses "ad-hoc
analysis" for its mode without gold tables, which the app doesn't offer. The
app now lives in app/, which matches its docs under /app; the bundle and the
default app name are impulse-app, the workflow is app.yml, and
`make test FRAMEWORK=..` builds the contract tests against the framework
source. The new bundle name starts a new deployment, which affects no
installs, because the app hasn't been released from this repository yet.

Co-authored-by: Isaac <no-reply@databricks.com>
Drops the setup-python step. The Makefile already asks uv for Python 3.11
(the app) and 3.12 (the framework contract tests), and uv downloads them when
the runner doesn't have them, as in the other Databricks Labs workflows on the
same runner group. The workflow now uses the same setup actions as the
framework's CI, plus setup-node for the frontend.

Co-authored-by: Isaac <no-reply@databricks.com>
@MaximHammer
MaximHammer requested a review from a team as a code owner October 23, 2026 12:07
@codecov

codecov Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.30%. Comparing base (e3daaa4) to head (c1b7760).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #132   +/-   ##
=======================================
  Coverage   90.30%   90.30%           
=======================================
  Files          68       68           
  Lines        6068     6068           
  Branches      758      758           
=======================================
  Hits         5480     5480           
  Misses        466      466           
  Partials      122      122           
Flag Coverage Δ
query_engine 86.80% <ø> (ø)
reporting 94.92% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tombonfert tombonfert left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

1. Validate report names in the /api/reports/{name} routes (app/server/routes/reports.py)

The name path parameter is never checked against NAME_RE, but store.delete and runs.delete build workspace paths from it and delete them with recursive=True. A request like DELETE /api/reports/%2E%2E deletes:

  • /Users/{email}/Impulse/.., which is the user's home folder (user token)
  • /Users/{sp}/Impulse/{email}/.., which holds every user's published notebooks (service principal)

The get, put, runs and results routes have the same gap. Today only chat._report validates the name. I'd suggest validating it once for all routes, for example:

ReportName = Annotated[str, Path(pattern=NAME_RE.pattern)]

FastAPI then rejects bad names with a 422 before the handler runs.

2. Move the app to apps/adhoc-reporting/

We may build more apps on top of Impulse in the future, so each one should get its own folder under apps/. Please move app/ to apps/adhoc-reporting/ and update the docs and CI paths to match.

…ting

The routes under /api/reports/{name} used the name in workspace paths
without checking it. store.delete and runs.delete delete the report's
folders recursively, so DELETE /api/reports/%2E%2E asked the workspace to
delete /Users/<user>/Impulse/.. with the user's token and
/Users/<service principal>/Impulse/<user>/.. as the service principal.
All of them now declare the name as ReportName, a path parameter with the
NAME_RE pattern, so FastAPI rejects other names with a 422 before the
handler runs. Unit tests send .. and . to every route that takes a name.

The app moves to apps/adhoc-reporting, so that each app built on Impulse
gets its own folder under apps/. The CI path filters, its working
directory, `make test FRAMEWORK=../..` and the paths in the README,
CONTRIBUTING and the app docs follow. The bundle and default app name
(impulse-app), the workflow and the docs URL (/app) don't change.

Co-authored-by: Isaac <no-reply@databricks.com>

@tombonfert tombonfert left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM!

@MaximHammer
MaximHammer merged commit 3224ca8 into main Oct 27, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants