PR Review Summary

Status: Request Changes

Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The fair-picker direction is correct and the backup priority-0 case is covered. Two contract gaps remain: the selector reorders non-heavy candidates despite #13586's unchanged-non-heavy boundary, and the PR still auto-closes #13586 while its bounded-summary-hold AC has been split to #13592.

Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #13586, #13592, PR #13593 head 3f466631ad9a39043a0ccc96ec494230730a7634, PR body, changed scheduling files, registry order, and current CI.
• Expected Solution Shape: Backup must be priority-0; staleness fairness should reorder the heavy/REM lease-competing set without changing non-heavy/continuous scheduling unless the issue explicitly updates that contract. If Part 2 moves to #13592, #13593 must not close a ticket whose AC still includes Part 2.
• Patch Verdict: The picker improves the heavy-task starvation case, but its current global staleness comparison changes non-heavy ordering and lacks that regression test.

Context & Graph Linking

• Target Epic / Issue ID: #13586
• Related Graph Nodes: #13592 bounded summary lease-hold, #12065 REM orchestrator SSOT, heavy-maintenance lease

Depth Floor

Challenge: I actively falsified the "non-heavy / continuous behavior unchanged" boundary rather than only rerunning the added tests. The new selector can choose an overdue heavy task over a registry-earlier non-heavy task with neutral staleness metadata.

Close-Target Challenge: #13592 exists and is a reasonable split, but #13586 still contains the bounded summary lease-hold AC. Resolves #13586 would close a ticket whose stated AC is intentionally not delivered here.

Evidence Audit

Findings: one behavioral contract mismatch.

Manual falsifier at 3f466631a:

node -e 'import {pickNextCandidate} from "./ai/daemons/orchestrator/scheduling/picker.mjs"; const cand=(taskName, maintenanceClass)=>({taskName, trigger:{reason:taskName}, descriptor:{taskName, maintenanceClass, dependencies:[], executionKind:"x"}}); const winner=pickNextCandidate({candidates:[cand("tenant-repo-sync","continuous"), cand("dream","heavy")], runningTasks:[], policyContext:{now:1000000, priorityZeroTasks:["backup"], taskMeta:{dream:{lastRunAt:0,cadenceMs:600000}}}}); console.log(winner.taskName);'

returns:

dream

But tenant-repo-sync is registry-earlier than dream, is non-heavy/continuous, and #13586 says non-heavy behavior is unchanged / the new selector only re-orders heavy candidates.

Close-Target Audit

• Close-targets identified: Resolves #13586
• #13586 is open and not epic-labeled.
• #13592 is open and carries the split bounded-summary-hold work.

Findings: fail until reconciled. Either #13593 must stop auto-closing #13586, or #13586 must be explicitly amended/comment-reconciled so the bounded-hold AC is no longer part of its close contract and #13592 is the authoritative successor for that part.

Test-Execution & Location Audit

• Branch checked out locally under /Users/Shared/codex/neomjs/neo/tmp/review-13593.
• Verified head: 3f466631ad9a39043a0ccc96ec494230730a7634.
• npm run test-unit -- test/playwright/unit/ai/daemons/orchestrator/scheduling/picker.spec.mjs test/playwright/unit/ai/daemons/orchestrator/scheduling/pipeline.spec.mjs --workers=1 -> 29 passed.
• git diff --check origin/dev...HEAD -> OK.
• node --check for picker.mjs, pipeline.mjs, picker.spec.mjs, pipeline.spec.mjs -> OK.
• Full scheduling-directory run was not usable in this fresh worktree because unrelated SessionSummaryBacklogCount.spec.mjs failed to import missing local ai/mcp/server/memory-core/config.mjs; focused changed specs pass.

Contract Completeness Audit

Findings: fail. The code and PR body say lightweight / health / continuous tasks are excluded so they keep registry order, but the selector compares eligible positive staleness scores against neutral 0 scores globally. That changes ordering between a non-heavy candidate and an overdue heavy candidate.

Required Actions

To proceed with merging, please address the following:

• Constrain the staleness selector so non-heavy/continuous candidates keep the documented registry-order behavior, or explicitly update the issue/PR contract and add coverage for the intended cross-class ordering. At minimum add a regression test for a registry-earlier non-heavy candidate versus an overdue heavy candidate.
• Reconcile the close target: do not auto-close #13586 while its bounded summary lease-hold AC is still open in #13592, unless #13586 is explicitly updated/comment-reconciled to delegate that AC to #13592 as successor scope.

Evaluation Metrics

• [ARCH_ALIGNMENT]: 78 - Correct high-level fairness lane; current selector crosses the documented class boundary.
• [CONTENT_COMPLETENESS]: 76 - Part 1 is covered, but close-target and non-heavy behavior contracts are incomplete.
• [EXECUTION_QUALITY]: 82 - Focused pure picker and tests; missing one critical cross-class regression.
• [PRODUCTIVITY]: 88 - High-value scheduler hardening once narrowed.
• [IMPACT]: 92 - Backup/backfill/REM starvation is a live stability problem.
• [COMPLEXITY]: 64 - Moderate scheduler policy change with live-cloud blast radius.
• [EFFORT_PROFILE]: Maintenance - Agent OS scheduler fairness.