Refactor pipeline task handling and UI flows
This commit is contained in:
@@ -139,16 +139,22 @@ test("structured output schemas avoid optional top-level fields for OpenAI stric
|
||||
() =>
|
||||
qualityReviewSchema.parse({
|
||||
isValid: true,
|
||||
severity: "ok",
|
||||
issues: [],
|
||||
suggestions: [],
|
||||
rewriteRequired: false,
|
||||
revisedCopy: null,
|
||||
}),
|
||||
/notes|invalid|required/i,
|
||||
);
|
||||
assert.equal(
|
||||
qualityReviewSchema.parse({
|
||||
isValid: true,
|
||||
severity: "ok",
|
||||
issues: [],
|
||||
suggestions: [],
|
||||
rewriteRequired: false,
|
||||
revisedCopy: null,
|
||||
notes: null,
|
||||
}).notes,
|
||||
null,
|
||||
@@ -338,8 +344,11 @@ test("outreach schemas parse German customer-facing payloads", () => {
|
||||
});
|
||||
const qualityParsed = qualityReviewSchema.parse({
|
||||
isValid: true,
|
||||
severity: "ok",
|
||||
issues: [],
|
||||
suggestions: ["Mehr Kundennutzen konkret beschreiben."],
|
||||
rewriteRequired: false,
|
||||
revisedCopy: null,
|
||||
notes: null,
|
||||
});
|
||||
|
||||
@@ -350,6 +359,46 @@ test("outreach schemas parse German customer-facing payloads", () => {
|
||||
assert.equal(Array.isArray(qualityParsed.suggestions), true);
|
||||
});
|
||||
|
||||
test("quality review schema accepts one-shot revised copy payloads", () => {
|
||||
const parsed: QualityReview = qualityReviewSchema.parse({
|
||||
isValid: false,
|
||||
severity: "warning",
|
||||
issues: ["Betreff klingt noch etwas generisch."],
|
||||
suggestions: ["Betreff konkreter machen."],
|
||||
rewriteRequired: true,
|
||||
revisedCopy: {
|
||||
publicSummary: "Mir ist aufgefallen, dass die mobile Seite etwas traege wirkt.",
|
||||
publicBody:
|
||||
"Mein Vorschlag waere, zuerst die sichtbaren Ladebremsen der Startseite zu pruefen.",
|
||||
emailSubject: "Kurzer Hinweis zur mobilen Seite",
|
||||
emailBody:
|
||||
"Guten Tag, mir ist beim Blick auf Ihre Website aufgefallen, dass die mobile Seite etwas traege wirkt.",
|
||||
phoneScript: {
|
||||
openingLine: "Guten Tag, hier ist Matthias Meister.",
|
||||
callScript: [
|
||||
"Mir ist bei Ihrer mobilen Website ein konkreter Ladezeitpunkt aufgefallen.",
|
||||
"Mein Vorschlag waere, diesen Punkt kurz zu priorisieren.",
|
||||
],
|
||||
closeLine: "Soll ich Ihnen den Hinweis kurz per E-Mail senden?",
|
||||
},
|
||||
followUpDraft: {
|
||||
message:
|
||||
"Ich wollte kurz nachfassen, ob der Hinweis zur mobilen Seite fuer Sie relevant ist.",
|
||||
followInDays: 7,
|
||||
goals: ["kurze Rueckmeldung", "Interesse klaeren"],
|
||||
},
|
||||
},
|
||||
notes: ["Ein Rewrite ist sinnvoll."],
|
||||
});
|
||||
|
||||
assert.equal(parsed.rewriteRequired, true);
|
||||
assert.equal(parsed.revisedCopy?.emailSubject, "Kurzer Hinweis zur mobilen Seite");
|
||||
assert.deepEqual(parsed.revisedCopy?.followUpDraft.goals, [
|
||||
"kurze Rueckmeldung",
|
||||
"Interesse klaeren",
|
||||
]);
|
||||
});
|
||||
|
||||
test("schema-inferred types are exported for Convex action wiring", () => {
|
||||
const typedFindings: InternalFindings = {
|
||||
findings: [
|
||||
@@ -393,8 +442,11 @@ test("schema-inferred types are exported for Convex action wiring", () => {
|
||||
|
||||
const typedQuality: QualityReview = {
|
||||
isValid: true,
|
||||
severity: "ok",
|
||||
issues: [],
|
||||
suggestions: [],
|
||||
rewriteRequired: false,
|
||||
revisedCopy: null,
|
||||
notes: null,
|
||||
};
|
||||
|
||||
|
||||
@@ -93,10 +93,10 @@ test("auditGenerationAction exports processAuditGeneration with runId validator"
|
||||
assert.equal(
|
||||
hasPattern(
|
||||
actionSource,
|
||||
/processAuditGeneration\s*=\s*internalAction\(\s*{\s*args:\s*{\s*runId:\s*v\.id\(\s*["']agentRuns["']\s*\)\s*,?\s*}/,
|
||||
/processAuditGeneration\s*=\s*internalAction\(\s*{\s*args:\s*{[\s\S]*?runId:\s*v\.id\(\s*["']agentRuns["']\s*\)[\s\S]*?rootRunId:\s*v\.optional\(v\.id\(\s*["']agentRuns["']\s*\)\)/,
|
||||
),
|
||||
true,
|
||||
"processAuditGeneration should validate runId: v.id(\"agentRuns\")",
|
||||
"processAuditGeneration should validate runId and optional rootRunId as agentRuns IDs",
|
||||
);
|
||||
});
|
||||
|
||||
@@ -280,18 +280,38 @@ test("German copy prompt uses first-contact email tone guidelines without a new
|
||||
);
|
||||
});
|
||||
|
||||
test("quality review blocks when model review or German copy guard fails", () => {
|
||||
test("quality review can rewrite copy once without making copy feedback a hard failure", () => {
|
||||
const qualityPromptSource = extractFunctionSource("buildQualityReviewPrompt");
|
||||
|
||||
assert.match(
|
||||
assert.doesNotMatch(
|
||||
actionSource,
|
||||
/qualityPassed\s*=\s*qualityResult\.object\.isValid\s*&&\s*guardResult\.passed/,
|
||||
"qualityPassed should require both model review validity and German copy guard.",
|
||||
"Copy quality feedback should not be a hard AND-gate with the deterministic German copy guard.",
|
||||
);
|
||||
assert.doesNotMatch(
|
||||
actionSource,
|
||||
/qualityPassed\s*=\s*guardResult\.passed\s*;/,
|
||||
"qualityPassed must not ignore the model quality review.",
|
||||
"The deterministic German copy guard should not be the quality pass condition.",
|
||||
);
|
||||
assert.match(
|
||||
actionSource,
|
||||
/rewriteRequired[\s\S]*revisedCopy[\s\S]*applyRevisedCopy/,
|
||||
"Quality review should be able to request one revised copy and apply it before persistence.",
|
||||
);
|
||||
assert.match(
|
||||
actionSource,
|
||||
/copyReviewAttempts\s*<\s*2/,
|
||||
"Quality review should run at most the initial review plus one rewrite review.",
|
||||
);
|
||||
assert.match(
|
||||
actionSource,
|
||||
/message:\s*["']Copy-Review hat korrigiert\.["']/,
|
||||
"A successful rewrite should be visible as a warning event.",
|
||||
);
|
||||
assert.match(
|
||||
actionSource,
|
||||
/message:\s*["']Copy-Review mit Hinweisen abgeschlossen\.["']/,
|
||||
"Remaining copy feedback should be stored as warning telemetry.",
|
||||
);
|
||||
assert.match(
|
||||
qualityPromptSource,
|
||||
@@ -308,6 +328,11 @@ test("quality review blocks when model review or German copy guard fails", () =>
|
||||
/verified findings|verifizierte Befunde/i,
|
||||
"Quality review should keep concrete claims tied to verified findings.",
|
||||
);
|
||||
assert.match(
|
||||
qualityPromptSource,
|
||||
/revisedCopy|rewriteRequired/,
|
||||
"Quality review prompt should ask for revised copy when rewrite is needed.",
|
||||
);
|
||||
});
|
||||
|
||||
test("action handles post-start failure paths in action-level catch", () => {
|
||||
@@ -536,27 +561,21 @@ test("action handles missing screenshots with warning event fallback", () => {
|
||||
);
|
||||
});
|
||||
|
||||
test("action runs german copy guard and blocks outreach-ready on validation failure", () => {
|
||||
test("action keeps German copy guard as telemetry without blocking outreach-ready", () => {
|
||||
assert.equal(
|
||||
hasPattern(actionSource, /validateCustomerFacingCopy/),
|
||||
true,
|
||||
"Action should run German copy validation",
|
||||
"Action should still run German copy validation for telemetry.",
|
||||
);
|
||||
assert.equal(
|
||||
hasPattern(
|
||||
actionSource,
|
||||
/qualityPassed\s*=\s*qualityResult\.object\.isValid\s*&&\s*guardResult\.passed/,
|
||||
),
|
||||
true,
|
||||
"Model QA and deterministic German copy guard failures should hard-block the audit run.",
|
||||
assert.doesNotMatch(
|
||||
actionSource,
|
||||
/guardResult\.passed[\s\S]{0,500}finishAuditGenerationRun[\s\S]{0,250}status:\s*["']failed["']/,
|
||||
"German copy guard findings should not finish the audit generation as failed.",
|
||||
);
|
||||
assert.equal(
|
||||
hasPattern(
|
||||
actionSource,
|
||||
/qualityPassed\s*=\s*guardResult\.passed\s*;/,
|
||||
),
|
||||
false,
|
||||
"Action must not ignore the model QA validity flag.",
|
||||
assert.match(
|
||||
actionSource,
|
||||
/guardTelemetry|deterministicGuard/,
|
||||
"German copy guard output should be persisted as telemetry in the quality payload.",
|
||||
);
|
||||
assert.equal(
|
||||
hasPattern(actionSource, /internal\.leads\.reviewUpdateInternal/),
|
||||
|
||||
41
tests/audit-progress.test.ts
Normal file
41
tests/audit-progress.test.ts
Normal file
@@ -0,0 +1,41 @@
|
||||
import assert from "node:assert/strict";
|
||||
import test from "node:test";
|
||||
|
||||
import {
|
||||
AUDIT_PROGRESS_TOTAL_STEPS,
|
||||
getAuditProgressForStep,
|
||||
} from "../lib/audits/progress";
|
||||
|
||||
test("audit progress mapping exposes stable customer-facing progress steps", () => {
|
||||
assert.equal(AUDIT_PROGRESS_TOTAL_STEPS, 6);
|
||||
|
||||
assert.deepEqual(getAuditProgressForStep("pagespeed_insights"), {
|
||||
step: 2,
|
||||
total: 6,
|
||||
label: "Messe PageSpeed",
|
||||
percent: 33,
|
||||
});
|
||||
|
||||
assert.deepEqual(getAuditProgressForStep("qualityReview"), {
|
||||
step: 6,
|
||||
total: 6,
|
||||
label: "Speichere Audit",
|
||||
percent: 100,
|
||||
});
|
||||
});
|
||||
|
||||
test("audit progress mapping falls back safely for historical runs", () => {
|
||||
assert.deepEqual(getAuditProgressForStep(undefined), {
|
||||
step: 1,
|
||||
total: 6,
|
||||
label: "Audit vorbereitet",
|
||||
percent: 17,
|
||||
});
|
||||
|
||||
assert.deepEqual(getAuditProgressForStep("some_old_step"), {
|
||||
step: 1,
|
||||
total: 6,
|
||||
label: "Audit vorbereitet",
|
||||
percent: 17,
|
||||
});
|
||||
});
|
||||
92
tests/audit-workflow-source.test.ts
Normal file
92
tests/audit-workflow-source.test.ts
Normal file
@@ -0,0 +1,92 @@
|
||||
import assert from "node:assert/strict";
|
||||
import { existsSync, readFileSync } from "node:fs";
|
||||
import path from "node:path";
|
||||
import test from "node:test";
|
||||
|
||||
const source = (relativePath: string) => {
|
||||
return readFileSync(path.join(process.cwd(), ...relativePath.split("/")), "utf8");
|
||||
};
|
||||
|
||||
const fileExists = (relativePath: string) => {
|
||||
return existsSync(path.join(process.cwd(), ...relativePath.split("/")));
|
||||
};
|
||||
|
||||
test("Convex Workflow and Workpool dependencies and components are registered", () => {
|
||||
const packageSource = source("package.json");
|
||||
const configSource = source("convex/convex.config.ts");
|
||||
|
||||
assert.match(packageSource, /"@convex-dev\/workflow"/);
|
||||
assert.match(packageSource, /"@convex-dev\/workpool"/);
|
||||
assert.match(configSource, /from\s+["@']@convex-dev\/workflow\/convex\.config["@']/);
|
||||
assert.match(configSource, /from\s+["@']@convex-dev\/workpool\/convex\.config["@']/);
|
||||
assert.match(configSource, /app\.use\(workflow/);
|
||||
assert.match(configSource, /app\.use\(auditWorkpool/);
|
||||
});
|
||||
|
||||
test("audit workflow defines durable workflow manager with retrying workpool options", () => {
|
||||
assert.equal(fileExists("convex/auditWorkflow.ts"), true);
|
||||
const workflowSource = source("convex/auditWorkflow.ts");
|
||||
|
||||
assert.match(workflowSource, /WorkflowManager/);
|
||||
assert.match(workflowSource, /components\.workflow/);
|
||||
assert.match(workflowSource, /workpoolOptions/);
|
||||
assert.match(workflowSource, /maxParallelism:\s*3/);
|
||||
assert.match(workflowSource, /retryActionsByDefault:\s*true/);
|
||||
assert.match(workflowSource, /maxAttempts:\s*3/);
|
||||
assert.match(workflowSource, /initialBackoffMs:\s*1000/);
|
||||
assert.match(workflowSource, /base:\s*2/);
|
||||
assert.match(workflowSource, /step\.runAction/);
|
||||
assert.match(workflowSource, /step\.runMutation/);
|
||||
assert.match(workflowSource, /Promise\.all/);
|
||||
});
|
||||
|
||||
test("requestLeadAudit creates a visible agentRun and starts the workflow", () => {
|
||||
const pageSpeedSource = source("convex/pageSpeed.ts");
|
||||
|
||||
assert.match(pageSpeedSource, /internal\.auditWorkflow\.startLeadAuditWorkflow/);
|
||||
assert.match(pageSpeedSource, /type:\s*"audit"/);
|
||||
assert.match(pageSpeedSource, /progressLabel:\s*"Audit vorbereitet"/);
|
||||
assert.match(pageSpeedSource, /workflowId/);
|
||||
});
|
||||
|
||||
test("workflow PageSpeed start accepts root runs already marked running", () => {
|
||||
const pageSpeedSource = source("convex/pageSpeed.ts");
|
||||
|
||||
assert.match(
|
||||
pageSpeedSource,
|
||||
/run\.status\s*!==\s*"pending"[\s\S]*run\.status\s*!==\s*"failed"[\s\S]*run\.status\s*!==\s*"running"/,
|
||||
);
|
||||
});
|
||||
|
||||
test("workflow failure progress stays on the failing step instead of jumping to quality review", () => {
|
||||
const workflowSource = source("convex/auditWorkflow.ts");
|
||||
const catchIndex = workflowSource.indexOf("} catch (error)");
|
||||
assert.notEqual(catchIndex, -1, "Expected workflow catch block.");
|
||||
const nextExportIndex = workflowSource.indexOf("export const startLeadAuditWorkflow", catchIndex);
|
||||
assert.notEqual(nextExportIndex, -1, "Expected workflow catch block end.");
|
||||
const catchSource = workflowSource.slice(catchIndex, nextExportIndex);
|
||||
|
||||
assert.doesNotMatch(catchSource, /progressPatch\(args\.runId,\s*"qualityReview"\)/);
|
||||
assert.doesNotMatch(catchSource, /progressStep|progressTotal|progressLabel|progressPercent/);
|
||||
assert.match(catchSource, /status:\s*"failed"/);
|
||||
});
|
||||
|
||||
test("audit dashboard query includes root audit runs and exposes progress and retry fields", () => {
|
||||
const auditsSource = source("convex/audits.ts");
|
||||
|
||||
assert.match(auditsSource, /\.eq\("type",\s*"audit"\)/);
|
||||
assert.match(auditsSource, /kind:\s*"generation"/);
|
||||
assert.match(auditsSource, /runType/);
|
||||
assert.match(auditsSource, /progress:/);
|
||||
assert.match(auditsSource, /retry:/);
|
||||
assert.match(auditsSource, /canRetry/);
|
||||
});
|
||||
|
||||
test("audit retry mutation restarts final failed or canceled runs through workflow", () => {
|
||||
const auditsSource = source("convex/audits.ts");
|
||||
|
||||
assert.match(auditsSource, /export const retryAuditRun = mutation/);
|
||||
assert.match(auditsSource, /requireOperator\(ctx\)/);
|
||||
assert.match(auditsSource, /status !== "failed"[\s\S]*status !== "canceled"/);
|
||||
assert.match(auditsSource, /internal\.auditWorkflow\.restartAuditWorkflow/);
|
||||
});
|
||||
@@ -32,7 +32,12 @@ test("AuditsBoard keeps audit detail links and non-clickable pipeline cards", as
|
||||
assert.match(source, /row\.kind === "audit"/);
|
||||
assert.match(source, /href=\{row\.detailHref\}/);
|
||||
assert.match(source, /Öffnen/);
|
||||
assert.match(source, /Pipeline läuft/);
|
||||
assert.match(source, /row\.progress/);
|
||||
assert.match(source, /progressbar/);
|
||||
assert.match(source, /row\.retry/);
|
||||
assert.match(source, /Versuch/);
|
||||
assert.match(source, /Erneut starten/);
|
||||
assert.match(source, /retryAuditRun/);
|
||||
assert.match(source, /getGenerationStatusLabel\(row\)/);
|
||||
assert.match(source, /row\.errorSummary/);
|
||||
});
|
||||
|
||||
@@ -122,3 +122,24 @@ test("audits dashboard query suppresses generation rows once a final audit exist
|
||||
"Generation rows should surface run or stage errors.",
|
||||
);
|
||||
});
|
||||
|
||||
test("audits dashboard query hides child generation rows behind root audit runs", async () => {
|
||||
const auditsSource = await source("convex/audits.ts");
|
||||
const querySource = extractExportSource(auditsSource, "listDashboardRows");
|
||||
|
||||
assert.match(
|
||||
querySource,
|
||||
/rootAuditRunLeadIds/,
|
||||
"Query should track lead ids that already have root audit runs.",
|
||||
);
|
||||
assert.match(
|
||||
querySource,
|
||||
/run\.type\s*===\s*"audit_generation"[\s\S]*rootAuditRunLeadIds\.has\(run\.leadId\)/,
|
||||
"Child audit_generation rows should be skipped when a root audit run for the same lead is already visible.",
|
||||
);
|
||||
assert.match(
|
||||
querySource,
|
||||
/continue/,
|
||||
"Child generation rows should be skipped instead of rendered as a duplicate card.",
|
||||
);
|
||||
});
|
||||
|
||||
@@ -138,7 +138,7 @@ test("pageSpeedAction has action-level guard to fail whole run on unexpected err
|
||||
assert.equal(
|
||||
hasPattern(
|
||||
actionSource,
|
||||
/try\s*{[\s\S]*?await ctx\.runMutation\(internal\.pageSpeed\.startPageSpeedAuditRun,\s*{[\s\S]*?}\);\s*[\s\S]*?for\s*\(\s*(?:const|let)\s+strategy\s+of\s+STRATEGIES[\s\S]*?\}\s*catch \(error\)\s*{[\s\S]*classifyPageSpeedFailure\(error,\s*apiKeyRaw\)[\s\S]*?internal\.pageSpeed\.finishPageSpeedAuditRun[\s\S]*status:\s*["']failed["']/,
|
||||
/try\s*{[\s\S]*?Promise\.all\([\s\S]*?STRATEGIES\.map\(async \(strategy\)[\s\S]*?\}\s*catch \(error\)\s*{[\s\S]*classifyPageSpeedFailure\(error,\s*apiKeyRaw\)[\s\S]*?internal\.pageSpeed\.finishPageSpeedAuditRun[\s\S]*status:\s*["']failed["']/,
|
||||
),
|
||||
true,
|
||||
"Action should wrap run lifecycle in an outer try/catch that finalizes the run as failed.",
|
||||
@@ -182,7 +182,7 @@ test("pageSpeedAction enforces raw payload size guard before storage", () => {
|
||||
assert.equal(
|
||||
hasPattern(
|
||||
actionSource,
|
||||
/if\s*\(\s*rawJsonBytes\s*>\s*MAX_RAW_PAGESPEED_BYTES[\s\S]*?}\s*[\s\S]*?continue;[\s\S]*?await ctx\.storage\.store\(/,
|
||||
/if\s*\(\s*rawJsonBytes\s*>\s*MAX_RAW_PAGESPEED_BYTES[\s\S]*?return\s+["']failed["']\s+as\s+const;[\s\S]*?}\s*[\s\S]*?await ctx\.storage\.store\(/,
|
||||
),
|
||||
true,
|
||||
"Raw payload storage must be skipped for oversized payloads.",
|
||||
@@ -202,10 +202,10 @@ test("pageSpeedAction runs both strategies and catches per-strategy errors", ()
|
||||
assert.equal(
|
||||
hasPattern(
|
||||
actionSource,
|
||||
/for\s*\(\s*(?:const|let)\s+strategy\s+of[\s\S]*?\)\s*{[\s\S]*?try[\s\S]*?catch\s*\([^)]*\)[\s\S]*?}/,
|
||||
/Promise\.all\([\s\S]*?STRATEGIES\.map\(async \(strategy\)[\s\S]*?try[\s\S]*?catch\s*\([^)]*\)[\s\S]*?return\s+["']failed["']\s+as\s+const/,
|
||||
),
|
||||
true,
|
||||
"Action should catch errors inside per-strategy loop",
|
||||
"Action should catch errors inside each parallel strategy task",
|
||||
);
|
||||
});
|
||||
|
||||
|
||||
@@ -140,7 +140,7 @@ test("getLeadAuditStartStates exposes active audit run status for lead review bu
|
||||
assert.match(source, /canStart/);
|
||||
});
|
||||
|
||||
test("queueLeadPageSpeedAudit dedupes per lead and schedules pagespeed action", () => {
|
||||
test("queueLeadPageSpeedAudit dedupes per lead and schedules audit workflow", () => {
|
||||
const queueSource = pageSpeedSource;
|
||||
assert.equal(
|
||||
hasPattern(
|
||||
@@ -169,10 +169,10 @@ test("queueLeadPageSpeedAudit dedupes per lead and schedules pagespeed action",
|
||||
assert.equal(
|
||||
hasPattern(
|
||||
queueSource,
|
||||
/ctx\.scheduler\.runAfter\(\s*0,\s*internal\.pageSpeedAction\.processPageSpeedAudit,\s*\{[\s\S]*?runId/,
|
||||
/ctx\.scheduler\.runAfter\(\s*0,\s*internal\.auditWorkflow\.startLeadAuditWorkflow,\s*\{[\s\S]*?runId/,
|
||||
),
|
||||
true,
|
||||
"queueLeadPageSpeedAudit must schedule internal.pageSpeedAction.processPageSpeedAudit with runAfter(0, ...).",
|
||||
"queueLeadPageSpeedAudit must schedule internal.auditWorkflow.startLeadAuditWorkflow with runAfter(0, ...).",
|
||||
);
|
||||
assert.equal(
|
||||
hasPattern(
|
||||
|
||||
Reference in New Issue
Block a user