Skip to content

fix(frontend): show bundles in dependency compare picker (gate on native_packages, not manifest)#2373

Merged
WcaleNieWolny merged 2 commits into
mainfrom
wolny/distracted-gould-87d85d
May 30, 2026
Merged

fix(frontend): show bundles in dependency compare picker (gate on native_packages, not manifest)#2373
WcaleNieWolny merged 2 commits into
mainfrom
wolny/distracted-gould-87d85d

Conversation

@WcaleNieWolny
Copy link
Copy Markdown
Contributor

@WcaleNieWolny WcaleNieWolny commented May 30, 2026

Problem

On a bundle's Dependencies tab, the "Compare with bundle" dropdown is empty for many apps, so you can't compare two bundles' native dependencies at all.

The compare selector was originally built for the Manifest tab and later extracted into a shared BundleCompareSelect component and reused on the Dependencies tab (#1557). It carried over the Manifest tab's eligibility filter — manifest_count > 0 — to all three of its candidate queries (latest / deploy-history / search).

That filter is correct for manifest diffs (a bundle with no manifest has nothing to diff), but wrong for dependency diffs:

  • The Dependencies tab compares native_packages (a jsonb[] column on app_versions).
  • native_packages is populated on upload independently of the manifest. manifest/manifest_count is only populated for bundles using the partial-update system.
  • So any app whose bundles were uploaded without partial updates has manifest_count = 0 on every bundle — and the dependency compare picker filters them all out, even though every bundle has perfectly comparable native packages.

Reproduction / impact (real data)

For one affected app with 13 active bundles, every bundle has manifest_count = 0 and 3 native packages each:

Picker filter Bundles offered
manifest_count > 0 (old) 0 → empty dropdown
native_packages IS NOT NULL (new) 12

Fix

Add a compareMode prop ('manifest' | 'dependencies', default 'manifest') to BundleCompareSelect. A single buildCompareBaseQuery() helper now gates candidates on the column each tab actually diffs:

  • Manifest tabmanifest_count > 0 (unchanged behavior)
  • Dependencies tabnative_packages IS NOT NULL

The Dependencies page passes compare-mode="dependencies"; the Manifest page is untouched and keeps its existing (correct) behavior via the default.

Scope

  • src/components/bundle/BundleCompareSelect.vue — add compareMode prop + buildCompareBaseQuery() helper; route all three candidate queries (latest, deploy-history, search) through it.
  • src/pages/app/[app].bundle.[bundle].dependencies.vue — pass compare-mode="dependencies".

No schema, backend, or API changes. The actual dependency comparison logic on the page is unchanged — this only fixes which bundles are eligible to appear in the picker.

Test plan

  • On a bundle whose app has multiple bundles with native packages but manifest_count = 0, open Dependencies → "Compare with bundle" now lists other bundles (latest + searchable by name/ID).
  • Selecting a bundle shows the native-package diff (changed vs unchanged) as before.
  • Manifest tab compare dropdown behaves exactly as before (still gated on manifest_count > 0).
  • An app with only one bundle still shows an empty compare list (nothing to compare).

Summary by CodeRabbit

  • New Features
    • Added mode-aware bundle comparison that filters comparison candidates by either manifest data or native-package dependencies, showing only relevant bundle versions per mode.
    • Dependencies page now uses the dependencies comparison mode so native-package dependency comparisons surface appropriate versions.

Review Change Stack

The bundle compare selector was extracted from the Manifest tab (#1557) and
reused on the Dependencies tab, carrying over its `manifest_count > 0` filter.
That gate is correct for manifest diffs but wrong for dependency diffs: the
Dependencies tab compares `native_packages`, which is populated independently
of the manifest. Bundles uploaded without the partial-update manifest system
have `manifest_count = 0` but still carry native packages, so the compare
dropdown showed an empty list and dependency comparison was impossible.

Add a `compareMode` prop so each tab gates candidates on the column it actually
diffs: `manifest_count` for manifests, `native_packages` for dependencies.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 30, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: c91a8272-b4b7-44c8-8520-4c998de1a090

📥 Commits

Reviewing files that changed from the base of the PR and between e2551a0 and 850f256.

📒 Files selected for processing (1)
  • src/components/bundle/BundleCompareSelect.vue

📝 Walkthrough

Walkthrough

BundleCompareSelect gains a compareMode prop ('manifest' | 'dependencies', default 'manifest') and a buildCompareBaseQuery() helper that applies mode-aware Supabase filters (require native_packages for dependencies mode; manifest_count > 0 otherwise) across latest, preferred, and search compare flows. The dependencies page sets compare-mode="dependencies".

Changes

Bundle Compare Mode Support

Layer / File(s) Summary
Compare mode prop and query helper
src/components/bundle/BundleCompareSelect.vue
Introduces compareMode prop ('manifest' | 'dependencies', default 'manifest') and centralizes Supabase filtering in buildCompareBaseQuery(), used for latest-version loading.
Apply mode-aware filtering across comparison methods
src/components/bundle/BundleCompareSelect.vue
Applies buildCompareBaseQuery() to preferred-version resolution and search logic, replacing hardcoded manifest_count > 0 filters and isolating query builders per search branch.
Enable dependencies mode in dependencies view
src/pages/app/[app].bundle.[bundle].dependencies.vue
Adds compare-mode="dependencies" prop to BundleCompareSelect component.

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely describes the main change: fixing the dependency compare picker by changing the eligibility filter from manifest-based to native_packages-based.
Description check ✅ Passed The PR description provides comprehensive context (problem, fix, scope, test plan) but the test plan checkboxes are unchecked, indicating manual testing steps are not yet marked as completed.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.


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

@codspeed-hq
Copy link
Copy Markdown
Contributor

codspeed-hq Bot commented May 30, 2026

Merging this PR will not alter performance

✅ 43 untouched benchmarks
⏩ 2 skipped benchmarks1


Comparing wolny/distracted-gould-87d85d (850f256) with main (819909d)

Open in CodSpeed

Footnotes

  1. 2 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Copy link
Copy Markdown
Contributor

@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.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/components/bundle/BundleCompareSelect.vue (1)

235-264: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Fix unsafe reuse of the same Supabase query builder in searchCompareVersions

searchCompareVersions() builds one baseQuery and then chains both .ilike(...) and .eq('id', ...) off that same mutable builder inside Promise.all, so the second chain can leak/mix filters/options into the first request. Create a fresh buildCompareBaseQuery() for each query chain.

🛡️ Proposed fix — build an independent query per request
-  const baseQuery = buildCompareBaseQuery()
-    .neq('id', props.currentVersionId)
-
   const numericId = Number(term)
   let data: VersionRow[] | null = null
   let error: unknown = null

   if (Number.isNaN(numericId)) {
-    const response = await baseQuery
+    const response = await buildCompareBaseQuery()
+      .neq('id', props.currentVersionId)
       .ilike('name', `%${term}%`)
       .order('created_at', { ascending: false })
       .limit(5)

     if (requestId !== compareSearchRequestId.value)
       return

     data = response.data ?? null
     error = response.error
   }
   else {
     const [nameResponse, idResponse] = await Promise.all([
-      baseQuery
+      buildCompareBaseQuery()
+        .neq('id', props.currentVersionId)
         .ilike('name', `%${term}%`)
         .order('created_at', { ascending: false })
         .limit(5),
-      baseQuery
+      buildCompareBaseQuery()
+        .neq('id', props.currentVersionId)
         .eq('id', numericId)
         .order('created_at', { ascending: false })
         .limit(5),
     ])
🤖 Prompt for 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.

In `@src/components/bundle/BundleCompareSelect.vue` around lines 235 - 264, The
code reuses a single mutable Supabase query builder (baseQuery from
buildCompareBaseQuery()) for multiple concurrent queries in
searchCompareVersions, which can leak filters between chains; to fix, call
buildCompareBaseQuery() separately for each branch instead of reusing baseQuery
(e.g., create one local query via buildCompareBaseQuery() for the .ilike(...)
call and another fresh buildCompareBaseQuery() for the .eq('id', ...) call used
in Promise.all), ensuring each chain mutates its own query builder and then use
their responses as before.
🤖 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.

Outside diff comments:
In `@src/components/bundle/BundleCompareSelect.vue`:
- Around line 235-264: The code reuses a single mutable Supabase query builder
(baseQuery from buildCompareBaseQuery()) for multiple concurrent queries in
searchCompareVersions, which can leak filters between chains; to fix, call
buildCompareBaseQuery() separately for each branch instead of reusing baseQuery
(e.g., create one local query via buildCompareBaseQuery() for the .ilike(...)
call and another fresh buildCompareBaseQuery() for the .eq('id', ...) call used
in Promise.all), ensuring each chain mutates its own query builder and then use
their responses as before.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 0315486d-dd4e-4b19-b707-2d4cd4f0ab99

📥 Commits

Reviewing files that changed from the base of the PR and between 819909d and e2551a0.

📒 Files selected for processing (2)
  • src/components/bundle/BundleCompareSelect.vue
  • src/pages/app/[app].bundle.[bundle].dependencies.vue

The Supabase query builder is mutable and returns `this`, so reusing one
`baseQuery` instance across the two concurrent chains in the numeric-search
`Promise.all` mutated shared state and conflated their filters (name ilike +
id eq), returning near-nothing for numeric searches. Build an independent
query via buildCompareBaseQuery() for each chain.

Addresses CodeRabbit review on #2373.
@WcaleNieWolny
Copy link
Copy Markdown
Contributor Author

Addressed the CodeRabbit "outside diff range" finding on BundleCompareSelect.vue (searchCompareVersions): the mutable Supabase query builder was reused across the two concurrent chains in the numeric-search Promise.all, which could leak filters between requests. Fixed in 850f256 by building a fresh buildCompareBaseQuery() (each re-applying .neq('id', currentVersionId)) per chain. No inline thread existed to resolve (the flagged lines fell outside the PR diff).

@sonarqubecloud
Copy link
Copy Markdown

@WcaleNieWolny WcaleNieWolny merged commit eeaf275 into main May 30, 2026
54 of 55 checks passed
@WcaleNieWolny WcaleNieWolny deleted the wolny/distracted-gould-87d85d branch May 30, 2026 15:05
WcaleNieWolny added a commit that referenced this pull request May 31, 2026
* feat(frontend): bundle dependency compatibility view

Adds an OTA compatibility verdict and a status-aware dependency diff to the
bundle Dependencies tab, comparing the viewed bundle (candidate) against a
selected baseline bundle.

- Per-package status with colours: changed (blue, old -> new), added (green),
  removed (red), unchanged (gray).
- Compatibility verdict banner (compatible vs requires-app-store-update) using
  logic ported from the CLI's getCompatibilityDetails: version-range
  intersection + native checksum changes; new native plugins block OTA.
- Comparison is deep-linkable via ?compare=<versionId>.
- Compare picker excludes deleted bundles and falls back past deleted
  deployments when choosing preferred baselines.
- New unit-tested util src/services/bundleCompatibility.ts.

Builds on the compare-picker fix already merged in #2373.

* fix(frontend): apply picker filters on deep-link restore; address Sonar nits

- restoreCompareFromQuery now requires deleted=false and (dependencies mode)
  native_packages IS NOT NULL, matching the compare picker's eligibility. A
  ?compare= link to a deleted or package-less bundle is scrubbed instead of
  comparing against an empty baseline that reports every dep as added. (Codex P2)
- Replace the void operator on router.replace with a .catch() (Sonar S3735).
- Flip the negated ternary in the sort comparator (Sonar S7735).

* refactor(frontend): drop redundant Boolean() in checksum comparison

Use explicit != null guards so iosChanged/androidChanged are still typed as
boolean (bare && would infer string | boolean). Addresses CodeRabbit nitpick on #2375.

* fix(frontend): classify native-code-only changes as 'changed', not 'unchanged'

statusFor marked a package 'changed' only on a version-string difference, but
evaluateReasons also flags incompatibility when a native checksum changes at the
same version. That produced a contradictory row: a gray 'Unchanged' pill next to
red 'native code changed' text while the verdict reported incompatible.

Derive status from the computed reasons too, so a same-version bundle whose
native code changed is shown as 'changed' and stays consistent with the verdict.
Addresses CodeRabbit review on #2375.
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.

1 participant