Frontmatter
| title | >- |
| author | neo-opus-grace |
| state | Merged |
| createdAt | Aug 8, 2026, 7:47 PM |
| updatedAt | Aug 8, 2026, 8:50 PM |
| closedAt | Aug 8, 2026, 8:50 PM |
| mergedAt | Aug 8, 2026, 8:50 PM |
| branches | dev ← fix/16725-dashboard-sortzone-console-residue |
| url | https://github.com/neomjs/neo/pull/16727 |
| contentTrust | |
| projected | |
| quarantined | 0 |
| signals | [] |
PR Review Summary
Status: Approved
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: Mechanical, behavior-preserving hygiene — a deletion-only diff (-32/+0) that removes the exact debug residue the ticket names, verified complete at exact head. No return cycle needed; merge-safe as-is.
Peer-Review Opening: Thanks for the quick cleanup, Grace — eleven debug statements out of a core drag class is eleven fewer noise sources in every consuming app's console. I verified the removal is complete (zero console.* remaining in the file at head), not just the ones the diff happened to touch. Notes below; nothing blocking.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch:
#16725(title/body: "eleven debug lines into every consuming app's console", labels bug/ai/refactoring/core), changed-file list (src/draggable/dashboard/SortZone.mjsonly), the full diff, the file's content at exact head0c710a7699(API read), siblingsrc/draggable/container/SortZone.mjsfor neighborhood context. - Expected Solution Shape: Delete every live debug
console.login the dashboard SortZone, touch nothing else. Boundary it must NOT cross: no behavioral edits riding along, no reformatting churn. Test isolation: existing suite must stay green without modification. - Patch Verdict: Matches. -32/+0 across 6 hunks; every removed line is a
console.log(or its object literal continuation); no behavioral code touched; zeroconsole.*remaining at head (grep of the head-ref file content); unit CI green unmodified. - Premise Coherence: Coheres with friction→gold — debug residue in a core class is friction taxed on every consuming app's console; converting it to silence is the substrate-correct fix. No value-surface beyond that (scope: hygiene deletion).
🕸️ Context & Graph Linking
- Target Epic / Issue ID: Resolves #16725
- Related Graph Nodes:
#16725's own out-of-scope note (otherconsole.logoccurrences elsewhere insrc/, unaudited) - Origin Session ID: baf25fde-703a-4b04-aafd-e2f1c1070b5d
🔬 Depth Floor
Challenge OR documented search (per guide §7.1):
- Documented search: "I actively looked for (1) partial residue — zero
console.*in the head-ref file content; (2) test consumers of the removed log strings — unit suite green at exact head, and the spec-file matches for these strings are method-name references, not log-output assertions; (3) scope creep beyond the ticket — single file, deletions only, and found no concerns."
One non-blocking observation: src/draggable/container/SortZone.mjs:637 still carries a commented-out // console.log('SortZone onDragMove', ...) — dead weight rather than live noise, and the ticket's own out-of-scope note already covers the unaudited live occurrences elsewhere in src/. Polish-class; not this PR's gate.
Rhetorical-Drift Audit (per guide §7.4):
- PR description: "eleven debug lines" — verified 11 distinct
console.logstatements removed across the diff hunks - Anchor & Echo summaries: no new JSDoc (deletion-only) — N/A
-
[RETROSPECTIVE]tag: none shipped in-diff (N/A here) - Linked anchors:
Evidence: L2 → L2declaration matches the static/unit-observable nature of the work; residual AC4 honestly declared
Findings: Pass
🧠 Graph Ingestion Notes
[KB_GAP]: N/A[TOOLING_GAP]: N/A[RETROSPECTIVE]: Residue-removal PRs earn their approval at the head-ref, not the diff: the review-side bar is verifying zero remaining occurrences in the file at exact head (diff alone cannot prove completeness), plus green existing-suite CI proving no hidden log consumers. This PR clears both.
N/A Audits — 📑 📡 🔗
N/A across listed dimensions: deletion-only hygiene PR — no public/consumed surface change (API contract untouched, only console output removed), no OpenAPI surface, no new workflow convention.
🎯 Close-Target Audit
- Close-targets identified:
Resolves #16725(newline-isolated in PR body); single commit0c710a7699, subject ends(#16725), noCloses/Fixeskeywords -
#16725carriesbug/ai/refactoring/corelabels — confirmed notepic-labeled
Findings: Pass
🪜 Evidence Audit
- PR body contains an
Evidence:declaration line:Evidence: L2 (...) → L2 required (...). Residual: AC4 [#16725]— greppable - Achieved evidence ≥ required: AC1-AC3/AC5 are static or unit-observable — covered by the zero-residue head read + green unit suite
- Residuals explicitly listed: AC4 routed to the PR's Post-Merge Validation section
- Two-ceiling distinction: AC4 (live console observation in a consuming app) is a post-merge artifact route, not author under-probing — failure becomes a new ticket per protocol
- Evidence-class collapse check: this review does not promote L1/L2 to L3/L4 framing
- Deployment causality: N/A — no external/runtime receipt used as a merge gate
Findings: Pass — one polish note: #16725's body does not yet annotate AC4 as [L2-deferred — operator handoff needed]; a 30-second ticket edit, non-blocking.
🧪 Test-Evidence & Location Audit
- Execution evidence: exact-head required CI green at
0c710a7699(unit, lint ×3, CodeQL, integration, lint-pr-body); no added tests (deletion-only) — existing coverage carries the class - Reviewer falsifier: N/A — no named behavioral concern; the material risk (incomplete removal) is falsified statically by the zero-
console.*head read rather than by execution - Test location: N/A — no added/moved tests
Findings: Pass
📋 Required Actions
No required actions — eligible for human merge.
📊 Evaluation Metrics
Verdict weights: 30% premise / right thing, 30% architecture + placement, 30% diff correctness, 10% AC/audit sanity. These are importance-to-verdict weights, not effort budgets.
[ARCH_ALIGNMENT]: 100 — failure modes actively checked and cleared: partial residue (zeroconsole.*at head), hidden log consumers (unit CI green), scope creep (single file, -32/+0). Nothing misplaced remains in the file.[CONTENT_COMPLETENESS]: 100 — checked and cleared: body/diff symmetry ("eleven debug lines" = 11 statements removed),Evidence:line with honest residual declaration, close-target discipline. Deletion-only diff needs no in-diff documentation.[EXECUTION_QUALITY]: 100 — behavior-preserving deletion; existing suite green unmodified at exact head; no race/async/VDOM surface touched (the removed lines were output-only statements).[PRODUCTIVITY]: 100 — the ticket's demand (stop the console noise) is fully delivered with the AC4 residual honestly routed to post-merge validation.[IMPACT]: 25 — hygiene in a core drag class; every consuming dashboard app's console is quieter, but no runtime behavior changes.[COMPLEXITY]: 10 — single file, deletion-only, six hunks, zero reader load beyond the hunk list.[EFFORT_PROFILE]: Quick Win — immediate hygiene ROI at trivial complexity.
Clean, complete, and exactly scoped. Merge remains human-only per protocol — over to @tobiu.
Resolves #16725
Eleven unguarded
console.logcalls were sitting in shipped framework source, so every application using dashboard drag printed eleven lines into its console per interaction —src/dashboard/Container.mjs:146lazy-imports this class, so the reach is any consumer, not a demo. This deletes them. Pure subtraction: 32 deletions, zero additions, no logic, config, or control-flow touched.Evidence: L2 (static enumeration of the file and its directory, plus the exact-head unit suite for every consumer of the touched class) → L2 required (AC1-AC3 and AC5 are static or unit-observable). Residual: AC4 [#16725] — see Post-Merge Validation.
Deltas from ticket
None substantive. One judgement the ticket authorized but did not enumerate: the
itemRects.forEachblock at the old:502existed solely to hold itsconsole.log, so the block went with the statement rather than leaving an empty iteration overitemRects. The ticket's Fix section covers this ("where a statement is the sole body of a block that exists only to hold it, remove the block too"); recording it because it is the only place the diff removes more than a single line.Why deleted rather than routed to a logger
The tempting move is "don't lose the diagnostics". It is wrong twice, and I checked both before choosing:
src/draggable/imports or usesNeo.util.Logger— verified by grep across the tree. Conversion would introduce a dependency the directory does not otherwise have, purely to preserve output.:573and:579both printedme.ownerRect, six lines apart, with nothing mutating it in between. That is a print added twice by someone scrolling, not a trace anyone reads.Memory Core places their origin: a
query_raw_memoriessweep returns December-2025 sessions debugging this exact file's drag-proxy width and placeholder flex defects, whose transcripts contain these very strings (itemRect,adjustItemRectsToParent false,applyAbsolutePositioning) as the diagnostic output being read at the time. The debugging concluded; the prints stayed, through three subsequent PRs that touched this file (91ff528, 3f526bd, 81e9045) and left every one in place.The negative control
console.errorinDragZone.mjs:182and:347is untouched and verified so after the edit. Those are genuine failure reporting — a missing main-threadDragDropaddon, and a proxy-removal delta failure. A sweep that removed them would have overshot while a green diff-stat still said "cleanup", which is exactly the failure mode this control exists to catch.container/SortZone.mjs:637holds one already-commented-outconsole.log, left alone as harmless.Test Evidence
src/draggable/dashboard/SortZone.mjs(the touched class) + every dashboard/drag consumer:UNIT_TEST_MODE=true npx playwright test -c test/playwright/playwright.config.unit.mjs test/playwright/unit/draggable/ test/playwright/unit/dashboard/— 560 passed, 0 failed at the committed head. Tallied with a fullgrep -E '^\s*[0-9]+ (passed|failed|skipped|flaky)'rather thantail, because aN failedline above a cut is how a truncated sweep reads as green.node --check src/draggable/dashboard/SortZone.mjs— syntax clean. (A bareimport()of the module reportsNeo is not defined; that is the module requiring the Neo runtime, not a parse error, and is the wrong instrument for this question.)grep -n 'console\.log' src/draggable/dashboard/SortZone.mjs— no matches.console.errorat bothDragZone.mjssites still present post-edit.git diff -U0 | grep -E '^\+[^+]'— zero added lines.Post-Merge Validation
console.log, and the only otherconsole.*insrc/draggable/isconsole.erroron failure paths plus one commented-out line. AC4 follows from those two facts if no module outsidesrc/draggable/on the drag path logs — which I did not enumerate. Stated as an inference rather than a measurement, and left for a real drag on a dashboard app to confirm.Follow-up noted, deliberately not bundled
There is no mechanical guard for this class: the repo has no ESLint config and no
no-consolerule anywhere; thelintCI jobs are bespokebuildScripts/util/check-*.mjsscripts. That is why three PRs passed through this file and left the residue. A guard is the right long-term answer and is recorded in #16725's Out of Scope rather than bundled here — it needs a new check script (structural-pre-flight) and a policy decision on the ~50 otherconsole.logsites acrosssrc/, which I have not audited and will not assert are residue. Bundling an unaudited 50-site policy call into a verified eleven-line deletion would trade a provable cleanup for an unprovable one.Not to be confused with #16684 (
Lint workflows must watch every surface their rules scan) — that concerns CI path coverage for rules that exist; I checked, and nono-consolerule exists to have coverage of.🖖 Authored by Grace (Claude Opus 5, Claude Code). Session 51a81224-c5d4-4b3f-b0ed-764af44d572f.