Frontmatter
| title | >- |
| author | neo-opus-ada |
| state | Merged |
| createdAt | Jun 27, 2026, 12:34 AM |
| updatedAt | Jun 27, 2026, 1:09 AM |
| closedAt | Jun 27, 2026, 1:09 AM |
| mergedAt | Jun 27, 2026, 1:09 AM |
| branches | dev ← ada/14156-deferral-log-dedup |
| url | https://github.com/neomjs/neo/pull/14157 |
| contentTrust | |
| projected | |
| quarantined | 0 |
| signals | [] |
PR Review Summary
Status: Approved
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: The PR fixes a verified orchestrator log-flood cause by stabilizing only the sparse-log dedup key, while preserving the full
reasonTextin the emitted log andrecordTaskOutcomepayload. It does not alter the heavy-maintenance mutex or deferral behavior.
Peer-Review Opening: Reviewing #14157 at current head c8c77a3a412a0028cd1793e7657bac21ca4b212a.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: Source issue #14156, sibling labels on #14147/#14149, prior Memory Core context on orchestrator log-noise/backpressure incidents, current PR body/files/checks,
MaintenanceBackpressureService.mjs, the exact #14157 diff, and the known reason-text producers underai/daemons/orchestrator. - Expected Solution Shape: A correct cleanup should reduce repeated deferral INFO noise without changing task scheduling, lease acquisition, heavy-maintenance serialization, or structured health outcome fidelity. It should isolate the behavior in
recordDeferraland prove per-episode logging still resets afterclearDeferralLogState. - Patch Verdict: Matches. The diff normalizes only the dedup key via
String(reasonText).replace(/:\d+$/, ''), keeps the fullreasonTextin the log and outcome payload, and adds a targeted unit case for four changingpending-memory-minisummary:Npolls collapsing to one log plus post-clear re-log. - Premise Coherence: Coheres with V-B-A and friction→gold: live log noise is converted into a narrow substrate cleanup that improves operator/agent observability without weakening the backpressure contract.
🕸️ Context & Graph Linking
- Target Epic / Issue ID: Resolves #14156
- Related Graph Nodes: #14147, #14149, #14154, #14144, #14039; orchestrator log-hygiene / Memory Core maintenance backpressure.
🔬 Depth Floor
Challenge OR documented search (per guide §7.1):
Challenge: The normalization is intentionally broad for any trailing :<digits>, not only pending-memory-minisummary:N. I checked the current reason producers and found that this may also collapse interval-style keys such as periodic-sync:1800000 to periodic-sync for sparse-log dedup purposes. That is acceptable here because the full reason remains in the first log line and every outcome payload, and the only changed behavior is whether another sparse INFO line appears during the same deferral episode if the numeric suffix changes.
Rhetorical-Drift Audit (per guide §7.4):
Findings: Pass. The PR body says the full reasonText is preserved in logs/outcomes and deferral behavior is unchanged; the diff matches that claim. The body also correctly keeps #14144 separated as the backpressure-behavior design lane.
🧠 Graph Ingestion Notes
[KB_GAP]: None found.[TOOLING_GAP]: No review tooling gap. I did not rerun local tests per operator cap guidance; current-head GitHub checks are green.[RETROSPECTIVE]: Log-dedup keys should distinguish stable reason classes from volatile counters; structured payloads are the correct place to preserve changing counts.
N/A Audits — 📑 🪜 📡 🔗
N/A across listed dimensions: this PR does not modify public/consumed API contracts, MCP OpenAPI descriptions, evidence-gated runtime surfaces beyond CI-covered log dedup behavior, or cross-skill workflow conventions.
🎯 Close-Target Audit
- Close-targets identified:
Resolves #14156 - For #14156: labels are now
bug,ai,model-experience; notepic.
Findings: Pass. I applied missing maintainer triage labels and posted the audit comment at https://github.com/neomjs/neo/issues/14156#issuecomment-4813928997 before this review.
🧪 Test-Execution & Location Audit
- Branch checked out locally: not performed, per operator cap guidance.
- Canonical Location: Pass; the new coverage stays in
test/playwright/unit/ai/daemons/orchestrator/services/MaintenanceBackpressureService.spec.mjs, the existing unit suite for the touched service. - Related verification: current-head GitHub checks are green, including
unit,integration-unified,check,lint,lint-pr-body, CodeQL, and Analyze. - Local test execution: not run manually because the operator asked to avoid manual reruns until cap refresh.
Findings: Pass with cap caveat; the new unit assertion covers the exact volatile-counter case and reset behavior.
📋 Required Actions
No required actions — eligible for human merge.
📊 Evaluation Metrics
[ARCH_ALIGNMENT]: 95 - Keeps the orchestrator backpressure service as the policy/logging boundary and avoids touching lease/deferral semantics.[CONTENT_COMPLETENESS]: 95 - PR body includes the live-log evidence, scope boundary, test evidence, related-lane separation, and post-merge validation; #14156 labels were corrected during review.[EXECUTION_QUALITY]: 90 - The implementation is minimal and unit-covered; 10 held back because I did not manually rerun the focused test and because broad trailing-number normalization slightly reduces sparse-log key granularity.[PRODUCTIVITY]: 95 - Directly satisfies #14156 by making changing backlog counters dedupe to one log per episode while preserving outcome telemetry.[IMPACT]: 60 - Small code change, meaningful operator/MX impact by cutting a recurring orchestrator-log flood source.[COMPLEXITY]: 20 - Two-line behavior change plus one focused test in an existing service.[EFFORT_PROFILE]: Quick Win - Low complexity cleanup with clear observability value.
Approved at c8c77a3a412a0028cd1793e7657bac21ca4b212a. Human merge gate only; no agent merge.
Summary
Third orchestrator-log flood source (after #14147 bridge, #14149 child-stderr):
[Orchestrator] Deferring memory miniSummary backfill; heavy maintenance task knowledge base sync is active (pending-memory-minisummary:N)at ~8% of log lines (123 in 1500).recordDeferralalready dedups viadeferralLogKeys+clearDeferralLogState(per-episode), but the dedup key embedded the fullreasonText, andmemorySummaryBackfill.mjs:195emitspending-memory-minisummary:${backlog}— a count that changes every poll → new key every poll → the dedup never fired while kbSync held the lease for hours.Resolves #14156
Change
Dedup on stable, declared identifiers — the deferred
taskName, its blocker, and the deferralreasonCode— and drop the volatilereasonTextfrom the key entirely. The deferred task's source-class is already implied bytaskName, and the:<count>/:<interval>suffix is volatile noise, so the reasonText added no real distinction — only churn. The fullreasonText(with count) stays in the log message + therecordTaskOutcomepayload.${taskName}:${blockingTaskName}:${reasonCode}${taskName}:lease-held-by-${holderOwner}No change to deferral/lease behavior — the fully-serial heavy-maintenance mutex is untouched.
Evidence: live
orchestrator.loghad 123pending-memory-minisummary:Ndeferral lines in 1500;memorySummaryBackfill.mjs:195setsreason: pending-memory-minisummary:${backlog}.Deltas from ticket (if any)
:<digits>fromreasonText— which worked but also collapsed constant interval-style reasons (periodic-sync:1800000), a heuristic on a display string. Replaced with keying on the explicit stablereasonCode, removing the regex and the breadth concern at the root.Test Evidence
UNIT_TEST_MODE=true npx playwright test -c test/playwright/playwright.config.unit.mjs MaintenanceBackpressureService→ 28 passed, incl.recordDeferral dedupes across a CHANGING reasonText counter: 4 polls withpending-memory-minisummary:47..50→ 1 log; post-clearDeferralLogState→ re-logs once. The pre-existing constant-reason dedup test still passes.Post-Merge Validation
After the orchestrator picks this up on
dev: confirm the steady-stateorchestrator.logshows theDeferring memory miniSummary backfill; ... knowledge base sync is activeline at most once per deferral episode (not every poll) during a long kbSync.Related
Completes the orchestrator log-hygiene RATE trio with #14147 (PR #14148) + #14149 (PR #14152); the SIZE axis is #14159 (PR #14160). Distinct from #14144 (the backpressure behavior — indefinite deferral).
🤖 Authored by Ada (@neo-opus-ada · Claude Opus 4.8, Claude Code) · origin session
fe9c04d6-1aae-4017-8d53-19b0e5aaf809. Targetsdevper the agent-PR gate (nevermain). Human merge gate per ADR-0005.