What the PR does. omacom/omarchy#13968 (buger) replaces stripJsonc's two regex passes with one string-aware scanner and makes parseMenuJsonc return no rows for an array root. Fixes #13250 (comma in a string eaten), #13492 (array root -> rows "0","1") and #13493 (inline // empties the file); docs/menu.md states the rules. The evaluated tree 9f26c213 is AHEAD of the PR head ff77cd02: it also contains the pending, not-yet-public follow-up b2dae8db, which reads any JS \s character outside strings (BOM, Unicode spaces, VT, FF) as a space. The rest of the mirror is dogfood proof work. This is an advisory, audit-only review of an upstream PR we observe: it is not an approval.
Risk rationale: Small, well-covered parser change with differential evidence for all three fixes. But the public PR head lacks the Unicode-whitespace fix the package evaluates, so the package does not show the head's behaviour. The policy budget raise is a protection_weakened disclosure a policy approver must judge, and no test outcome is observed. Not low risk.
Read the full review (4 more paragraphs)
Is the fix verified? The changed code is fully covered (17/17 decision lines, 34/34 decisions), and reviewer node differentials match each issue. No test outcome is observed (test_results_not_joined), so no obligation cell counts as covered. Engine readiness: blocked (introduced); 4 introduced missing_obligation blockers on 66FW/BMFE/C8W1/BNZG. Next decision: human_judgement on 1 protection reduction (policy_changed_unclassified: max_variables 128->130), by a policy_approver under the approved policy @ 6fe078f2 with prior-policy signoff.
Questions for a human. (1) At ff77cd02 WITHOUT b2dae8db, a BOM / NBSP / VT before a // comment yields 0 rows where baseline yields 1: the public PR still carries that regression. (2) Do QV4 (Quickshell) and node agree on /\s/ and on cost? (3) Previously ignored user extensions now load and override defaults.
Traceability. stripJsonc/parseMenuJsonc are traced at function level (E4J2, 3T3F, 66FW, C8W1, BNZG, BMFE); 24 more requirements are touched only through menu-test.sh's file-level trace. No spec-gap candidates. 3 unmapped files are proof/test code.
Not caused by this PR. obligation_* gates fail identically at baseline. known_issue_sibling_disposition is introduced via the KI ledger (3 open parsing siblings). Unavailable: change effects (JS), per-test outcomes.
AI-generated review opinion derived from the evidence package. It is not a verdict, does not change readiness, and must be confirmed by a human reviewer.
14 context notes (info — not a finding)
- AI · confirmed#13250: a comma is dropped only when the next token is } or ] (SW-REQ-260927-66FW)
Upstream change. stripJsonc's second regex pass
,(\s*[}\]])did not know about strings, so, ]inside a label or action lost its comma. The new scanner copies string contents verbatim (MenuModel.js:13-19) and, outside strings, looks ahead from each comma past whitespace and // comments (MenuModel.js:26-38). It keeps the comma unless the next other character is } or ]. Reviewer differential (node, outside the package):{"b":{"label":"x, ]y","action":"mv f{.bak,}"}}gives label 'x ]y' / action 'mv f{.bak}' at baseline 6fe078f2 and 'x, ]y' / 'mv f{.bak,}' at the evaluated tree. SW-REQ-260927-66FW is re-applied dogfood proof work that states this rule. - AI · confirmed#13493: a // comment outside strings is dropped wherever it starts on the line (SW-REQ-260928-C8W1)
Upstream change. The baseline removed only whole-line comments (
^\s*\/\/with the m flag), so an inline tail reached JSON.parse and the whole file gave 0 rows. The scanner drops from the opener up to, not including, the line feed (MenuModel.js:23-25), so the tokens on each side stay apart. Reviewer differential:{"a":{"label":"A"}} // notegives 0 rows at baseline and 1 at the evaluated tree. A CRLF file with inline comments and trailing commas also parses (the \r is read as a space). An opener inside a string (https://x.y//z) is still copied as data. - AI · confirmed#13492: an array root yields no rows (SW-REQ-260928-BMFE)
Upstream change.
typeof [] === "object", so the baseline let an array root through and for..in turned its indices into entries "0", "1". parseMenuJsonc now addsArray.isArray(parsed)to the non-object root check (MenuModel.js:100). Reviewer differential:[{"label":"x"},{"label":"y"}]gives ids 0,1 at baseline and [] at the evaluated tree. - AI · confirmedSW-REQ-260922-E4J2 (JSONC stripping) is reworded to the single-pass scanner
E4J2's description and rationale change (dogfood proof work). It now says comments and trailing commas are stripped outside string literals before JSON.parse, so the authored JSONC parses. That matches stripJsonc as read (MenuModel.js:2-48). It is traced at function level to stripJsonc and parseMenuJsonc.
- AI · confirmedSW-REQ-260922-3T3F (unparseable / non-object input gives an empty set) gains the array-root case in its rationale
Only the rationale changes. An array root now joins null, numbers and strings as a non-object root (MenuModel.js:100). The parse-failure path (MenuModel.js:92-96) is unchanged: a file that still fails to parse (e.g. a /* */ block comment) contributes no entries.
- AI opinion — unverified24 further requirements are touched only through the whole-file trace of menu-test.sh (and re-review stamps)
test/shell.d/menu-test.sh is traced at file level to 25 requirements (weak_mapping), so every requirement it verifies is listed as changed, though the product change touches only stripJsonc/parseMenuJsonc. Their YAML diffs on this mirror are trace re-review stamps (c59dbbb7) and a PPDW wording change; the code they govern (merge, routing, guards, search) is not modified. Read against the code, none of their behaviour changes. Opinion: splitting menu-test.sh's file-level Verifies header into per-block markers would stop every JSONC test edit from touching ~25 requirements.
- AI opinion — unverifiedThe purpose branches are covered: 17/17 changed decision lines and 34/34 changed decisions, but no test outcome is observed
review.diffCoverage (mcdc:aggregate+bash+js+qml, measured) reports 0 uncovered lines and 0 uncovered outcomes in the changed code, which includes the string branch, the comment eater, the comma look-ahead, the \s branch and the array-root check. Coverage says those branches EXECUTE under the menu tests. It does not say an assertion passed: testResults is not_configured, so every obligation cell is unobserved or not_linked (test_results_not_joined). No test is recorded as passing or failing. Reviewer re-runs outside the package (node differential, see the other findings) agree with the stated behaviour.
- AI · confirmedobligation_decomposition_complete and obligation_evidence_complete fail identically at baseline (inherited)
Both are failed_behavior with delta unchanged against baseline run-a1d72522 (deferred obligations + accepted KI-debt). The package marks both disclosures preExisting.
- AI opinion — unverified16 further missing_obligation reasons sit on requirements touched only at file level
Not marked pre-existing in the package (preExisting unset). SW-REQ-260922-2JZT, SW-REQ-260922-3T3F, SW-REQ-260922-74BZ, SW-REQ-260922-JRW1, SW-REQ-260922-MQ37, SW-REQ-260922-N3RM, SW-REQ-260922-RGCV, SW-REQ-260922-W17G, SW-REQ-260922-Y58B, SW-REQ-260922-Z680, SW-REQ-260928-8VJQ, SYS-REQ-260922-0M8A, SYS-REQ-260922-PPDW, SYS-REQ-260922-R8DQ, SYS-REQ-260922-V7W6, SYS-REQ-260927-WC89. Context, not attribution: none of them governs stripJsonc/parseMenuJsonc. Each is listed because menu-test.sh is traced to it at file level, and the obligation gates that count them are unchanged from baseline. A human should confirm this against the baseline. The disclosures stay as the engine states them.
- AI · confirmedUnmapped changed file proof.yaml: project configuration, no product intent
proof.yaml changes only the max_variables budget and its comment (see the policy finding). It ships no product behaviour, so no requirement should claim it. It remains part of the unmapped_code disclosure, which stands.
- AI · confirmedUnmapped changed file menumodel-replay.test.mjs: verification code that carries a Verifies header
The node:test replay of menu-test.sh's MenuModel.js assertions (adds the 66FW/BMFE/C8W1/BNZG witnesses, MC/DC rows and the Unicode differential). Line 1 carries
// Verifies:for 24 requirements including the four JSONC ones, yet the package records mapping none for it. Position intended: its intent is verification of those requirements. A human may want to check why the engine does not count the .mjs Verifies header as a trace. The unmapped_code disclosure stands. - AI · confirmedUnmapped changed file report-reproducers.test.mjs: the three JSONC report tests flipped from expected failure to fixed
The #13250/#13492/#13493 reporter-script tests now expect exit 0 and the
ok:line (report-reproducers.test.mjs:36-59). TheirReproduces:markers stay, so they guard against the defects returning. Test code bound to KIs, not product behaviour. The unmapped_code disclosure stands. - AI opinion — unverifiedChange-effects analysis is unavailable for JS; function-level facts are bounded
review.effects is absent (effects_unavailable), so the package has no new callees, gained effects, callers or new mutable state for stripJsonc/parseMenuJsonc. Read by hand: the new code adds only locals (input, out, inString, i, next), no module state, and no calls beyond String, RegExp.test and Array.isArray. The callers are Menu.qml's wrappers (Menu.qml:245, 260). review.scope reports no spec-gap candidates and coherence single.
- AI opinion — unverifiedMirror composition: upstream product change vs dogfood proof work
0e28f60d = upstream PR ff77cd02 (MenuModel.js, docs/menu.md, its menu-test.sh hunk) + pending efa5170b, which was amended to b2dae8db in 0eec885b (VT and FF added to the whitespace tests). The rest is proof work: 66FW/BMFE/C8W1 re-applied and reworded, new SW-REQ-261001-BNZG, DEFECT-260927-CMMA/-4NG9/-7AX9, REVIEW-21/72/73/261001-ENZ2, three KIs set to fixed, the report-reproducer flip, witnesses/differentials, trace re-reviews, review-citation re-location and the menu budget 128->130. The shipped default menu parses to the same 344 rows at baseline and at the evaluated tree (reviewer-checked).