mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-03 17:51:58 +01:00
fix(evals): bind complete unassigned TestOps plan buckets (#18608)
## Thinking Path > - Paperclip manages agents that do real work. > - Product evals check the saved outcome of that work. > - TestOps plans select the exact automated cases to run. > - A new plan can report its unassigned cases as a job-statistics bucket. > - The pilot registration guard rejects that bucket as a foreign job. > - This change binds only a complete, known unassigned bucket and records assignment diagnostics before writes. ## Linked Issues or Issue Description Refs #18516 and #18542. The metadata-only bootstrap in [41464514278](https://github.com/paperclipai/paperclip/actions/runs/41464514278) created the dedicated plan and verified its GitHub job definition, then stopped at `plan_job_mismatch` before job or case-assignment mutations. No model work ran. The retained receipt did not include the assignment rows. The live failure's exact shape remains unconfirmed. The API's job-statistics carrier permits an absent job. The old guard rejects a complete unassigned bucket. This PR covers that case and records the actual rows in the next closed inspection. ## What Changed - Accept one closed unassigned bucket with the exact selected-case count. Support an explicit null job or an omitted optional job field. - Assign the exact selected cases once. Keep the existing owned assignment without assigning it twice. - Reject foreign jobs, mixed buckets, wrong counts and unknown unassigned fields before mutations. - Retain only bounded assignment kinds, numeric job IDs and counts in the existing observation receipt. - Merge current master, including the full-catalog adapter from #18542. Retain its context-aware plan and job checks alongside the new pre-write assignment inspection. The helper consumes that context’s expected case count. - Document the inspection and binding rules. The helper takes the expected count from its owner. This does not import or run the full-catalog route from #18542. ## Verification - Red: both complete unassigned carriers fail on the old guard; the other 70 dispatch/API controls pass. - Green: all 72 dispatch/API controls passed before the master sync. After the sync, all 104 related pilot and full-catalog Node controls pass. They include ordinary registration, no duplicate assignment, foreign-job rejection, incomplete and unknown bucket rejection, launch fences, uncertain-response protection and result readback. - Diff check passes. No TypeScript or compiled product code changes. Full repository checks were not run locally. All required CI checks pass on current head `a8bec6418065a7faae28c8daab3efdfc9b184797` ([run 41473510972](https://github.com/paperclipai/paperclip/actions/runs/41473510972)). Both current-head reviews are 5/5 with no unresolved threads. Before the master sync, one unchanged server timing test failed on attempt one and passed on the only targeted rerun; that original failure remains in CI attempt one. No rerun was needed on the current head. - Live acceptance still requires another metadata-only inspection, followed by a genuine three-case TestOps launch. This PR does not claim a new passing measurement. ## Risks An overly broad unassigned rule could replace an existing job. The new rule permits only one known bucket with the exact selected-case count and no foreign job. All final binding checks still apply. Unknown assignment shapes stop before writes. Original case queries, model, clocks, graders, evidence rules and retries are unchanged. ## Model Used OpenAI GPT-6 through Codex assisted with repository inspection, code execution and tool use. The host does not expose the exact model revision or context-window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either linked existing issues or described the issue in-PR following the relevant issue template - [x] I have not referenced internal or instance-local Paperclip issues or links - [x] My branch name describes the change and contains no internal Paperclip ticket id - [x] I have run the relevant local controls and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge
This commit is contained in:
1 parent
71a20453e1
commit
9203ab52f0
3 files changed
+35
-3
No files matched your search
@@ -225,6 +225,9 @@ Initial setup uses the existing workflow operations:
|
||||
binding. Only the observed single `Branch/defaultValue` definition for
|
||||
`refs/heads/master` is accepted. Bootstrap enables the job and assigns cases
|
||||
while preserving that definition; the plan uses its default branch.
|
||||
Assignment inspection records unassigned and existing-job counts before
|
||||
binding. A single unassigned bucket must cover the exact selected cases;
|
||||
foreign jobs and unknown or incomplete buckets stop before writes.
|
||||
2. Read the closed registration receipt. Store its observed `binding` JSON as
|
||||
`CHAT_COMPLETION_TESTOPS_BINDING` in `runner-e2e-testops-reporting`.
|
||||
3. Use TestOps **Run**. For the initial operator launch, set an exact next
|
||||
|
||||
@@ -72,6 +72,11 @@ function assertBranchDefinition(job) {
|
||||
|| job.parameters[0].name !== 'Branch' || job.parameters[0].defaultValue !== 'refs/heads/master'
|
||||
|| Object.hasOwn(job.parameters[0], 'value')) reject('job_parameters');
|
||||
}
|
||||
function isUnassignedBucket(row, expectedCount) {
|
||||
return row !== null && typeof row === 'object' && !Array.isArray(row)
|
||||
&& Object.keys(row).every(key => ['job', 'testCasesCount'].includes(key))
|
||||
&& (row.job === null || !Object.hasOwn(row, 'job')) && row.testCasesCount === expectedCount;
|
||||
}
|
||||
|
||||
export async function validatePilotSelection(api, env, context, workflowId) {
|
||||
const { cases, targetSha, planName } = catalogDefinition(context);
|
||||
@@ -168,6 +173,7 @@ export async function completePilotRegistration(api, prepared, official, context
|
||||
const binding = { ...prepared, jobId: run.job.id };
|
||||
assertPlan(await api(`/api/testplan/${binding.planId}`), binding, context);
|
||||
const job = await api(`/api/job/${binding.jobId}`); assertJob(job, binding, workflowId, context);
|
||||
const assignments = await api(`/api/testplan/${binding.planId}/job`);
|
||||
await observe({ schema: 'paperclip.chat-completion-testops-registration-observed/v1', ...context,
|
||||
projectId: 1, jobId: job.id, launchId: official.launchId, allureJobRunId: official.jobRunId,
|
||||
parameterCount: Array.isArray(job.parameters) ? job.parameters.length : null,
|
||||
@@ -175,17 +181,22 @@ export async function completePilotRegistration(api, prepared, official, context
|
||||
id: positive(row?.id) ? row.id : null, name: row?.name === 'Branch' ? 'Branch' : 'UNRESOLVED',
|
||||
defaultValue: row?.defaultValue === 'refs/heads/master' ? 'refs/heads/master' : 'NOT_PROJECTED',
|
||||
runtimeValuePresent: Object.hasOwn(row ?? {}, 'value') })) : null,
|
||||
assignmentCount: Array.isArray(assignments) ? assignments.length : null,
|
||||
assignments: Array.isArray(assignments) ? assignments.slice(0, 4).map(row => ({
|
||||
kind: isUnassignedBucket(row, cases.length) ? 'unassigned' : row?.job?.id === binding.jobId ? 'owned' : 'foreign_or_unrecognized',
|
||||
jobId: positive(row?.job?.id) ? row.job.id : null,
|
||||
testCasesCount: Number.isSafeInteger(row?.testCasesCount) && row.testCasesCount >= 0 ? row.testCasesCount : null })) : null,
|
||||
qualification: 'inspection_only', actualResult: null });
|
||||
assertBranchDefinition(job);
|
||||
const assignments = await api(`/api/testplan/${binding.planId}/job`);
|
||||
if (!Array.isArray(assignments) || assignments.length > 1 || assignments.some(row => row.job?.id !== binding.jobId || row.testCasesCount !== cases.length)) reject('plan_job_mismatch');
|
||||
if (!Array.isArray(assignments) || assignments.length > 1 || assignments.some(row => !isUnassignedBucket(row, cases.length)
|
||||
&& (row?.job?.id !== binding.jobId || row?.testCasesCount !== cases.length))) reject('plan_job_mismatch');
|
||||
await markOnce('bind_owned_job');
|
||||
// PATCH only the documented enable/integration fields. Preserve the opaque
|
||||
// definition exactly; no guessed parameter-definition write codec is used.
|
||||
await api(`/api/job/${binding.jobId}`, 'PATCH', { integrationId: binding.integrationId, canRun: true });
|
||||
const patched = await api(`/api/job/${binding.jobId}`); assertJob(patched, binding, workflowId, context); assertBranchDefinition(patched);
|
||||
if (!same(patched.parameters, job.parameters) || (Object.hasOwn(patched, 'canRun') && patched.canRun !== true)) reject('job_parameters');
|
||||
if (!assignments.length) {
|
||||
if (!assignments.length || isUnassignedBucket(assignments[0], cases.length)) {
|
||||
await markOnce('assign_plan');
|
||||
await api(`/api/testplan/${binding.planId}/assign`, 'POST', { jobId: binding.jobId,
|
||||
selection: { inverted: false, manual: false, leafsInclude: binding.cases.map(row => row.caseId) } });
|
||||
|
||||
@@ -92,6 +92,24 @@ test('registration keeps an existing exact assignment without assigning it twice
|
||||
assert.equal(f.calls.filter(row => row.route.endsWith('/assign')).length, 0);
|
||||
});
|
||||
|
||||
for (const bucket of [{ job: null, testCasesCount: 3 }, { testCasesCount: 3 }]) {
|
||||
test(`registration binds an explicit unassigned bucket (${Object.hasOwn(bucket, 'job') ? 'null' : 'omitted'} job)`, async () => {
|
||||
const f = mutationApi({ assignments: [bucket] }), markers = []; let observed;
|
||||
await completePilotRegistration(f.api, prepared, official, context, 999, async stage => markers.push(stage), async value => { observed = value; });
|
||||
assert.deepEqual(markers, ['bind_owned_job', 'assign_plan']);
|
||||
assert.equal(f.calls.filter(row => row.route.endsWith('/assign')).length, 1);
|
||||
assert.deepEqual(observed.assignments, [{ kind: 'unassigned', jobId: null, testCasesCount: 3 }]);
|
||||
});
|
||||
}
|
||||
|
||||
for (const bucket of [{ job: null, testCasesCount: 2 }, { job: null, testCasesCount: 3, otherJobId: 99 }]) {
|
||||
test('registration rejects incomplete or unknown unassigned bucket data without writes', async () => {
|
||||
const f = mutationApi({ assignments: [bucket] });
|
||||
await assert.rejects(completePilotRegistration(f.api, prepared, official, context, 999, async () => assert.fail('no mutation marker')));
|
||||
assert.equal(f.calls.filter(row => row.method !== 'GET').length, 0);
|
||||
});
|
||||
}
|
||||
|
||||
test('bootstrap retains a closed observation before rejecting an unknown parameter shape', async () => {
|
||||
const f = mutationApi({ parameters: [{ id: 34, name: 'PRIVATE_NAME', defaultValue: 'PRIVATE_DEFAULT', value: 'PRIVATE_VALUE' }] });
|
||||
let observed;
|
||||
|
||||
Reference in new issue
Block a user