fix: manifest load from local template path#4785
Merged
Conversation
MarioCadenas
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Collaborator
|
Commit: 433c936
18 interesting tests: 9 SKIP, 7 KNOWN, 2 flaky
Top 20 slowest tests (at least 2 minutes):
|
Collaborator
|
Commit: 7675d8c
101 interesting tests: 50 MISS, 16 flaky, 16 RECOVERED, 9 KNOWN, 8 FAIL, 1 PANIC, 1 SKIP
Top 50 slowest tests (at least 2 minutes):
|
github-merge-queue bot
pushed a commit
that referenced
this pull request
Mar 18, 2026
## Why The CLI's CODEOWNERS catch-all assigns 6 people to every PR outside a few narrow paths. This creates review noise and diffuses responsibility. We want targeted reviewer suggestions based on who actually worked on the changed code recently. ## Changes Before: Every PR touching core code auto-assigns all 6 CODEOWNERS. No signal about who is best suited to review. Now: A new GitHub Action analyzes git history of the changed files and posts a PR comment with two sections: - **Suggested reviewers** (1-3 people best suited based on recency-weighted git history) - **Eligible reviewers** (everyone from CODEOWNERS who could review, minus the suggested ones) This is additive only. CODEOWNERS and auto-assign stay unchanged. How it works: - Triggers on PR open, synchronize, and ready-for-review (skips drafts and fork PRs) - Classifies changed files by type (source=1.0, tests=0.3, acceptance=0.2, generated=0.0) - Scores contributors using recency-weighted commit history (half-life 150 days) - Resolves git author names to GitHub logins via the GitHub API (no hardcoded alias table to maintain) - Parses `.github/CODEOWNERS` to find eligible reviewers for the changed paths - Updates the comment in-place on re-runs (no notification churn) Implementation: a single Python script (`tools/suggest_reviewers.py`, 281 lines) and a minimal workflow YAML. ## Test plan - [x] Action ran on this PR itself and posted a comment successfully - [x] Verified script parses cleanly, passed `make checks`, passed `ruff format` - [x] Dry-run tested against 4 recent merged PRs with different characteristics: **PR #4784** (66 files, by pietern, big DABs rename): ``` ## Suggested reviewers - @denik -- recent work in `./`, `bundle/`, `cmd/bundle/generate/` Confidence: high ## Eligible reviewers @andrewnester, @anton-107, @lennartkats-db, @shreyas-goenka, @simonfaltum ``` Correctly identifies Denis as the clear top reviewer (2x second place score). All 6 CODEOWNERS shown as eligible. **PR #4782** (13 files, by denik, bundle engine priority): ``` ## Suggested reviewers - @andrewnester -- recent work in `bundle/schema/`, `bundle/internal/schema/`, `cmd/bundle/generate/` - @pietern -- recent work in `bundle/schema/`, `cmd/bundle/generate/`, `bundle/internal/schema/` - @shreyas-goenka -- recent work in `bundle/schema/`, `bundle/internal/schema/`, `cmd/bundle/` Confidence: medium ## Eligible reviewers @anton-107, @simonfaltum ``` Suggests 3 reviewers when scores are close. Remaining CODEOWNERS shown as eligible. **PR #4785** (2 files, by MarioCadenas, apps bug fix): ``` ## Suggested reviewers - @arsenyinfo -- recent work in `cmd/apps/` - @pietern -- recent work in `cmd/apps/` - @pkosiec -- recent work in `cmd/apps/` Confidence: low ## Eligible reviewers @databricks/eng-apps-devex ``` Correctly suggests apps-area contributors (not the catch-all CODEOWNERS). Shows the apps team as eligible. Low confidence since only 2 files. **PR #4774** (28 files, by denik, direct engine grants): ``` ## Suggested reviewers - @andrewnester -- recent work in `bundle/direct/dresources/`, `acceptance/bundle/invariant/` - @shreyas-goenka -- recent work in `bundle/direct/dresources/` Confidence: high ## Eligible reviewers @anton-107, @pietern, @simonfaltum ``` Correctly identifies the two main bundle/direct contributors. High confidence with clear score separation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This branch fixes
databricks apps manifestso it behaves like a true manifest viewer for AppKit templates, especially when resolving a local template viaDATABRICKS_APPKIT_TEMPLATE_PATHor--template.Preserve raw manifest output (
cmd/apps/manifest.go):Previously, the command loaded
appkit.plugins.jsoninto the CLI's internal manifest struct and then re-serialized it, which dropped fields not represented in that struct such asscaffolding,rules, and richer field metadata. The change now readsappkit.plugins.jsondirectly and writes the raw file contents to stdout, so local templates and newer manifest schema fields are preserved exactly as authored.Supporting infrastructure
cmd/apps/init_test.go— Strengthens manifest-related tests by asserting exact raw output instead of checking for a couple of JSON substrings.cmd/apps/init_test.go— Adds coverage for theDATABRICKS_APPKIT_TEMPLATE_PATHfallback sorunManifestOnly()is verified to resolve and print a local template manifest when no explicit--templateis provided.Other changes
apps manifesthelp text incmd/apps/manifest.goto clarify that it prints the manifest's raw contents.apps init; only the output format ofapps manifestchanged.