Delta manifest: reduce update response sizes by 75-85% #1430
Conversation
📝 WalkthroughWalkthroughPropagates an optional oldVersionId through manifest/query helpers and request functions to enable delta manifests that exclude unchanged files. Adds getVersionIdByName to resolve version names to numeric ids, integrates it into the update flow, and extends tests for delta manifest scenarios. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant UpdateLogic as update.ts
participant PGHelpers as pg.ts
participant DB as Database
Client->>UpdateLogic: Request update (may include version_name & fetchManifestEntries)
alt fetchManifestEntries && version_name present
UpdateLogic->>PGHelpers: getVersionIdByName(appId, version_name)
PGHelpers->>DB: SELECT id FROM app_versions WHERE app_id=... AND name=...
DB-->>PGHelpers: oldVersionId or null
PGHelpers-->>UpdateLogic: oldVersionId
end
UpdateLogic->>PGHelpers: requestInfosPostgres(..., oldVersionId)
PGHelpers->>PGHelpers: build schema/manifest query (getSchemaUpdatesAlias)
alt oldVersionId provided
PGHelpers->>DB: SELECT manifest rows EXCLUDING files matching old manifest (NOT EXISTS)
else
PGHelpers->>DB: SELECT full manifest rows (COALESCE/json_agg)
end
DB-->>PGHelpers: manifest rows (delta or full)
PGHelpers-->>UpdateLogic: schema/manifest response
UpdateLogic-->>Client: update response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @supabase/functions/_backend/utils/pg.ts:
- Around line 532-559: Update getVersionIdByName to accept an optional boolean
allowedDeleted (default false) like getAppVersionPostgres, and when
allowedDeleted is false add a filter on schema.app_versions.deleted = false in
the drizzleClient query; modify the function signature to include
allowedDeleted, conditionally include the deleted-filter in the .where(...)
(mirroring the approach used in getAppVersionPostgres) and preserve the existing
early-return for internal names and error handling.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
supabase/functions/_backend/utils/pg.tssupabase/functions/_backend/utils/update.ts
🧰 Additional context used
📓 Path-based instructions (7)
supabase/functions/_backend/**/*.{ts,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
supabase/functions/_backend/**/*.{ts,js}: Backend code must be placed insupabase/functions/_backend/as shared code deployed to Cloudflare Workers (API/Plugin/Files workers), Supabase Edge Functions, and other platforms
UsecreateHonofromutils/hono.tsfor all Hono framework application initialization and routing
All database operations must usegetPgClient()orgetDrizzleClient()fromutils/pg.tsfor PostgreSQL access during active migration to Cloudflare D1
All Hono endpoint handlers must acceptContext<MiddlewareKeyVariables>and usec.get('requestId'),c.get('apikey'), andc.get('auth')for request context
Use structured logging withcloudlog({ requestId: c.get('requestId'), message: '...' })for all backend logging
UsemiddlewareAPISecretfor internal API endpoints andmiddlewareKeyfor external API keys; validate againstowner_orgin theapikeystable
Checkc.get('auth')?.authTypeto determine authentication type ('apikey' vs 'jwt') in backend endpoints
Use Drizzle ORM query patterns withschemafrompostgress_schema.tsfor all database operations; usealiasV2()for self-joins or multiple table references
Files:
supabase/functions/_backend/utils/update.tssupabase/functions/_backend/utils/pg.ts
supabase/functions/**/*.{ts,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Backend ESLint must pass before commit; run
bun lint:backendfor backend files
Files:
supabase/functions/_backend/utils/update.tssupabase/functions/_backend/utils/pg.ts
**/*.{vue,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Run
bun lintto lint Vue, TypeScript, and JavaScript files; usebun lint:fixto auto-fix issues
Files:
supabase/functions/_backend/utils/update.tssupabase/functions/_backend/utils/pg.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use single quotes and no semicolons per @antfu/eslint-config rules
Files:
supabase/functions/_backend/utils/update.tssupabase/functions/_backend/utils/pg.ts
supabase/functions/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
supabase/functions/**/*.ts: Never use the Supabase admin SDK with service key for user-facing APIs; always use client SDK with user authentication to enforce RLS policies. Admin SDK should only be used for internal operations (triggers, CRON jobs, etc.)
When admin access is unavoidable for a user-facing endpoint, sanitize all user inputs carefully to prevent PostgREST query injection
Files:
supabase/functions/_backend/utils/update.tssupabase/functions/_backend/utils/pg.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
Run
bun lintor lint/format command before validating any backend or frontend task to ensure consistent formatting
Files:
supabase/functions/_backend/utils/update.tssupabase/functions/_backend/utils/pg.ts
supabase/functions/_backend/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Place core backend logic in supabase/functions/_backend/ with plugins, private, public, triggers, and utils subdirectories
Files:
supabase/functions/_backend/utils/update.tssupabase/functions/_backend/utils/pg.ts
🧠 Learnings (5)
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to supabase/functions/_backend/**/*.{ts,js} : Use `createHono` from `utils/hono.ts` for all Hono framework application initialization and routing
Applied to files:
supabase/functions/_backend/utils/update.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to supabase/functions/_backend/**/*.{ts,js} : All database operations must use `getPgClient()` or `getDrizzleClient()` from `utils/pg.ts` for PostgreSQL access during active migration to Cloudflare D1
Applied to files:
supabase/functions/_backend/utils/update.tssupabase/functions/_backend/utils/pg.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to supabase/functions/_backend/**/*.{ts,js} : Use Drizzle ORM query patterns with `schema` from `postgress_schema.ts` for all database operations; use `aliasV2()` for self-joins or multiple table references
Applied to files:
supabase/functions/_backend/utils/pg.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to supabase/functions/_backend/utils/postgress_schema.ts : Schema definitions must be placed in `utils/postgress_schema.ts` using Drizzle ORM and never edited in committed migration files
Applied to files:
supabase/functions/_backend/utils/pg.ts
📚 Learning: 2026-01-10T04:55:25.264Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T04:55:25.264Z
Learning: Applies to supabase/seed.sql : Update supabase/seed.sql for new or evolved tests; keep fixtures focused on current behavior while leaving committed migrations unchanged
Applied to files:
supabase/functions/_backend/utils/pg.ts
🧬 Code graph analysis (1)
supabase/functions/_backend/utils/update.ts (1)
supabase/functions/_backend/utils/pg.ts (2)
getVersionIdByName(532-559)requestInfosPostgres(434-463)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run tests
🔇 Additional comments (6)
supabase/functions/_backend/utils/pg.ts (4)
325-348: Delta manifest SQL implementation looks correct.The NOT EXISTS subquery pattern is an efficient approach for delta filtering. The query correctly excludes files where both
file_nameANDfile_hashmatch the old version, ensuring only new or modified files are included.A few observations:
- The
oldVersionIdparameter is typed asnumber | null, preventing SQL injection- The fallback to full manifest when
oldVersionIdis falsy maintains backward compatibility- The FILTER clause with NOT EXISTS leverages the existing
manifesttable index onapp_version_id
352-361: Parameter propagation is correct.The
oldVersionIdparameter is properly added as an optional parameter and forwarded togetSchemaUpdatesAlias.
389-399: LGTM!Consistent parameter propagation pattern.
434-455: LGTM!The
oldVersionIdis correctly propagated to bothrequestInfosChannelDevicePostgresandrequestInfosChannelPostgres, ensuring delta manifest filtering works regardless of whether a channel override exists.supabase/functions/_backend/utils/update.ts (2)
18-18: LGTM!Import addition follows the existing pattern.
165-179: Delta manifest lookup implementation is correct.The logic correctly:
- Only performs the lookup when manifest fetching is enabled
- Guards against invalid/internal version names before making a DB call
- Logs the lookup result for debugging
The duplicate check for
'builtin'and'unknown'(line 168) alongside the check insidegetVersionIdByNameis a minor redundancy but serves as a useful short-circuit to avoid an unnecessary async call.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 997992400b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Look up old version ID for delta manifest calculation | ||
| // Only do this if we're fetching manifest entries | ||
| let oldVersionId: number | null = null | ||
| if (fetchManifestEntries && version_name && version_name !== 'builtin' && version_name !== 'unknown') { | ||
| oldVersionId = await getVersionIdByName(c, app_id, version_name, drizzleClient) |
There was a problem hiding this comment.
Because oldVersionId is derived from version_name and passed into the delta query, when a device is already on the latest version this lookup resolves to the same version ID, so the delta filter removes all manifest entries. The subsequent no_bundle guard (which runs before the no_new_version_available check) will then return no_bundle for manifest-only bundles (no r2_path/external_url) even though the device is up to date. This is a regression from always returning no_new_version_available in that scenario. Consider short‑circuiting before the manifest lookup or disabling delta when the old version equals the target version.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @tests/updates-manifest.test.ts:
- Around line 267-287: Update the top comment that currently reads "old (1.0.0)
and new (1.361.0)" to reflect the actual version assignments used by the
queries: the code selects '1.359.0' into oldVersion and '1.0.0' into newVersion
(which map to deltaOldVersionId and deltaNewVersionId respectively); ensure the
comment consistently states that old is 1.359.0 and new is 1.0.0 to match the
.select('id').eq('name', ...) calls and subsequent comments.
- Around line 378-416: The test 'returns empty manifest when all files are
identical' currently performs cleanup (deleting rows from supabase and resetting
manifest_count) at the end of the test block, which will be skipped if the test
fails; wrap the test logic that inserts manifest entries and calls postUpdate in
a try/finally so the finally always runs the cleanup (the same supabase.delete()
and app_versions update currently in the if (identicalVersion) block), or
alternatively move the cleanup into an afterAll that looks up the inserted
identicalVersion.id and performs the delete and manifest_count reset; reference
the variables/functions identicalVersion, insertMultipleManifestEntries,
postUpdate and the supabase deletes so the cleanup targets the same rows and
state.
🧹 Nitpick comments (2)
tests/updates-manifest.test.ts (2)
116-126: Consider batch insert for cleaner code.Supabase
.insert()supports array input for batch operations, which is cleaner and potentially more efficient than looping:♻️ Suggested batch insert
- // Insert all manifest entries - for (const entry of entries) { - const { error } = await supabase.from('manifest').insert({ + // Insert all manifest entries in a single batch + const { error } = await supabase.from('manifest').insert( + entries.map(entry => ({ app_version_id: appVersionId, file_name: entry.file_name, s3_path: entry.s3_path, file_hash: entry.file_hash, file_size: 100, - }) - if (error) - throw new Error(`Failed to insert manifest entry: ${error.message}`) - } + })) + ) + if (error) + throw new Error(`Failed to insert manifest entries: ${error.message}`)
313-342: Consider usingit.concurrent()for read-only tests.Per coding guidelines, tests should use
it.concurrent()to maximize parallelism. Tests 1-3 only read data set up inbeforeAlland could run concurrently.♻️ Use concurrent tests
- it('returns delta manifest with only changed/new files', async () => { + it.concurrent('returns delta manifest with only changed/new files', async () => {Apply similarly to the "returns full manifest for first install" and "returns full manifest when old version does not exist" tests.
Based on coding guidelines.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/updates-manifest.test.ts
🧰 Additional context used
📓 Path-based instructions (5)
tests/**/*.{ts,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Backend tests must use helpers from
tests/test-utils.tsincludinggetEndpointUrl(path)for correct worker routing andUSE_CLOUDFLARE_WORKERS=truefor CF Workers testing
Files:
tests/updates-manifest.test.ts
**/*.{vue,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Run
bun lintto lint Vue, TypeScript, and JavaScript files; usebun lint:fixto auto-fix issues
Files:
tests/updates-manifest.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use single quotes and no semicolons per @antfu/eslint-config rules
Files:
tests/updates-manifest.test.ts
tests/**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
tests/**/*.test.ts: ALL TEST FILES RUN IN PARALLEL - design tests accordingly with proper isolation and useit.concurrent()to maximize parallelism within the same file
Create dedicated seed data for tests that modify shared resources instead of reusing existing data; use unique naming conventions prefixed with test file name or feature being tested
Only reuse existing seed data if you only READ the data without modifying it, or create your OWN child resources under it without modifying the parent
Backend tests located in tests/ directory use Vitest test runner and require running Supabase instance
Files:
tests/updates-manifest.test.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
Run
bun lintor lint/format command before validating any backend or frontend task to ensure consistent formatting
Files:
tests/updates-manifest.test.ts
🧠 Learnings (1)
📚 Learning: 2026-01-10T04:55:25.264Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T04:55:25.264Z
Learning: Applies to tests/**/*.test.ts : Create dedicated seed data for tests that modify shared resources instead of reusing existing data; use unique naming conventions prefixed with test file name or feature being tested
Applied to files:
tests/updates-manifest.test.ts
🧬 Code graph analysis (1)
tests/updates-manifest.test.ts (1)
tests/test-utils.ts (6)
getSupabaseClient(385-404)resetAndSeedAppData(231-293)resetAppData(295-327)resetAppDataStats(358-383)getBaseData(216-218)postUpdate(475-482)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run tests
🔇 Additional comments (1)
tests/updates-manifest.test.ts (1)
105-108: LGTM!Delta test constants follow the established pattern with unique UUID-based app name for test isolation.
riderx
force-pushed
the
riderx/delta-manifest
branch
from
44ca2f5 to
6555826
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/updates-manifest.test.ts (1)
116-126: Consider batch insert for efficiency (optional).The sequential inserts work correctly but could be optimized using a single batch insert for better performance in tests with many entries.
♻️ Optional batch insert optimization
- // Insert all manifest entries - for (const entry of entries) { - const { error } = await supabase.from('manifest').insert({ - app_version_id: appVersionId, - file_name: entry.file_name, - s3_path: entry.s3_path, - file_hash: entry.file_hash, - file_size: 100, - }) - if (error) - throw new Error(`Failed to insert manifest entry: ${error.message}`) - } + // Insert all manifest entries in batch + const manifestEntries = entries.map(entry => ({ + app_version_id: appVersionId, + file_name: entry.file_name, + s3_path: entry.s3_path, + file_hash: entry.file_hash, + file_size: 100, + })) + const { error } = await supabase.from('manifest').insert(manifestEntries) + if (error) + throw new Error(`Failed to insert manifest entries: ${error.message}`)
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
supabase/functions/_backend/utils/pg.tssupabase/functions/_backend/utils/update.tstests/updates-manifest.test.ts
🧰 Additional context used
📓 Path-based instructions (9)
supabase/functions/_backend/**/*.{ts,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
supabase/functions/_backend/**/*.{ts,js}: Backend code must be placed insupabase/functions/_backend/as shared code deployed to Cloudflare Workers (API/Plugin/Files workers), Supabase Edge Functions, and other platforms
UsecreateHonofromutils/hono.tsfor all Hono framework application initialization and routing
All database operations must usegetPgClient()orgetDrizzleClient()fromutils/pg.tsfor PostgreSQL access during active migration to Cloudflare D1
All Hono endpoint handlers must acceptContext<MiddlewareKeyVariables>and usec.get('requestId'),c.get('apikey'), andc.get('auth')for request context
Use structured logging withcloudlog({ requestId: c.get('requestId'), message: '...' })for all backend logging
UsemiddlewareAPISecretfor internal API endpoints andmiddlewareKeyfor external API keys; validate againstowner_orgin theapikeystable
Checkc.get('auth')?.authTypeto determine authentication type ('apikey' vs 'jwt') in backend endpoints
Use Drizzle ORM query patterns withschemafrompostgress_schema.tsfor all database operations; usealiasV2()for self-joins or multiple table references
Files:
supabase/functions/_backend/utils/update.tssupabase/functions/_backend/utils/pg.ts
supabase/functions/**/*.{ts,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Backend ESLint must pass before commit; run
bun lint:backendfor backend files
Files:
supabase/functions/_backend/utils/update.tssupabase/functions/_backend/utils/pg.ts
**/*.{vue,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Run
bun lintto lint Vue, TypeScript, and JavaScript files; usebun lint:fixto auto-fix issues
Files:
supabase/functions/_backend/utils/update.tstests/updates-manifest.test.tssupabase/functions/_backend/utils/pg.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use single quotes and no semicolons per @antfu/eslint-config rules
Files:
supabase/functions/_backend/utils/update.tstests/updates-manifest.test.tssupabase/functions/_backend/utils/pg.ts
supabase/functions/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
supabase/functions/**/*.ts: Never use the Supabase admin SDK with service key for user-facing APIs; always use client SDK with user authentication to enforce RLS policies. Admin SDK should only be used for internal operations (triggers, CRON jobs, etc.)
When admin access is unavoidable for a user-facing endpoint, sanitize all user inputs carefully to prevent PostgREST query injection
Files:
supabase/functions/_backend/utils/update.tssupabase/functions/_backend/utils/pg.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
Run
bun lintor lint/format command before validating any backend or frontend task to ensure consistent formatting
Files:
supabase/functions/_backend/utils/update.tstests/updates-manifest.test.tssupabase/functions/_backend/utils/pg.ts
supabase/functions/_backend/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Place core backend logic in supabase/functions/_backend/ with plugins, private, public, triggers, and utils subdirectories
Files:
supabase/functions/_backend/utils/update.tssupabase/functions/_backend/utils/pg.ts
tests/**/*.{ts,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Backend tests must use helpers from
tests/test-utils.tsincludinggetEndpointUrl(path)for correct worker routing andUSE_CLOUDFLARE_WORKERS=truefor CF Workers testing
Files:
tests/updates-manifest.test.ts
tests/**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
tests/**/*.test.ts: ALL TEST FILES RUN IN PARALLEL - design tests accordingly with proper isolation and useit.concurrent()to maximize parallelism within the same file
Create dedicated seed data for tests that modify shared resources instead of reusing existing data; use unique naming conventions prefixed with test file name or feature being tested
Only reuse existing seed data if you only READ the data without modifying it, or create your OWN child resources under it without modifying the parent
Backend tests located in tests/ directory use Vitest test runner and require running Supabase instance
Files:
tests/updates-manifest.test.ts
🧠 Learnings (7)
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to supabase/functions/_backend/**/*.{ts,js} : Use `createHono` from `utils/hono.ts` for all Hono framework application initialization and routing
Applied to files:
supabase/functions/_backend/utils/update.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to supabase/functions/_backend/**/*.{ts,js} : All database operations must use `getPgClient()` or `getDrizzleClient()` from `utils/pg.ts` for PostgreSQL access during active migration to Cloudflare D1
Applied to files:
supabase/functions/_backend/utils/update.tssupabase/functions/_backend/utils/pg.ts
📚 Learning: 2026-01-10T04:55:25.264Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T04:55:25.264Z
Learning: Applies to tests/**/*.test.ts : Create dedicated seed data for tests that modify shared resources instead of reusing existing data; use unique naming conventions prefixed with test file name or feature being tested
Applied to files:
tests/updates-manifest.test.ts
📚 Learning: 2026-01-10T04:55:25.264Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T04:55:25.264Z
Learning: Applies to supabase/seed.sql : Update supabase/seed.sql for new or evolved tests; keep fixtures focused on current behavior while leaving committed migrations unchanged
Applied to files:
tests/updates-manifest.test.tssupabase/functions/_backend/utils/pg.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Backend tests modify local database; always reset database with `supabase db reset` before running tests to ensure clean state
Applied to files:
tests/updates-manifest.test.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to supabase/functions/_backend/**/*.{ts,js} : Use Drizzle ORM query patterns with `schema` from `postgress_schema.ts` for all database operations; use `aliasV2()` for self-joins or multiple table references
Applied to files:
supabase/functions/_backend/utils/pg.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to supabase/functions/_backend/utils/postgress_schema.ts : Schema definitions must be placed in `utils/postgress_schema.ts` using Drizzle ORM and never edited in committed migration files
Applied to files:
supabase/functions/_backend/utils/pg.ts
🧬 Code graph analysis (2)
supabase/functions/_backend/utils/update.ts (2)
supabase/functions/_backend/utils/pg.ts (2)
getVersionIdByName(532-559)requestInfosPostgres(434-463)supabase/functions/_backend/utils/logging.ts (1)
cloudlog(3-15)
tests/updates-manifest.test.ts (1)
tests/test-utils.ts (6)
getSupabaseClient(388-407)resetAndSeedAppData(234-296)resetAppData(298-330)resetAppDataStats(361-386)getBaseData(219-221)postUpdate(478-485)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run tests
🔇 Additional comments (8)
supabase/functions/_backend/utils/update.ts (1)
165-179: LGTM! Clean delta manifest integration.The delta manifest lookup is properly guarded by
fetchManifestEntriesand correctly excludes internal version names. The null fallback fromgetVersionIdByNamegracefully degrades to full manifest behavior in the downstream SQL.tests/updates-manifest.test.ts (4)
105-136: Good test isolation with dedicated app name.The delta tests use a separate
DELTA_APPNAMEwith proper setup/teardown, following the guideline to create dedicated seed data for tests that modify shared resources. Based on learnings, this is the correct approach.
261-306: Well-structured delta manifest test setup.The test setup clearly documents the manifest entry relationships between versions with helpful comments explaining what should be included/excluded in delta calculations. The version naming (1.359.0 as old, 1.0.0 as new on channel) aligns with the seed data structure.
313-342: Comprehensive delta manifest verification.The test properly validates that:
- Only changed/new files are returned (file_b.js, file_c.js)
- Unchanged files are excluded (file_a.js)
- File hashes match expected new values
378-416: Good inline cleanup pattern for test-specific modifications.The test modifies the 1.0.1 version to create an identical manifest scenario and properly cleans up afterwards. This prevents test pollution while keeping the test self-contained.
supabase/functions/_backend/utils/pg.ts (3)
532-559: Well-implemented version ID lookup with proper guards.The function correctly:
- Returns null for internal version names ('builtin', 'unknown')
- Uses Drizzle ORM patterns per coding guidelines
- Handles errors gracefully with
logPgErrorand null return- Is consistent with the existing
getAppVersionPostgrespattern
352-363: Clean parameter propagation through the call chain.The
oldVersionIdparameter is consistently propagated throughrequestInfosPostgres→requestInfosChannelDevicePostgres/requestInfosChannelPostgres→getSchemaUpdatesAlias. All signatures maintain backwards compatibility with the optional parameter.
325-348: Delta manifest SQL logic is properly indexed for efficiency.The NOT EXISTS subquery correctly filters out files that exist identically (same file_name AND file_hash) in the old version. The composite index
idx_manifest_file_name_hash_version(file_name, file_hash, app_version_id)created in migration 20250714021423_manifest_perf.sql covers all three columns used in the subquery's WHERE clause, ensuring efficient query execution. The conditional expression cleanly switches between delta and full manifest modes based onoldVersionId.
riderx
force-pushed
the
riderx/delta-manifest
branch
from
b6adb24 to
d0a5b4d
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @tests/updates-manifest.test.ts:
- Around line 412-418: The finalizer currently only resets manifest_count on
app_versions but not the app's manifest_bundle_count updated by
insertMultipleManifestEntries; after deleting the manifest rows and updating
app_versions in the finally block, also call supabase.from('apps').update({
manifest_bundle_count: 0 }).eq('id', identicalVersion.app_id') (or .eq('id',
identicalVersion.appId) if that is the field) to reset the app's
manifest_bundle_count to 0 so no stale state remains for other tests.
🧹 Nitpick comments (2)
tests/updates-manifest.test.ts (2)
105-136: Good test isolation with dedicated app name and helper.The helper
insertMultipleManifestEntriesproperly manages test data. However, inserting entries one-by-one in a loop creates N database round-trips.Consider batching the inserts for efficiency:
♻️ Batch insert for efficiency
async function insertMultipleManifestEntries(appVersionId: number, entries: { file_name: string, file_hash: string, s3_path: string }[]) { const supabase = getSupabaseClient() // First, delete any existing manifest entries for this version await supabase.from('manifest').delete().eq('app_version_id', appVersionId) - // Insert all manifest entries - for (const entry of entries) { - const { error } = await supabase.from('manifest').insert({ + // Insert all manifest entries in batch + const { error } = await supabase.from('manifest').insert( + entries.map(entry => ({ app_version_id: appVersionId, file_name: entry.file_name, s3_path: entry.s3_path, file_hash: entry.file_hash, file_size: 100, - }) - if (error) - throw new Error(`Failed to insert manifest entry: ${error.message}`) - } + })) + ) + if (error) + throw new Error(`Failed to insert manifest entries: ${error.message}`)
313-342: Consider usingit.concurrent()for parallel test execution.Per coding guidelines, test files should use
it.concurrent()to maximize parallelism. Since these tests operate on separate data scenarios (different version_name inputs), they could run concurrently.♻️ Use concurrent tests
- it('returns delta manifest with only changed/new files', async () => { + it.concurrent('returns delta manifest with only changed/new files', async () => {Apply similar change to other tests in this describe block.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
supabase/functions/_backend/utils/pg.tssupabase/functions/_backend/utils/update.tstests/updates-manifest.test.ts
🧰 Additional context used
📓 Path-based instructions (9)
tests/**/*.{ts,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Backend tests must use helpers from
tests/test-utils.tsincludinggetEndpointUrl(path)for correct worker routing andUSE_CLOUDFLARE_WORKERS=truefor CF Workers testing
Files:
tests/updates-manifest.test.ts
**/*.{vue,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Run
bun lintto lint Vue, TypeScript, and JavaScript files; usebun lint:fixto auto-fix issues
Files:
tests/updates-manifest.test.tssupabase/functions/_backend/utils/pg.tssupabase/functions/_backend/utils/update.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use single quotes and no semicolons per @antfu/eslint-config rules
Files:
tests/updates-manifest.test.tssupabase/functions/_backend/utils/pg.tssupabase/functions/_backend/utils/update.ts
tests/**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
tests/**/*.test.ts: ALL TEST FILES RUN IN PARALLEL - design tests accordingly with proper isolation and useit.concurrent()to maximize parallelism within the same file
Create dedicated seed data for tests that modify shared resources instead of reusing existing data; use unique naming conventions prefixed with test file name or feature being tested
Only reuse existing seed data if you only READ the data without modifying it, or create your OWN child resources under it without modifying the parent
Backend tests located in tests/ directory use Vitest test runner and require running Supabase instance
Files:
tests/updates-manifest.test.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
Run
bun lintor lint/format command before validating any backend or frontend task to ensure consistent formatting
Files:
tests/updates-manifest.test.tssupabase/functions/_backend/utils/pg.tssupabase/functions/_backend/utils/update.ts
supabase/functions/_backend/**/*.{ts,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
supabase/functions/_backend/**/*.{ts,js}: Backend code must be placed insupabase/functions/_backend/as shared code deployed to Cloudflare Workers (API/Plugin/Files workers), Supabase Edge Functions, and other platforms
UsecreateHonofromutils/hono.tsfor all Hono framework application initialization and routing
All database operations must usegetPgClient()orgetDrizzleClient()fromutils/pg.tsfor PostgreSQL access during active migration to Cloudflare D1
All Hono endpoint handlers must acceptContext<MiddlewareKeyVariables>and usec.get('requestId'),c.get('apikey'), andc.get('auth')for request context
Use structured logging withcloudlog({ requestId: c.get('requestId'), message: '...' })for all backend logging
UsemiddlewareAPISecretfor internal API endpoints andmiddlewareKeyfor external API keys; validate againstowner_orgin theapikeystable
Checkc.get('auth')?.authTypeto determine authentication type ('apikey' vs 'jwt') in backend endpoints
Use Drizzle ORM query patterns withschemafrompostgress_schema.tsfor all database operations; usealiasV2()for self-joins or multiple table references
Files:
supabase/functions/_backend/utils/pg.tssupabase/functions/_backend/utils/update.ts
supabase/functions/**/*.{ts,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Backend ESLint must pass before commit; run
bun lint:backendfor backend files
Files:
supabase/functions/_backend/utils/pg.tssupabase/functions/_backend/utils/update.ts
supabase/functions/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
supabase/functions/**/*.ts: Never use the Supabase admin SDK with service key for user-facing APIs; always use client SDK with user authentication to enforce RLS policies. Admin SDK should only be used for internal operations (triggers, CRON jobs, etc.)
When admin access is unavoidable for a user-facing endpoint, sanitize all user inputs carefully to prevent PostgREST query injection
Files:
supabase/functions/_backend/utils/pg.tssupabase/functions/_backend/utils/update.ts
supabase/functions/_backend/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Place core backend logic in supabase/functions/_backend/ with plugins, private, public, triggers, and utils subdirectories
Files:
supabase/functions/_backend/utils/pg.tssupabase/functions/_backend/utils/update.ts
🧠 Learnings (7)
📚 Learning: 2026-01-10T04:55:25.264Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T04:55:25.264Z
Learning: Applies to tests/**/*.test.ts : Create dedicated seed data for tests that modify shared resources instead of reusing existing data; use unique naming conventions prefixed with test file name or feature being tested
Applied to files:
tests/updates-manifest.test.ts
📚 Learning: 2026-01-10T04:55:25.264Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T04:55:25.264Z
Learning: Applies to supabase/seed.sql : Update supabase/seed.sql for new or evolved tests; keep fixtures focused on current behavior while leaving committed migrations unchanged
Applied to files:
tests/updates-manifest.test.tssupabase/functions/_backend/utils/pg.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Backend tests modify local database; always reset database with `supabase db reset` before running tests to ensure clean state
Applied to files:
tests/updates-manifest.test.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to supabase/functions/_backend/**/*.{ts,js} : Use Drizzle ORM query patterns with `schema` from `postgress_schema.ts` for all database operations; use `aliasV2()` for self-joins or multiple table references
Applied to files:
supabase/functions/_backend/utils/pg.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to supabase/functions/_backend/utils/postgress_schema.ts : Schema definitions must be placed in `utils/postgress_schema.ts` using Drizzle ORM and never edited in committed migration files
Applied to files:
supabase/functions/_backend/utils/pg.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to supabase/functions/_backend/**/*.{ts,js} : All database operations must use `getPgClient()` or `getDrizzleClient()` from `utils/pg.ts` for PostgreSQL access during active migration to Cloudflare D1
Applied to files:
supabase/functions/_backend/utils/pg.tssupabase/functions/_backend/utils/update.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to supabase/functions/_backend/**/*.{ts,js} : Use `createHono` from `utils/hono.ts` for all Hono framework application initialization and routing
Applied to files:
supabase/functions/_backend/utils/update.ts
🧬 Code graph analysis (2)
tests/updates-manifest.test.ts (1)
tests/test-utils.ts (4)
getSupabaseClient(388-407)resetAndSeedAppData(234-296)getBaseData(219-221)postUpdate(478-485)
supabase/functions/_backend/utils/update.ts (1)
supabase/functions/_backend/utils/pg.ts (2)
getVersionIdByName(532-561)requestInfosPostgres(434-463)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run tests
🔇 Additional comments (5)
supabase/functions/_backend/utils/pg.ts (2)
325-348: Delta manifest SQL logic is correctly implemented.The NOT EXISTS subquery properly excludes files that exist identically (same file_name AND file_hash) in the old version. This correctly includes:
- Changed files (same name, different hash)
- New files (name not in old version)
And excludes:
- Unchanged files (same name and hash)
434-455: LGTM: Parameter propagation is consistent.The
oldVersionIdparameter is correctly propagated through the entire call chain fromrequestInfosPostgrestogetSchemaUpdatesAlias.supabase/functions/_backend/utils/update.ts (3)
165-177: LGTM: Delta manifest lookup is appropriately guarded.The lookup correctly skips for
builtin/unknownversions and only runs when manifest fetching is enabled. The logging provides good observability for debugging delta manifest behavior.
202-211: Critical fix: Version check correctly moved before bundle check.This reordering prevents false "Cannot get bundle" errors when delta manifest returns empty (all files identical). The detailed comment explains the reasoning well.
18-18: LGTM: Import correctly added.
| export async function getVersionIdByName( | ||
| c: Context, | ||
| appId: string, | ||
| versionName: string, | ||
| drizzleClient: ReturnType<typeof getDrizzleClient>, | ||
| allowedDeleted?: boolean, | ||
| ): Promise<number | null> { | ||
| try { | ||
| // Return null for internal version names | ||
| if (!versionName || versionName === 'builtin' || versionName === 'unknown') | ||
| return null | ||
|
|
||
| const result = await drizzleClient | ||
| .select({ id: schema.app_versions.id }) | ||
| .from(schema.app_versions) | ||
| .where(and( | ||
| eq(schema.app_versions.app_id, appId), | ||
| eq(schema.app_versions.name, versionName), | ||
| ...(allowedDeleted !== undefined ? [eq(schema.app_versions.deleted, allowedDeleted)] : []), | ||
| )) | ||
| .limit(1) | ||
| .then(data => data[0]) | ||
|
|
||
| return result?.id ?? null | ||
| } | ||
| catch (e: unknown) { | ||
| logPgError(c, 'getVersionIdByName', e) | ||
| return null | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all calls to getVersionIdByName with context
rg -n 'getVersionIdByName\(' --type ts -A3 -B1Repository: Cap-go/capgo
Length of output: 897
🏁 Script executed:
#!/bin/bash
# Get full context of the getVersionIdByName call in update.ts
sed -n '150,190p' supabase/functions/_backend/utils/update.ts
# Find getAppVersionPostgres to compare implementation pattern
rg -n 'getAppVersionPostgres\(' --type ts -A20 | head -50Repository: Cap-go/capgo
Length of output: 6761
🏁 Script executed:
#!/bin/bash
# Check for comments around allowedDeleted parameter definitions
rg -n 'allowedDeleted' --type ts -B3 -A3 | head -80
# Search for any TODOs or comments about deleted versions in delta calculations
rg -n 'deleted.*delta|delta.*deleted' --type ts -iRepository: Cap-go/capgo
Length of output: 3648
Consider defaulting allowedDeleted to false in getVersionIdByName for clarity.
The function is called without the allowedDeleted parameter (line 169 in update.ts), meaning it defaults to undefined and skips the deletion filter entirely. This allows the query to return deleted versions when looking up the old version ID for delta manifest calculations.
While getAppVersionPostgres uses the same pattern, callers in stats.ts always explicitly set allowedDeleted (defaulting to false). For consistency and to avoid ambiguity about whether deleted versions should be returned in delta calculations, consider either:
- Explicitly passing
allowedDeleted: falseat the call site, or - Changing the function signature to default
allowedDeleted?: booleantofalseinstead ofundefined
| finally { | ||
| // Clean up: remove the manifest entries we just added | ||
| if (identicalVersion) { | ||
| await supabase.from('manifest').delete().eq('app_version_id', identicalVersion.id) | ||
| await supabase.from('app_versions').update({ manifest_count: 0 }).eq('id', identicalVersion.id) | ||
| } | ||
| } |
There was a problem hiding this comment.
Incomplete cleanup: manifest_bundle_count not reset on app.
The insertMultipleManifestEntries helper updates manifest_bundle_count on the app (line 134), but the cleanup only resets manifest_count on the version. This could leave stale state affecting other tests.
🐛 Fix cleanup to reset app manifest_bundle_count
finally {
// Clean up: remove the manifest entries we just added
if (identicalVersion) {
await supabase.from('manifest').delete().eq('app_version_id', identicalVersion.id)
await supabase.from('app_versions').update({ manifest_count: 0 }).eq('id', identicalVersion.id)
+ // Also reset manifest_bundle_count on the app to match beforeAll state
+ const { data: version } = await supabase.from('app_versions').select('app_id').eq('id', identicalVersion.id).single()
+ if (version) {
+ // Restore to the count from beforeAll (3 entries on deltaNewVersionId)
+ await supabase.from('apps').update({ manifest_bundle_count: 3 }).eq('app_id', version.app_id)
+ }
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| finally { | |
| // Clean up: remove the manifest entries we just added | |
| if (identicalVersion) { | |
| await supabase.from('manifest').delete().eq('app_version_id', identicalVersion.id) | |
| await supabase.from('app_versions').update({ manifest_count: 0 }).eq('id', identicalVersion.id) | |
| } | |
| } | |
| finally { | |
| // Clean up: remove the manifest entries we just added | |
| if (identicalVersion) { | |
| await supabase.from('manifest').delete().eq('app_version_id', identicalVersion.id) | |
| await supabase.from('app_versions').update({ manifest_count: 0 }).eq('id', identicalVersion.id) | |
| // Also reset manifest_bundle_count on the app to match beforeAll state | |
| const { data: version } = await supabase.from('app_versions').select('app_id').eq('id', identicalVersion.id).single() | |
| if (version) { | |
| // Restore to the count from beforeAll (3 entries on deltaNewVersionId) | |
| await supabase.from('apps').update({ manifest_bundle_count: 3 }).eq('app_id', version.app_id) | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In @tests/updates-manifest.test.ts around lines 412 - 418, The finalizer
currently only resets manifest_count on app_versions but not the app's
manifest_bundle_count updated by insertMultipleManifestEntries; after deleting
the manifest rows and updating app_versions in the finally block, also call
supabase.from('apps').update({ manifest_bundle_count: 0 }).eq('id',
identicalVersion.app_id') (or .eq('id', identicalVersion.appId) if that is the
field) to reset the app's manifest_bundle_count to 0 so no stale state remains
for other tests.
0528f5e to
69a70ac
Compare
Add server-side manifest delta computation to reduce response sizes by 75-85%. Instead of returning full manifest, only return files that are new or changed compared to device's current version using NOT EXISTS subquery optimization. Backwards compatible - gracefully falls back to full manifest for first installs, unknown versions, or missing old versions.
Add comprehensive tests for delta manifest functionality: - Delta returns only changed/new files (excludes unchanged) - First install (builtin) receives full manifest - Non-existent old version receives full manifest (fallback) - Identical manifests return empty delta
- Add allowedDeleted parameter to getVersionIdByName for consistency with getAppVersionPostgres - Move no_new_version_available check before no_bundle check to fix regression where manifest-only bundles on same version would return no_bundle instead of no_new_version_available - Fix stale comment about version assignments in tests - Add try/finally for proper test cleanup on failure Co-Authored-By: Claude Opus 4.5 <[email protected]>
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Co-Authored-By: Claude Opus 4.5 <[email protected]>
riderx
force-pushed
the
riderx/delta-manifest
branch
from
69a70ac to
42adc53
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/updates-manifest.test.ts (1)
110-136: Consider extracting shared logic withinsertManifestEntries.This helper duplicates the cleanup/insert/update pattern from
insertManifestEntries(lines 27-50). Both could share a common base, but given this is test code and the duplication is localized, this is optional.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
supabase/functions/_backend/utils/pg.tssupabase/functions/_backend/utils/update.tstests/updates-manifest.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- supabase/functions/_backend/utils/pg.ts
🧰 Additional context used
📓 Path-based instructions (9)
tests/**/*.{ts,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Backend tests must use helpers from
tests/test-utils.tsincludinggetEndpointUrl(path)for correct worker routing andUSE_CLOUDFLARE_WORKERS=truefor CF Workers testing
Files:
tests/updates-manifest.test.ts
**/*.{vue,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Run
bun lintto lint Vue, TypeScript, and JavaScript files; usebun lint:fixto auto-fix issues
Files:
tests/updates-manifest.test.tssupabase/functions/_backend/utils/update.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use single quotes and no semicolons per @antfu/eslint-config rules
Files:
tests/updates-manifest.test.tssupabase/functions/_backend/utils/update.ts
tests/**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
tests/**/*.test.ts: ALL TEST FILES RUN IN PARALLEL - design tests accordingly with proper isolation and useit.concurrent()to maximize parallelism within the same file
Create dedicated seed data for tests that modify shared resources instead of reusing existing data; use unique naming conventions prefixed with test file name or feature being tested
Only reuse existing seed data if you only READ the data without modifying it, or create your OWN child resources under it without modifying the parent
Backend tests located in tests/ directory use Vitest test runner and require running Supabase instance
Files:
tests/updates-manifest.test.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
Run
bun lintor lint/format command before validating any backend or frontend task to ensure consistent formatting
Files:
tests/updates-manifest.test.tssupabase/functions/_backend/utils/update.ts
supabase/functions/_backend/**/*.{ts,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
supabase/functions/_backend/**/*.{ts,js}: Backend code must be placed insupabase/functions/_backend/as shared code deployed to Cloudflare Workers (API/Plugin/Files workers), Supabase Edge Functions, and other platforms
UsecreateHonofromutils/hono.tsfor all Hono framework application initialization and routing
All database operations must usegetPgClient()orgetDrizzleClient()fromutils/pg.tsfor PostgreSQL access during active migration to Cloudflare D1
All Hono endpoint handlers must acceptContext<MiddlewareKeyVariables>and usec.get('requestId'),c.get('apikey'), andc.get('auth')for request context
Use structured logging withcloudlog({ requestId: c.get('requestId'), message: '...' })for all backend logging
UsemiddlewareAPISecretfor internal API endpoints andmiddlewareKeyfor external API keys; validate againstowner_orgin theapikeystable
Checkc.get('auth')?.authTypeto determine authentication type ('apikey' vs 'jwt') in backend endpoints
Use Drizzle ORM query patterns withschemafrompostgress_schema.tsfor all database operations; usealiasV2()for self-joins or multiple table references
Files:
supabase/functions/_backend/utils/update.ts
supabase/functions/**/*.{ts,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Backend ESLint must pass before commit; run
bun lint:backendfor backend files
Files:
supabase/functions/_backend/utils/update.ts
supabase/functions/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
supabase/functions/**/*.ts: Never use the Supabase admin SDK with service key for user-facing APIs; always use client SDK with user authentication to enforce RLS policies. Admin SDK should only be used for internal operations (triggers, CRON jobs, etc.)
When admin access is unavoidable for a user-facing endpoint, sanitize all user inputs carefully to prevent PostgREST query injection
Files:
supabase/functions/_backend/utils/update.ts
supabase/functions/_backend/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Place core backend logic in supabase/functions/_backend/ with plugins, private, public, triggers, and utils subdirectories
Files:
supabase/functions/_backend/utils/update.ts
🧠 Learnings (4)
📚 Learning: 2026-01-10T04:55:25.264Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T04:55:25.264Z
Learning: Applies to tests/**/*.test.ts : Create dedicated seed data for tests that modify shared resources instead of reusing existing data; use unique naming conventions prefixed with test file name or feature being tested
Applied to files:
tests/updates-manifest.test.ts
📚 Learning: 2026-01-10T04:55:25.264Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T04:55:25.264Z
Learning: Applies to supabase/seed.sql : Update supabase/seed.sql for new or evolved tests; keep fixtures focused on current behavior while leaving committed migrations unchanged
Applied to files:
tests/updates-manifest.test.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to supabase/functions/_backend/**/*.{ts,js} : All database operations must use `getPgClient()` or `getDrizzleClient()` from `utils/pg.ts` for PostgreSQL access during active migration to Cloudflare D1
Applied to files:
supabase/functions/_backend/utils/update.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to supabase/functions/_backend/**/*.{ts,js} : Use `createHono` from `utils/hono.ts` for all Hono framework application initialization and routing
Applied to files:
supabase/functions/_backend/utils/update.ts
🧬 Code graph analysis (1)
supabase/functions/_backend/utils/update.ts (5)
supabase/functions/_backend/utils/pg.ts (2)
getVersionIdByName(532-561)requestInfosPostgres(434-463)supabase/functions/_backend/utils/logging.ts (1)
cloudlog(3-15)supabase/functions/_backend/utils/version.ts (1)
version(1-1)supabase/functions/_backend/utils/stats.ts (1)
sendStatsAndDevice(86-96)supabase/functions/_backend/utils/hono.ts (1)
simpleError200(213-222)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run tests
🔇 Additional comments (10)
tests/updates-manifest.test.ts (5)
105-109: LGTM! Good test isolation with unique app name.Using the shared
idUUID to create a distinctDELTA_APPNAMEensures test isolation while maintaining consistency with the existing pattern in the file.
261-306: LGTM! Well-documented test setup.The beforeAll block clearly documents the test data scenario with comments explaining which files should be included/excluded in delta calculations. The manifest entry setup correctly models the expected delta behavior.
313-342: LGTM! Comprehensive delta manifest assertions.The test thoroughly verifies the delta behavior by checking:
- Correct number of files returned (2 instead of 3)
- Specific files included (changed + new)
- Unchanged file excluded
- Correct hashes on returned files
344-376: LGTM! Good coverage of fallback scenarios.These tests verify that the delta manifest gracefully falls back to full manifest when:
- Device is on first install (
builtin)- Device's version doesn't exist in the database
Both are essential for backwards compatibility.
378-420: LGTM! Proper test isolation with try/finally cleanup.Good practices:
- Sequential execution due to data modification (correctly documented)
try/finallyensures cleanup runs even on test failure- Tests the edge case of identical manifests returning empty delta
supabase/functions/_backend/utils/update.ts (5)
18-18: LGTM!Import of
getVersionIdByNamefrompg.tsfollows existing patterns and is necessary for the delta manifest lookup. Based on learnings, database operations should use utilities frompg.ts.
165-177: LGTM! Efficient conditional lookup.The explicit check for
builtinandunknownbefore callinggetVersionIdByNameavoids unnecessary database queries, since the function would returnnullanyway for these values (per the relevant code snippet at lines 531-560 of pg.ts). The structured logging aids debugging of delta manifest calculations.
179-179: LGTM!The
oldVersionIdparameter is correctly passed torequestInfosPostgres, matching the updated function signature inpg.ts.
202-211: Critical ordering fix for delta manifest correctness.Moving this check before the bundle validation (line 215) is essential. When delta calculation returns an empty manifest (all files unchanged), the old code path would incorrectly fail with
no_bundleerror becausemanifestEntries.length === 0. This reordering ensures the "already up to date" case is handled first.
215-219: The duplicate no-new-version check was successfully removed.The file now contains only one
version_name === version.namecheck at line 206, followed by the bundle check at lines 215-219. This ordering is correct and prevents falseno_bundleerrors when devices are already on the target version, since the no-new-version condition exits early before the manifest calculation occurs.
|
Summary (AI generated)
Implement server-side manifest delta computation to reduce response sizes by 75-85% for apps with large manifests. Only returns files that are new or changed compared to device's current version using an optimized NOT EXISTS subquery.
Motivation (AI generated)
Large apps (e.g., Codere with 3,253 files, ~1.4MB response) were returning full manifests even though devices already had most files from the previous version. Delta manifests reduce bandwidth and improve update performance.
Test Plan (AI generated)
Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.