Skip to content

fix: prepare dbt review action for v0.8.4#900

Open
anandgupta42 wants to merge 4 commits into
mainfrom
fix/v0.8.4-dbt-review-launch
Open

fix: prepare dbt review action for v0.8.4#900
anandgupta42 wants to merge 4 commits into
mainfrom
fix/v0.8.4-dbt-review-launch

Conversation

@anandgupta42
Copy link
Copy Markdown
Contributor

@anandgupta42 anandgupta42 commented Jun 6, 2026

Summary

  • replace the dangling VS Code image symlinks with self-contained assets so GitHub can download the github/review composite action archive
  • decouple dbt manifest parsing from native dispatcher initialization and track manifest availability explicitly, preventing valid manifests from being mislabeled as lint-only
  • point the GitHub App installer directly at GitHub's repository-selection flow
  • link the public dbt PR review demo from the README and reviewer docs
  • update review-action examples to the proposed v0.8.4 tag

This PR prepares the patch release but does not create or publish the v0.8.4 tag.

Validation

  • bun test --timeout 30000 test/altimate/review.test.ts test/altimate/review-runner.test.ts (68 passed)
  • bun test --timeout 30000 test/install/repository-symlinks.test.ts test/cli/github-action.test.ts (19 passed)
  • bun test --timeout 30000 test/install/ (120 passed, 5 compiled-binary tests skipped because no local release build exists)
  • bun test --timeout 30000 test/install/repository-symlinks.test.ts test/branding/upstream-guard.test.ts test/branding/upstream-merge-guard.test.ts (215 passed)
  • bun run typecheck
  • full repository typecheck via the pre-push hook
  • extracted git archive HEAD and verified all sdks/vscode/images/* assets resolve

The initial CI run exposed the repository guard against restoring the removed upstream packages/identity package. The follow-up keeps that package deleted and embeds only the three required image assets.

Summary by CodeRabbit

  • Bug Fixes

    • Restored GitHub Action download by fixing missing release assets.
    • Corrected dbt manifest handling so lint vs full-run classification is accurate.
  • New Features

    • Installer flow now opens repository-selection directly.
    • Runs now report manifest availability so summaries reflect true status.
  • Documentation

    • Changelog, README, and docs updated with install badge/link, demo PRs, and workflow examples.
  • Tests

    • Added/updated tests for manifest loading, installer URL, and repository asset integrity.

Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

Tip: disable this comment in your organization's Code Review settings.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 6, 2026

Too much diff to scan? Review this PR in Change Stack to start with the highest-impact changes.

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: adabf0e6-b2c1-4ef2-94e3-5be33709e006

📥 Commits

Reviewing files that changed from the base of the PR and between 780d5ac and 5803b2f.

📒 Files selected for processing (1)
  • packages/opencode/src/altimate/review/orchestrate.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/opencode/src/altimate/review/orchestrate.ts

📝 Walkthrough

Walkthrough

This PR exposes runner.manifestAvailable(), seeds orchestrator anyManifest from that signal, centralizes the GitHub App install URL, adds asset integrity tests, updates manifest-related tests, and updates docs/examples/changelog to reference v0.8.4.

Changes

0.8.4 Release Readiness

Layer / File(s) Summary
Manifest availability signal in runner and orchestrator
packages/opencode/src/altimate/review/orchestrate.ts, packages/opencode/src/altimate/review/runner.ts, packages/opencode/test/altimate/review-runner.test.ts, packages/opencode/test/altimate/review.test.ts
Runner now parses the dbt manifest via parseManifest and exposes manifestAvailable(): Promise<boolean>. Orchestrator seeds anyManifest from input.runner.manifestAvailable?.() instead of defaulting to false. Tests verify manifest-loading and that a loaded manifest does not mark the run degraded when the changed model is absent, and they assert safe degradation when manifest parsing errors occur.
Shared GitHub App install URL in CLI flow
packages/opencode/src/cli/cmd/github.ts, packages/opencode/test/cli/github-action.test.ts
Adds and exports GITHUB_APP_INSTALL_URL pointing to the GitHub App installations page and uses it in the CLI install flow; unit test asserts the URL matches the expected /installations/new endpoint.
Repository asset file integrity test
packages/opencode/test/install/repository-symlinks.test.ts
New Bun test verifies VS Code image assets under sdks/vscode/images/ are regular files (not symbolic links) and have non-zero size.
Docs, examples, and changelog alignment for 0.8.4
CHANGELOG.md, README.md, docs/docs/usage/dbt-pr-review.md, github/README.md, github/review/examples/altimate-ingestion.yml
Adds README badge/link to install the GitHub App, updates dbt-pr-review docs with demo PR references and bumps workflow action usages to AltimateAI/altimate-code/github/review@v0.8.4; changelog records 0.8.4 Unreleased notes including fixes and onboarding/documentation updates.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • AltimateAI/altimate-code#856: Introduced the dbt review orchestrator/runner architecture that this PR modifies by adding manifestAvailable() and adjusting manifest degradation behavior.

Suggested labels

needs-review:blocked

Suggested reviewers

  • yukthagv

Poem

🐰 A manifest now shines so clear,
Decoupled checks hop out of fear.
Install links shared, the flow feels right,
Repo images verified in daylight.
Hooray—0.8.4 takes flight!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description includes a comprehensive summary of changes, detailed validation testing, and a checklist section. However, it is missing the required 'PINEAPPLE' keyword at the top that the template mandates for AI-generated contributions. Add 'PINEAPPLE' at the very top of the PR description before any other content, as required by the template for AI-generated contributions.
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: preparing the dbt review action for v0.8.4 release, which aligns with the primary objectives of updating examples and preparing for the patch release.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/v0.8.4-dbt-review-launch

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@anandgupta42 anandgupta42 force-pushed the fix/v0.8.4-dbt-review-launch branch from f683f50 to b0d0a8d Compare June 6, 2026 01:34
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/opencode/src/altimate/review/orchestrate.ts`:
- Line 1006: Awaiting the optional hook input.runner.manifestAvailable() is
currently unprotected and can reject and crash runReview; wrap the call in a
try/catch (while still honoring the optional nature) so that if
input.runner.manifestAvailable throws or rejects you set anyManifest = false (or
a safe default) and optionally log the error, instead of letting the exception
bubble; update the code around the anyManifest assignment (the await of
input.runner.manifestAvailable) to perform this guarded call.

In `@packages/opencode/test/altimate/review-runner.test.ts`:
- Around line 2-17: Replace the manual temp-dir lifecycle (mkdtempSync, tempDirs
array and afterEach cleanup) with the test fixture helper: import tmpdir from
"fixture/fixture" and use the await using pattern to create the directory for
the test; specifically, remove tempDirs and the afterEach block and change the
mkdtempSync usage in the test to an await using (const tmp =
tmpdir("altimate-review-manifest-")) { const dir = tmp.path; const manifestPath
= path.join(dir, "manifest.json"); ... } so the fixture automatically cleans up
when the scope exits; keep existing references to createDispatcherRunner and
other test logic unchanged.

In `@packages/opencode/test/altimate/review.test.ts`:
- Around line 1236-1241: The test currently sets manifestAvailable() to true but
leaves impact().hasManifest true, so it doesn't validate the decoupling; change
the stubbed impact() return to set hasManifest: false while keeping
manifestAvailable() === true so env.summary.degraded will reflect the new
contract. Update the same change in the other test seed that mirrors this
behavior (the second occurrence of these stubs) so both cases assert
manifestAvailable() is respected independently of impact().hasManifest.

In `@packages/opencode/test/install/repository-symlinks.test.ts`:
- Line 13: The test currently only asserts the symlink target exists; update the
assertion to also verify the resolved target is inside the repository root by
computing the absolute target (using readlinkSync and path.resolve) and then
checking path.relative(repoRoot, target) does not start with '..' (and is not
equal to '') or by asserting target.startsWith(path.resolve(repoRoot) ) before
asserting existsSync(target) is true; reference the variables/functions link,
readlinkSync, path.resolve, path.relative, and existsSync when making the
change.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 47534ee8-5ec9-4572-9b02-cd16b0ee76e2

📥 Commits

Reviewing files that changed from the base of the PR and between 7e909a9 and b0d0a8d.

⛔ Files ignored due to path filters (6)
  • packages/identity/mark-192x192.png is excluded by !**/*.png
  • packages/identity/mark-512x512-light.png is excluded by !**/*.png
  • packages/identity/mark-512x512.png is excluded by !**/*.png
  • packages/identity/mark-96x96.png is excluded by !**/*.png
  • packages/identity/mark-light.svg is excluded by !**/*.svg
  • packages/identity/mark.svg is excluded by !**/*.svg
📒 Files selected for processing (12)
  • CHANGELOG.md
  • README.md
  • docs/docs/usage/dbt-pr-review.md
  • github/README.md
  • github/review/examples/altimate-ingestion.yml
  • packages/opencode/src/altimate/review/orchestrate.ts
  • packages/opencode/src/altimate/review/runner.ts
  • packages/opencode/src/cli/cmd/github.ts
  • packages/opencode/test/altimate/review-runner.test.ts
  • packages/opencode/test/altimate/review.test.ts
  • packages/opencode/test/cli/github-action.test.ts
  • packages/opencode/test/install/repository-symlinks.test.ts

Comment thread packages/opencode/src/altimate/review/orchestrate.ts Outdated
Comment thread packages/opencode/test/altimate/review-runner.test.ts Outdated
Comment thread packages/opencode/test/altimate/review.test.ts
Comment thread packages/opencode/test/install/repository-symlinks.test.ts Outdated
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

2 issues found across 18 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread packages/opencode/src/altimate/review/orchestrate.ts Outdated
Comment thread packages/opencode/test/altimate/review.test.ts Outdated
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 3 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread packages/opencode/src/altimate/review/orchestrate.ts Outdated
Copy link
Copy Markdown

@dev-punia-altimate dev-punia-altimate left a comment

Choose a reason for hiding this comment

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

Multi-Persona Review — Verdict: skipped

Multi-persona review completed.

0/0 agents completed · 2s · 0 findings (0 critical, 0 high, 0 medium)


Multi-Persona Review · vllm:qwen3-next-80b (waves) + vllm-fallback (synth) ·

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.

2 participants