perf: slim down release targets server response#869
Merged
adityachoudhari26 merged 1 commit intomainfrom Mar 26, 2026
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughNarrowed Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/workspace-engine/pkg/db/queries/release_targets.sql (1)
81-102:⚠️ Potential issue | 🟠 Major
intervalSecondsis still required by the published verification schema.This query no longer returns
jvm.interval_seconds, butlatestJob.verifications[].metrics[]is still typed asVerificationMetricStatusinapps/workspace-engine/oapi/openapi.jsonandpackages/workspace-engine-sdk/src/schema.ts, whereintervalSecondsremains required. The endpoint will now emit payloads that don't satisfy the generated contract. Keep selecting/serializing that field, or slim the verification schemas and regenerate clients in the same change.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/workspace-engine/pkg/db/queries/release_targets.sql` around lines 81 - 102, The SQL query ListVerificationMetricsByJobIDs removed the jvm.interval_seconds column but consumers still expect VerificationMetricStatus.intervalSeconds; restore the column in the SELECT (e.g., select jvm.interval_seconds AS metric_interval_seconds or the original name used by serialization) so the query result matches the openapi/schema types, or if you intend to remove it, update the VerificationMetricStatus schema in the OpenAPI (apps/workspace-engine/oapi/openapi.json) and regenerate the SDK (packages/workspace-engine-sdk) to keep API contract and clients in sync; locate the query named ListVerificationMetricsByJobIDs and adjust the SELECT or the schema accordingly.
🧹 Nitpick comments (2)
apps/workspace-engine/svc/http/server/openapi/release_targets/getters.go (1)
87-146: Prefer the generatedReleaseTargetItemtype over ad-hocgin.Hmaps.This block is manually re-declaring the response contract, so shape drift can slip through without a compiler failure. Building the generated type here would make future nested-field removals/additions much harder to miss.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/workspace-engine/svc/http/server/openapi/release_targets/getters.go` around lines 87 - 146, Replace the ad-hoc gin.H maps in the loop that builds items (where ReleaseTargetResult is constructed from rows, using currentVersionMap, latestJobMap and verificationsMap) with the generated ReleaseTargetItem/related types from the OpenAPI models: instantiate ReleaseTargetItem (and nested types for Environment, Resource, DesiredVersion, CurrentVersion, LatestJob, Verifications) and assign strongly-typed fields instead of gin.H; populate fields using the existing row.*, cv.* and lj.* values (use json.RawMessage for JobMetadata and proper zero/nil handling when DesiredVersionID == nilUUID or verificationsMap has no entry), keep the same key construction and map lookups but return the typed ReleaseTargetResult items so the response shape matches the generated contract.apps/workspace-engine/oapi/spec/schemas/release_targets.jsonnet (1)
17-55: Reuse the existing summary components instead of inlining copies.These shapes already exist as
EnvironmentSummary,ResourceSummary, andVersionSummary. Referencing them here would avoid the anonymous structs now emitted inapps/workspace-engine/pkg/oapi/oapi.gen.goand reduce the chance that one copy changes without the others.♻️ Suggested cleanup
- environment: { - type: 'object', - required: ['id', 'name'], - properties: { - id: { type: 'string' }, - name: { type: 'string' }, - }, - }, + environment: openapi.schemaRef('EnvironmentSummary'), - resource: { - type: 'object', - required: ['id', 'name', 'version', 'kind', 'identifier'], - properties: { - id: { type: 'string' }, - name: { type: 'string' }, - version: { type: 'string' }, - kind: { type: 'string' }, - identifier: { type: 'string' }, - }, - }, + resource: openapi.schemaRef('ResourceSummary'), - desiredVersion: { - nullable: true, - type: 'object', - required: ['id', 'name', 'tag'], - properties: { - id: { type: 'string' }, - name: { type: 'string' }, - tag: { type: 'string' }, - }, - }, + desiredVersion: openapi.schemaRef('VersionSummary', nullable=true), - currentVersion: { - nullable: true, - type: 'object', - required: ['id', 'name', 'tag'], - properties: { - id: { type: 'string' }, - name: { type: 'string' }, - tag: { type: 'string' }, - }, - }, + currentVersion: openapi.schemaRef('VersionSummary', nullable=true),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/workspace-engine/oapi/spec/schemas/release_targets.jsonnet` around lines 17 - 55, Replace the inline anonymous object schemas for environment, resource, desiredVersion, and currentVersion with references to the existing summary components: use EnvironmentSummary for environment, ResourceSummary for resource, and VersionSummary for desiredVersion and currentVersion (i.e., swap the explicit type/required/properties blocks under the keys environment, resource, desiredVersion, currentVersion to $ref entries that point to the existing components). This will prevent duplicate emitted structs and keep generated types consistent across EnvironmentSummary, ResourceSummary, and VersionSummary.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/workspace-engine/pkg/db/queries/release_targets.sql`:
- Around line 81-102: The SQL query ListVerificationMetricsByJobIDs removed the
jvm.interval_seconds column but consumers still expect
VerificationMetricStatus.intervalSeconds; restore the column in the SELECT
(e.g., select jvm.interval_seconds AS metric_interval_seconds or the original
name used by serialization) so the query result matches the openapi/schema
types, or if you intend to remove it, update the VerificationMetricStatus schema
in the OpenAPI (apps/workspace-engine/oapi/openapi.json) and regenerate the SDK
(packages/workspace-engine-sdk) to keep API contract and clients in sync; locate
the query named ListVerificationMetricsByJobIDs and adjust the SELECT or the
schema accordingly.
---
Nitpick comments:
In `@apps/workspace-engine/oapi/spec/schemas/release_targets.jsonnet`:
- Around line 17-55: Replace the inline anonymous object schemas for
environment, resource, desiredVersion, and currentVersion with references to the
existing summary components: use EnvironmentSummary for environment,
ResourceSummary for resource, and VersionSummary for desiredVersion and
currentVersion (i.e., swap the explicit type/required/properties blocks under
the keys environment, resource, desiredVersion, currentVersion to $ref entries
that point to the existing components). This will prevent duplicate emitted
structs and keep generated types consistent across EnvironmentSummary,
ResourceSummary, and VersionSummary.
In `@apps/workspace-engine/svc/http/server/openapi/release_targets/getters.go`:
- Around line 87-146: Replace the ad-hoc gin.H maps in the loop that builds
items (where ReleaseTargetResult is constructed from rows, using
currentVersionMap, latestJobMap and verificationsMap) with the generated
ReleaseTargetItem/related types from the OpenAPI models: instantiate
ReleaseTargetItem (and nested types for Environment, Resource, DesiredVersion,
CurrentVersion, LatestJob, Verifications) and assign strongly-typed fields
instead of gin.H; populate fields using the existing row.*, cv.* and lj.* values
(use json.RawMessage for JobMetadata and proper zero/nil handling when
DesiredVersionID == nilUUID or verificationsMap has no entry), keep the same key
construction and map lookups but return the typed ReleaseTargetResult items so
the response shape matches the generated contract.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 33c6ea3b-6b1a-4bf5-b7da-dd2fa5cf870f
📒 Files selected for processing (7)
apps/workspace-engine/oapi/openapi.jsonapps/workspace-engine/oapi/spec/schemas/release_targets.jsonnetapps/workspace-engine/pkg/db/queries/release_targets.sqlapps/workspace-engine/pkg/db/release_targets.sql.goapps/workspace-engine/pkg/oapi/oapi.gen.goapps/workspace-engine/svc/http/server/openapi/release_targets/getters.gopackages/workspace-engine-sdk/src/schema.ts
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
1 participant
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 by CodeRabbit
Release Notes