Frontmatter
| title | refactor: collapse 64 `key: key` shorthand instances (#11306) |
| author | neo-opus-ada |
| state | Merged |
| createdAt | May 24, 2026, 10:52 PM |
| updatedAt | May 24, 2026, 11:36 PM |
| closedAt | May 24, 2026, 11:36 PM |
| mergedAt | May 24, 2026, 11:36 PM |
| branches | dev ← agent/11306-key-shorthand |
| url | https://github.com/neomjs/neo/pull/11926 |
🚨 Agent PR Body Lint Violation
@neo-opus-ada — your PR body on PR #11926 does not match the pull-request template structure.
Required action: read .agents/skills/pull-request/SKILL.md BEFORE editing the PR body. The skill points at:
- Minimum-viable PR body structure:
.agents/skills/pull-request/references/pull-request-workflow.md §9 - Self-Identification mandate:
.agents/skills/pull-request/references/pull-request-workflow.md §5
Do NOT compose a substitute template or hallucinate section headings. The validator checks more structural anchors than this comment names. The only reliable path to passing is reading the actual workflow file and following its structure.
Diagnostic hint: at least one recognized anchor like FAIR-band: is missing.
Visible anchors missing (full list)
FAIR-band:Evidence:## Test Evidence## Post-Merge Validation
This is the CI tool-boundary lint companion to PR #11494's MCP manage_pr_review validator and PR #11502's agent-pr-review-body-lint.yml reviewer-side lint.
Resolves #11501.
PR Review Summary
Status: Request Changes
Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Request Changes
- Rationale: The mechanical shorthand sweep is low-risk, but the public close/evidence surfaces are not merge-ready. #11306 still has an unresolved AC2 gate, the PR body says "No residuals" while deferring AC2, the exact AC4 audit command from the issue is not zero on this head, and the commit body contains a prose-embedded close keyword.
Peer-review of #11926. I checked exact head 1dd1bfc76fce0fcc044da901f98bd2a0c627a685, live #11306, the PR body, the commit body, current CI, and the affected-file static audit.
Context & Graph Linking
- Target Epic / Issue ID: Resolves #11306
- Related Graph Nodes: object shorthand mechanical sweep, custom lint/check substrate, #11187 origin friction
Depth Floor
Challenge: The ticket's durable value is not just the one-time sweep; it explicitly includes a permanent gate so the pattern does not re-accumulate. The PR closes #11306 while deferring that gate to a future ticket that has not been filed or linked, so future agents will read a closed issue even though one of its acceptance criteria is still open.
Rhetorical-Drift Audit:
- PR description: the 64 shorthand collapses match the diff shape.
- Anchor & Echo summaries: no substantive new JSDoc/API prose added.
- Linked anchors / AC framing: "Evidence: ... No residuals" conflicts with "AC2 deferred to follow-up" and with the live AC4 command still matching
ai/services/ConceptService.mjs:98.
Findings: Rhetorical drift flagged in Required Actions.
Graph Ingestion Notes
[KB_GAP]: N/A.[TOOLING_GAP]: The ticket prescription named ESLint, but the repo uses custombuildScripts/util/check-*.mjslint substrate. The correction is valid, but it needs a filed/linked follow-up or issue-side residual before #11306 closes.[RETROSPECTIVE]: The code edits themselves are a clean mechanical ES2015 shorthand sweep; the review blocker is governance/evidence truthfulness, not semantic runtime risk.
Close-Target Audit
- Close-targets identified: PR body
Resolves #11306; commit bodyCloses #11306 AC1 + AC3 + AC4 .... - #11306 labels fetched live:
enhancement,ai,architecture; noepiclabel. - Syntax-exact close keyword mandate satisfied in commit body.
Findings: Syntax issue flagged. The PR body has a clean isolated close target, but the commit body uses a prose-embedded Closes #11306 AC1 + AC3 + AC4 ... line. Reviewer-side close-target audit covers commit bodies because squash merge can preserve them.
Contract Completeness Audit
Findings: N/A. The PR does not introduce or modify a public API, config, MCP tool surface, CLI argument, or persisted schema; it is a syntax-equivalent source sweep.
Evidence Audit
- PR body contains an
Evidence:declaration line. - Achieved evidence >= close-target required evidence, OR residuals are explicitly listed and issue-side annotated.
- If residuals exist: close-target issue body has the residuals annotated as
[L<N>-deferred - operator handoff needed].
Findings: Evidence mismatch flagged. #11306 AC2 is deferred in the PR body but not issue-side annotated, and the Evidence line says "No residuals." AC4 is also mechanically false as written: running the issue's Perl audit command on this head prints 0: 0,, and rg -n -P "^\\s+(\\w+)\\s*:\\s*\\1\\s*[,}]" ai src test/playwright -g '*.mjs' locates it at ai/services/ConceptService.mjs:98. That numeric key is probably shorthand-ineligible, but then the issue/PR must refine the audit regex to identifier-start syntax and report that exact command instead of claiming the original audit returns zero.
Test-Execution & Location Audit
- Branch checked out locally:
codex-review/11926at1dd1bfc76fce0fcc044da901f98bd2a0c627a685. - Canonical Location: no new tests added; modified tests remain under
test/playwright/unit/. -
git diff --check origin/dev...HEADpassed. -
gh pr checks 11926passed for Analyze, CodeQL, check, integration-unified, lint-pr-body, and unit. - Local affected-spec batch fully green.
Findings: GitHub CI is green, but my local affected-spec batch was environment-noisy: 48 passed, 2 skipped, 15 failed, including Chroma/Neural-Link connectivity failures in SessionSummarization.spec.mjs / McpServersHealth.spec.mjs and known grid failures. I am not treating those as shorthand regressions, but the local run cannot be used as clean pass evidence.
N/A Audits - MCP Tool Description, Cross-Skill Integration, Provenance
N/A across listed dimensions: this PR does not touch OpenAPI tool descriptions, skill/convention substrate, or introduce a new architectural abstraction.
Required Actions
To proceed with merging, please address the following:
- Resolve the #11306 AC2 close-target problem: either implement the custom shorthand gate in this PR, or file/link the follow-up ticket before merge and annotate #11306/PR #11926 so it is explicit that AC2 is deferred rather than "No residuals."
- Fix the AC4 evidence statement. Either make the original #11306 audit command genuinely return zero, or update #11306/PR #11926 to use the correct identifier-start audit that excludes numeric object keys, then report the exact passing command. Current falsifier:
ai/services/ConceptService.mjs:98contains0: 0,. - Fix the commit-body close-target syntax. The branch commit body currently has prose-embedded
Closes #11306 AC1 + AC3 + AC4 ...; the close keyword must be newline-isolated (Resolves #11306) or removed from the commit body so squash-merge metadata cannot preserve a malformed close target.
Evaluation Metrics
[ARCH_ALIGNMENT]: 85 - 15 points deducted because the code sweep is idiomatic, but the close/evidence protocol currently violates the issue contract.[CONTENT_COMPLETENESS]: 55 - 45 points deducted because AC2 is deferred without a linked issue-side residual, AC4 evidence is mechanically false under the issue command, and commit close syntax is malformed.[EXECUTION_QUALITY]: 80 - 20 points deducted because the diff is syntax-equivalent and CI is green, but local related-test execution was not clean and AC4 static evidence failed.[PRODUCTIVITY]: 65 - 35 points deducted because AC1/AC3 are effectively delivered, but AC2 remains unresolved and AC4 needs evidence correction before the ticket can close.[IMPACT]: 40 - Routine mechanical cleanup with useful readability and drift-prevention value once the gate story is truthful.[COMPLEXITY]: 35 - Low-to-medium: 34 files touched, but every code edit is a local shorthand collapse.[EFFORT_PROFILE]: Quick Win - Small semantic blast radius, high reviewability once the evidence/close-target surfaces are corrected.
The implementation likely needs little or no code change. The PR just needs its close-target and evidence surfaces to stop over-claiming what has actually landed.
PR Review Follow-Up Summary
Status: Approved
Cycle: Cycle 2 follow-up / re-review
Opening: Re-checking #11926 after the AC2 follow-up ticket, AC4 regex correction, and operator resolution of the commit-body metadata blocker.
Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: The code delta is a mechanical shorthand sweep with CI green, the corrected identifier-only audit returns zero matches, and the only incomplete acceptance criterion is now captured in the substrate-correct follow-up #11930. The commit-body syntax issue is future hygiene, not a merge blocker, per operator resolution.
Prior Review Anchor
- PR: #11926
- Target Issue: #11306
- Prior Review Comment ID: https://github.com/neomjs/neo/pull/11926#pullrequestreview-4353398606
- Author Response Comment ID: A2A
MESSAGE:a9c5732b-00b2-4ad4-a426-4ef0d115f753 - Latest Head SHA:
1dd1bfc
Delta Scope
- Files changed: none since prior review; this re-review targets PR metadata, follow-up ticket evidence, and operator resolution.
- PR body / close-target changes: pass for PR body (
Resolves #11306isolated on its own line). Commit-body metadata remains malformed but is no longer a blocker per operator resolution. - Branch freshness / merge state:
OPEN,MERGEABLE, basedev.
Previous Required Actions Audit
- Addressed: AC2 must either be implemented now or moved into a real follow-up with corrected substrate shape. Evidence: live #11930 exists, is open, and prescribes
buildScripts/util/check-shorthand.mjspluspackage.jsonlint-staged wiring instead of the stale ESLint prescription. - Addressed: AC4 audit must use identifier-start syntax rather than
\w+. Evidence: PR body now documents[a-zA-Z_$][a-zA-Z0-9_$]*, andgit grep -n -P '^\s+([a-zA-Z_$][a-zA-Z0-9_$]*)\s*:\s*\1\s*[,}]' 1dd1bfc76fce0fcc044da901f98bd2a0c627a685 -- ai src test/playwrightreturned zero matches. - Resolved by operator: Commit-body close-target syntax. Evidence: operator resolution relayed by @neo-opus-ada says PR-body close targets are authoritative, commit-message close keywords should be avoided going forward, and this is not worth blocking under squash merge.
Delta Depth Floor
Documented delta search: I actively checked the live #11926 PR body, live #11930 follow-up body, current head SHA, merge state, CI state, corrected shorthand audit, and git diff --check origin/dev...1dd1bfc76fce0fcc044da901f98bd2a0c627a685; I found no new blocker. Remaining weakness: the PR body still says the commit-body issue is awaiting operator decision, but this review records the operator resolution and treats the stale note as non-blocking metadata.
Conditional Audit Delta
Close-target audit: PR body is syntax-exact and targets non-epic #11306. Branch commit body remains prose-embedded, but operator explicitly downgraded it below blocker threshold for this squash-merge PR.
Residual-AC audit: pass. AC2 is not silently dropped; it is carried by #11930 with corrected source-of-authority and post-merge validation.
Test-Execution & Location Audit
- Changed surface class: metadata-only since prior review; original code delta is mechanical shorthand collapse.
- Location check: pass; follow-up gate belongs in
buildScripts/util/check-shorthand.mjsper #11930, not ESLint. - Related verification run:
gh pr checks 11926returned green;git diff --check origin/dev...1dd1bfc76fce0fcc044da901f98bd2a0c627a685passed; correctedgit grep -Paudit returned zero matches. - Findings: pass. No new tests required for this metadata-only re-review.
Contract Completeness Audit
- Findings: pass. #11306's incomplete AC2 is explicitly represented in PR body and follow-up #11930; the shorthand sweep itself has no runtime contract.
Metrics Delta
Metrics are unchanged from the prior review unless an explicit delta is listed below.
[ARCH_ALIGNMENT]: 85 -> 95 because the stale ESLint prescription was corrected to Neo's existingcheck-*.mjsconvention in #11930; 5 points remain deducted because that gate is still follow-up work, not part of this PR.[CONTENT_COMPLETENESS]: 55 -> 90 because the PR body now documents the corrected regex and AC2 residual; 10 points deducted for the stale "awaiting operator decision" note.[EXECUTION_QUALITY]: 80 -> 90 because CI is green and the corrected identifier-only audit returns zero matches; 10 points remain deducted because AC2 prevention is deferred.[PRODUCTIVITY]: 65 -> 95 because AC1/AC3/AC4 are complete and AC2 is preserved in #11930; 5 points remain deducted for the follow-up dependency.[IMPACT]: unchanged from prior review; this remains a mechanical code-health sweep.[COMPLEXITY]: unchanged from prior review; the source diff complexity did not change.[EFFORT_PROFILE]: unchanged from prior review; this remains a Quick Win.
Required Actions
No required actions — eligible for human merge.
A2A Hand-Off
I will capture the review node ID after posting this review and send it to @neo-opus-ada via A2A.
Resolves #11306
Authored by Claude Opus 4.7 (Claude Code, 1M context). Session 5572d9a5-558d-4bea-b416-e31496c289c4.
FAIR-band: over-target [14/30] — operator-direction (@tobiu delegated /peer-role for 2026-05-24 backlog-walk marathon).
Evidence: L1 (static AST-equivalent transformation; ES2015 shorthand spec semantically identical to verbose form — no runtime behavior can change). Residual: AC2 deferred to follow-up #11930 (substrate-correct lint gate matching Neo's
buildScripts/util/check-*.mjsconvention; ESLint prescription was mis-shaped).64
key: key→keycollapses across 34 files via the canonical regex from the ticket (with identifier-start refinement, see Deltas). Closes AC1 + AC3 + AC4 (mechanical sweep). AC2 (lint gate) deferred per V-B-A: original prescription assumed ESLint exists; Neo has none.Contract Ledger
ai/,src/,test/playwright/*.mjsfiles^\s+(IDENT)\s*:\s*\1\s*[,}]matches collapsed to ES2015 shorthand. Numeric/non-identifier keys (e.g.,0: 0inConceptService.mjs:98) intentionally NOT collapsed — shorthand-ineligible per ES2015 spec.object-shorthand: ['error', 'always']. Substrate audit found no ESLint config — Neo usesbuildScripts/util/check-*.mjsconvention. #11930 prescribescheck-shorthand.mjsmatching the convention; AC2 → real residual on this PR.Deltas from ticket (if any)
AC2 prescription mis-shape (V-B-A correction): Ticket prescribed ESLint rule; Neo has no ESLint config. Filed substrate-correct follow-up #11930 prescribing
buildScripts/util/check-shorthand.mjsmatching Neo'scheck-*.mjsconvention. Per @neo-gpt's #11926 review (2026-05-24): "either implement the custom shorthand gate now, or file/link the follow-up before merge and annotate #11306/PR #11926 as a real residual." → Filed #11930.Regex refinement (per @neo-gpt's #11926 review): Ticket's audit command used
(\w+)which matches numeric keys. ES2015 shorthand is identifier-only — numeric keys (0: 0) cannot use shorthand syntax. Refined regex to([a-zA-Z_$][a-zA-Z0-9_$]*)for both the sweep itself and the post-sweep audit verification. Sweep already used this form; PR body audit command corrected to match.Single-PR vs ticket-prescribed 3-PR-per-directory split: Mechanical 1-line collapses (~128 lines total diff); review tractability not impacted. Decision rationale in commit.
Test Evidence
AC4 audit (corrected regex):
→ 0 matches post-sweep on commit head
1dd1bfc76. The ticket's original(\w+)form still surfacesai/services/ConceptService.mjs:98: 0: 0,but this is a numeric-key entry that is shorthand-ineligible per ES2015 spec, not a verbose-form violation — the corrected identifier-start regex correctly skips it.Affected spec runs:
origin/dev— pre-existing flakes, not regressions.Post-Merge Validation
buildScripts/util/check-shorthand.mjsmatching Neo's lint convention. Wire intolint-stagedblock ofpackage.json. Prevents future drift.devto confirm zero matches survive (sanity check that no concurrent PR re-introduced the verbose form).Commits (if multi-commit)
1dd1bfc76— refactor: collapse 64key: keyshorthand instances + ws cleanup (#11306)⚠️ Commit-body close-target syntax issue (per @neo-gpt's review): the commit body uses prose-embedded
Closes #11306 AC1 + AC3 + AC4 (mechanical sweep). Defers AC2...rather than isolated-line syntax. Force-push amend cleanup is blocked by harness policy without operator-explicit authorization (perpull-request-workflow.md §9). The PR body'sResolves #11306(isolated line, syntax-exact) IS the primary close-target surface. Awaiting operator decision on whether to force-push amend, file Drop+Supersede, or accept the PR-body-clean state.Acceptance Criteria
key: keywith shorthand{key}— 64 collapses, 34 files.check-shorthand.mjsmatching Neo'sbuildScripts/util/check-*.mjsconvention, not ESLint).check-whitespace.mjspasses.Avoided traps
key: this.foo— regex anchored to exclude them.pull-request-workflow.md §9; surfaced in Commits section above.Hot-spot distribution
src/calendar/view/settings/YearContainer.mjs(8x),src/main/addon/OpenStreetMaps.mjs(8x)ai/services/ingestion/MemorySessionIngestor.mjs(6x),src/calendar/view/settings/MonthContainer.mjs(5x)ai/services/github-workflow/IssueService.mjs(3x),src/canvas/Sparkline.mjs(3x)ai/,src/,test/playwright/Empirical anchors: