Skip to content

fix: actually grab metric measurements in UI verifications query #1022

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
adityachoudhari26 merged 2 commits into from
Apr 21, 2026
Merged

fix: actually grab metric measurements in UI verifications query #1022

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
eda7dad
fix: actually grab metric measurements in UI verifications query
adityachoudhari26 Apr 21, 2026
9a18598
cleanup
adityachoudhari26 Apr 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 101 additions & 14 deletions packages/trpc/src/routes/verifications.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,64 @@
import { TRPCError } from "@trpc/server";
import { z } from "zod";

import { desc, eq } from "@ctrlplane/db";
import { asc, desc, eq, inArray } from "@ctrlplane/db";
import * as schema from "@ctrlplane/db/schema";
import { getClientFor } from "@ctrlplane/workspace-engine-sdk";

import { protectedProcedure, router } from "../trpc.js";

type MeasurementStatus = "failed" | "inconclusive" | "passed";
type JobVerificationStatus = "passed" | "failed" | "running" | "";

function getFailedCount(measurements: MeasurementStatus[]): number {
return measurements.filter((s) => s === "failed").length;
}

function getConsecutiveSuccessCount(measurements: MeasurementStatus[]): number {
let i = measurements.length;
while (i > 0 && measurements[i - 1] === "passed") i--;
return measurements.length - i;
}

type MeasurementRow = { metricId: string | null; status: MeasurementStatus };

function groupMeasurementsByMetric(
metricIds: string[],
measurements: MeasurementRow[],
): Map<string, MeasurementStatus[]> {
return new Map(
metricIds.map((id) => [
id,
measurements.filter((r) => r.metricId === id).map((r) => r.status),
]),
);
}

function computeMetricPhase(
measurements: MeasurementStatus[],
count: number,
successThreshold: number | null,
failureThreshold: number | null,
): "passed" | "failed" | "running" {
const failureLimit = failureThreshold ?? 0;
const failedCount = getFailedCount(measurements);
const consecutiveSuccessCount = getConsecutiveSuccessCount(measurements);

const hasAnyFailures = failedCount > 0;
const isFailureLimitExceeded = failureLimit > 0 && failedCount > failureLimit;
if ((failureLimit === 0 && hasAnyFailures) || isFailureLimitExceeded)
return "failed";

if (
successThreshold != null &&
successThreshold > 0 &&
consecutiveSuccessCount >= successThreshold
)
return "passed";

if (measurements.length >= count) return "passed";

return "running";
}
Comment on lines +35 to +60
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 21, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

❓ Verification inconclusive

🏁 Script executed:

# First, explore the verifications.ts file structure and context
cd packages/trpc/src/routes && wc -l verifications.ts

Repository: ctrlplanedev/ctrlplane

Length of output: 86

🏁 Script executed:

# Check if there are test files for this module
find . -type f -name "*verification*" | head -20

Repository: ctrlplanedev/ctrlplane

Length of output: 633

🏁 Script executed:

# Search for the full context of computeMetricPhase and related functions
cd packages/trpc/src/routes && sed -n '1,100p' verifications.ts

Repository: ctrlplanedev/ctrlplane

Length of output: 3359

🏁 Script executed:

# Look for tests that might clarify the expected behavior
find . -type f \( -name "*.test.ts" -o -name "*.spec.ts" \) | xargs grep -l "computeMetricPhase\|Measurement\|threshold" 2>/dev/null | head -10

Repository: ctrlplanedev/ctrlplane

Length of output: 48

🏁 Script executed:

# Search for any documentation or comments about thresholds
rg "failureThreshold|successThreshold" --type ts -B2 -A2

Repository: ctrlplanedev/ctrlplane

Length of output: 17061

🏁 Script executed:

# Check if there's documentation about the verification logic
find . -type f \( -name "*.md" -o -name "README*" \) | xargs grep -l "verification\|threshold" 2>/dev/null

Repository: ctrlplanedev/ctrlplane

Length of output: 189

🏁 Script executed:

# Look for test files specifically for the verifications route
find . -path "*/node_modules" -prune -o -type f \( -name "*verifications*test*" -o -name "*verifications*spec*" \) -print

Repository: ctrlplanedev/ctrlplane

Length of output: 48

🏁 Script executed:

# Search for test files that might contain tests for computeMetricPhase or similar logic
rg "computeMetricPhase\|getFailedCount\|getConsecutiveSuccessCount" --type ts

Repository: ctrlplanedev/ctrlplane

Length of output: 48

🏁 Script executed:

# Look at the full status procedure to understand the flow better
cd packages/trpc/src/routes && sed -n '70,136p' verifications.ts

Repository: ctrlplanedev/ctrlplane

Length of output: 2374

🏁 Script executed:

# Check if there are any comments or documentation about verification requirements
rg "verification.*metric|threshold.*behavior" --type ts -i -B2 -A2

Repository: ctrlplanedev/ctrlplane

Length of output: 33046

🏁 Script executed:

# Look at the database schema to understand defaults and constraints
cd packages/db/src/schema && cat -n job-verification-metric.ts

Repository: ctrlplanedev/ctrlplane

Length of output: 3625

🏁 Script executed:

# Check the deployment verification documentation
find . -name "deployment-verification.mdx" -exec cat {} \;

Repository: ctrlplanedev/ctrlplane

Length of output: 6522

🏁 Script executed:

# Search for examples or documentation about threshold semantics
rg "successThreshold|failureThreshold" --type mdx -B3 -A3

Repository: ctrlplanedev/ctrlplane

Length of output: 93

🏁 Script executed:

# Look for any Go implementation in the workspace-engine (backend) that might clarify semantics
find apps/workspace-engine -name "*.go" -type f | xargs grep -l "failureThreshold\|successThreshold" 2>/dev/null | head -5

Repository: ctrlplanedev/ctrlplane

Length of output: 369

🏁 Script executed:

# Check if there are any comments or logic in the Go code
find apps/workspace-engine -name "*.go" -type f | xargs grep -B5 -A10 "failureThreshold\|successThreshold" 2>/dev/null | head -80

Repository: ctrlplanedev/ctrlplane

Length of output: 6959

🏁 Script executed:

# Get the full test file to understand the threshold semantics
cat apps/workspace-engine/pkg/oapi/job_verification_test.go

Repository: ctrlplanedev/ctrlplane

Length of output: 3045

🏁 Script executed:

# Also look at the Status() method implementation to understand the logic
rg "func.*Status\(\)" apps/workspace-engine/pkg/oapi/ -A15

Repository: ctrlplanedev/ctrlplane

Length of output: 1175

🏁 Script executed:

# Get the full Status() method implementation
sed -n '/func (jv \*JobVerification) Status()/,/^func /p' apps/workspace-engine/pkg/oapi/oapi.go | head -80

Repository: ctrlplanedev/ctrlplane

Length of output: 1391

Verify threshold semantics against your verification spec and decision on count-cap fallback.

The code has two semantic behaviors worth confirming:

  1. Asymmetric thresholds are intentional. Failure uses strict > while success uses >=: with failureThreshold = 3 the metric fails on the 4th failure, but with successThreshold = 3 it passes on the 3rd success. This is confirmed by the backend test suite (FailureLimitReached_NotFailedBeforeExceeded shows failureThreshold=2 with 2 failures does NOT fail). Decide if this asymmetry matches your intent or should be unified.

  2. Count-cap fallback behavior. When successThreshold is not set (null or 0—the database defaults), reaching count measurements without exceeding the failure limit declares success, even if no actual successes occurred (e.g., all inconclusive). This is the current design, but consider whether inconclusive runs should be treated as "running" or "inconclusive" instead of "passed".

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@packages/trpc/src/routes/verifications.ts` around lines 35 - 60,
computeMetricPhase currently treats failureThreshold with strict '>' (fails
after exceeding) and successThreshold with '>=' (passes when reaching), and it
treats reaching count as automatic pass when successThreshold is null/0; confirm
intended semantics and update accordingly: either unify to symmetric comparisons
(use '>=' for failures to fail when failedCount >= failureThreshold, or use '>'
for both to require exceeding) inside computeMetricPhase (referencing
failureThreshold, failedCount, successThreshold, consecutiveSuccessCount) and
change the count-cap fallback behavior (the final if using measurements.length
>= count) to instead treat inconclusive/no-success runs as "running" or a new
"inconclusive" state per spec; ensure logic still consults getFailedCount and
getConsecutiveSuccessCount and add/tests to reflect the chosen behavior.


export const verificationsRouter = router({
measurements: protectedProcedure
.input(z.uuid())
Expand All @@ -31,19 +83,54 @@ export const verificationsRouter = router({

status: protectedProcedure
.input(z.object({ jobId: z.string().uuid() }))
.query(async ({ input }) => {
const result = await getClientFor().GET(
"/v1/jobs/{jobId}/verification-status",
{ params: { path: { jobId: input.jobId } } },
.query(async ({ input, ctx }) => {
const metrics = await ctx.db
.select({
id: schema.jobVerificationMetricStatus.id,
count: schema.jobVerificationMetricStatus.count,
successThreshold: schema.jobVerificationMetricStatus.successThreshold,
failureThreshold: schema.jobVerificationMetricStatus.failureThreshold,
})
.from(schema.jobVerificationMetricStatus)
.where(eq(schema.jobVerificationMetricStatus.jobId, input.jobId));

if (metrics.length === 0) return { status: "" as JobVerificationStatus };

const measurements = await ctx.db
.select({
metricId:
schema.jobVerificationMetricMeasurement
.jobVerificationMetricStatusId,
status: schema.jobVerificationMetricMeasurement.status,
})
.from(schema.jobVerificationMetricMeasurement)
.where(
inArray(
schema.jobVerificationMetricMeasurement
.jobVerificationMetricStatusId,
metrics.map((m) => m.id),
),
)
.orderBy(asc(schema.jobVerificationMetricMeasurement.measuredAt));

const byMetric = groupMeasurementsByMetric(
metrics.map((m) => m.id),
measurements,
);

if (result.error != null) {
throw new TRPCError({
code: "INTERNAL_SERVER_ERROR",
message: `Failed to get verification status: ${JSON.stringify(result.error)}`,
});
}
const phases = metrics.map((m) =>
computeMetricPhase(
byMetric.get(m.id) ?? [],
m.count,
m.successThreshold,
m.failureThreshold,
),
);

return result.data;
if (phases.some((p) => p === "failed"))
return { status: "failed" as JobVerificationStatus };
if (phases.some((p) => p === "running"))
return { status: "running" as JobVerificationStatus };
return { status: "passed" as JobVerificationStatus };
}),
});
Loading