explain: read an array literal member by member, and only where it is one (#225) - #256
Conversation
… one (#225) #225 V1 (interior). Each member of a `{…}`/`(…)` literal becomes its own `values.occurrences[]` row — `kind: "element"`, its own span and shape hints, a `parent` naming the container, and `name` for a keyed member. This is the expression/parser foundation #239 S2 and #255 are sequenced behind. Grounding it on CHR 7.23.3 found two hints already shipping that the device contradicts, and both are fixed here: - A `{…}` array literal is a syntax error in a CLI command argument. `/console/inspect` classes the `{` byte `error` and `:parse` refuses `comment={1;2}`, `servers={1.1.1.1;8.8.8.8}` (a LIST-typed attribute, which rules out a schema-shaped reading), `.proplist={…}`, the relative spelling, and `:log info message={1;2}`; only a root scripting directive's value slot takes one. centrs returned `pass` + `array` on all of them, including `source={ :return 1 }` script bodies. - There is no class-based wide IPv4 field. Every part is one octet: `1.256` is `time` `00:00:01.256` and `1.16777215` is `time`, never `1.255.255.255`. 26 corpus literals — GPS coordinates, fractional-second timings — were `ip`. A member is parsed as an EXPRESSION, so it has its own lexicon: a bare word is a variable reference (`{abc}` is `$abc`), as is a MAC and any time literal past 2^63 nanoseconds (the cliff is `15250w`/`15251w`); `*1` and `+1` do not parse there though `:local x *1` is `id`; hexadecimal is a number (`0x10` is 16, `0X10` is a variable). `;` splits braces and `,` splits parens — `{1,2}` is ONE member — and `=` binds a key in braces but compares in parens. An empty member is a syntax error and withdraws the enclosing `array` shape with it. The bare comma spelling is the first genuinely plural hint. `=1,2,3` is a syntax error nowhere, and only the argument's type decides whether it splits (`servers=1.1.1.1,8.8.8.8` lowers to a `;` list; `comment=a,b` stays one string), so a named attribute carries `["array", "str"]` and a directive's value slot carries `["array"]` alone. No members are located for it — the split is a `schemaType` fact — while the `(1,2)` spelling is descended into as usual. Also adds `bun run explain:value-census`. #242's corpus figures came from a throwaway script, so when the emission changed there was no way to recompute them under the same definitions; the fixture now records the command that derives them. Evidence - 61 device-typed member rows in `test/fixtures/explain/values.json`, scored one-sided: an abstention is always allowed, a contradicting type name never. That test is what caught the IPv4 bug. - Corpus (948 scripts): 19,709 values, 5,636 members (529 keyed, 1,147 nested); 0 boundary contradictions, 0 invalid spans, 0 dangling parents, 0 containment breaks. - `bun run lint:ci`, `bun test` (2,435 pass), `bun run build` - `bun run test:integration` — 146 pass, 0 fail on CHR 7.23.3, including new examples 28 and 28b Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe explain pipeline now supports array-element occurrences, nested parent links, context-specific shape inference, comma handling, and stricter literal validation. A SQLite-backed census CLI and expanded fixtures, tests, examples, and documentation validate the behavior. ChangesArray-aware value analysis
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Script
participant Explain
participant lexValueAnchors
participant valueShapeHints
Script->>Explain: analyze RouterOS statement
Explain->>lexValueAnchors: collect value anchors
lexValueAnchors->>lexValueAnchors: parse array members and parent links
lexValueAnchors->>valueShapeHints: infer contextual member shapes
valueShapeHints-->>lexValueAnchors: return shape hints
lexValueAnchors-->>Explain: return occurrences
Explain-->>Script: return explained values
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR advances explain’s offline value analysis (#225 V1) by flattening array literals into per-member values.occurrences[] entries (with parent linkage), and by introducing a distinct “array-member” shape-hint lexicon that matches RouterOS expression parsing inside {…}/(…).
Changes:
- Descend located array literals and emit each member as
kind: "element"with its own span, hints, optional keyname, andparentid. - Split value-shape hinting by context (
argumentvsarray-member), including corrected IPv4 shortcut bounds and member-only hex/time rules. - Add a re-derivable corpus census script (
bun run explain:value-census) and refresh fixtures/docs/tests accordingly.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit/explain.test.ts | Adds unit coverage for new example 28/28b value-occurrence shapes and member spans. |
| test/unit/explain-values.test.ts | Extends grounding + anchor tests for member lexicon, brace legality, comma spelling, and updated corpus invariants. |
| test/unit/explain-args.test.ts | Updates anchor-lexer tests to enable brace-array scanning where appropriate. |
| test/integration/explain-values.test.ts | Adds CHR-backed grounding for example 28/28b and validates device behavior for members/brace gating. |
| test/integration/chr.ts | Allows integration evidence to reference letter-suffixed examples (e.g., 28b). |
| test/fixtures/explain/values.json | Updates corpus + interior grounding fixture with member facts, invariants, and new census fields. |
| src/index.ts | Exposes new public types (ExplainValueAnchorKind, ExplainValueContext). |
| src/explain/values.ts | Introduces ValueContext and member-specific hint rules (hex, time range, comma behavior, fixed IPv4 shortcut bounds). |
| src/explain/args.ts | Adds element anchors + member descent and gates brace-array parsing by statement/path context. |
| src/explain.ts | Composes element occurrences into the envelope, including parent id linkage and member-context shape hinting. |
| scripts/explain-value-census.ts | New script to compute the fixture’s corpus census deterministically. |
| package.json | Adds explain:value-census script entry. |
| docs/MATRIX.md | Updates phase-0.5 file list to include values.ts. |
| commands/explain/README.md | Documents the new element-occurrence shape and interior rules (needs number sync per comments). |
| commands/explain/examples.md | Adds examples 28 and 28b narrative and expected output shape. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@commands/explain/README.md`:
- Around line 743-752: Update the corpus census paragraph in README.md to match
the checked-in assertions in test/unit/explain-values.test.ts: use 19,709 value
occurrences, 13,168 strict comparable anchors, 6,541 recovered prefix anchors,
570 unread statements with anchors, and 814 arrays. Preserve the remaining
census details unless regenerated output requires corresponding corrections,
then run bun run lint:ci.
In `@scripts/explain-value-census.ts`:
- Around line 64-72: Update the Emitted interface to include an optional name
field, then remove the casts around the Emitted object literal and keyedElements
entry so TypeScript validates them directly. Preserve the existing keyedElements
counting behavior while relying on the expanded Emitted type.
- Around line 234-249: Update the main execution flow around main and its
promise handlers to avoid calling process.exit immediately after console output.
Assign the resolved status to process.exitCode, and set process.exitCode to 1 in
the catch handler while preserving the existing error logging, allowing stdout
to drain before termination.
In `@test/fixtures/explain/values.json`:
- Around line 607-611: Remove the duplicate probe rows from the fixture arrays:
retain only one `interiorGrounding.members` entry for literal `100000w` with
type `nothing` and value `""`, and only one `interiorGrounding.positions` entry
for the `/ip/route/add comment=[:tostr {1;2}]` input with `parses: true`.
Preserve all other fixture entries unchanged.
In `@test/unit/explain.test.ts`:
- Around line 384-412: In the test case “28. An array literal is read member by
member, and only where it is one (`#225`)”, declare the repeated statement as a
single input variable before calling explainJson, pass that variable to
explainJson, and reuse it for the span-slicing assertions. Remove the duplicated
string literal while preserving all existing expectations.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0df49f39-806a-4139-9f43-bdf2a1941bc1
📒 Files selected for processing (15)
commands/explain/README.mdcommands/explain/examples.mddocs/MATRIX.mdpackage.jsonscripts/explain-value-census.tssrc/explain.tssrc/explain/args.tssrc/explain/values.tssrc/index.tstest/fixtures/explain/values.jsontest/integration/chr.tstest/integration/explain-values.test.tstest/unit/explain-args.test.tstest/unit/explain-values.test.tstest/unit/explain.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: CHR smoke (stable)
- GitHub Check: copilot-pull-request-reviewer
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Transport or RouterOS-touching code is not done untilbun run test:integrationpasses.
Do not silently fall back to another protocol when the caller pinned--via.
Do not make generated output the hand-edited source of truth.
Do not disable validation to make a test pass; validation is part of the product.
Files:
test/integration/chr.tssrc/index.tstest/unit/explain-args.test.tstest/integration/explain-values.test.tstest/unit/explain.test.tssrc/explain/values.tssrc/explain.tssrc/explain/args.tstest/unit/explain-values.test.tsscripts/explain-value-census.ts
docs/MATRIX.md
📄 CodeRabbit inference engine (AGENTS.md)
Read
docs/MATRIX.mdafter the constitution; use it as the only command×protocol status surface and work on the highest-priority cell that is notCHR-passed.
Files:
docs/MATRIX.md
{docs/**,.github/instructions/**,.github/**/*.yml,.github/**/*.yaml,**/*.{md,txt,dict}}
📄 CodeRabbit inference engine (AGENTS.md)
Run
bun run lint:ciwhen changing documentation, instructions, security configuration, spelling dictionaries, or workflow files.
Files:
docs/MATRIX.mdcommands/explain/examples.mdcommands/explain/README.md
commands/*/examples.md
📄 CodeRabbit inference engine (commands/AGENTS.md)
commands/<name>/examples.mdmust contain numbered, runnable examples, with example N corresponding to assertion N intest/integration/<name>.test.ts; these examples define whatCHR-passedis measured against.
Files:
commands/explain/examples.md
commands/*/{README.md,examples.md}
📄 CodeRabbit inference engine (AGENTS.md)
Read the target command's
README.mdandexamples.mdas the executable specification before writing code or tests.
Files:
commands/explain/examples.mdcommands/explain/README.md
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (src/AGENTS.md)
src/**/*.{ts,tsx}: Use Bun-native TypeScript and Web APIs where possible
Errors must be actionable for humans and agents, with next-step guidance when a dependency, protocol, credential, or validation source is missing
Files:
src/index.tssrc/explain/values.tssrc/explain.tssrc/explain/args.ts
test/integration/**/*.test.{ts,js}
📄 CodeRabbit inference engine (test/AGENTS.md)
Put long-running, RouterOS-backed, or platform-specific tests (including process-level tests that spawn the real
src/cli.tsthroughcli-process.tsand network-free CLI smoke tests incli-smoke.test.ts) undertest/integration/and wire them through QA or lab workflows.
Files:
test/integration/explain-values.test.ts
commands/*/README.md
📄 CodeRabbit inference engine (commands/AGENTS.md)
commands/*/README.md: Eachcommands/<name>/README.mdis the executable specification's designed tier: document intent, flags, and behavior; include aDesigned, not implementedtable for spec-only flags; do not duplicate implemented-flag tables generated indocs/CLI.md; link todocs/CONSTITUTION.mdrather than restating constitution-wide rules; document only command-specific behavior; and keep theStatusline consistent withdocs/MATRIX.md.
Implemented flags must be generated fromCliCommandMetadataintodocs/CLI.mdviabun run docs:cli; command READMEs must not maintain duplicate tables of implemented flags.
Files:
commands/explain/README.md
test/fixtures/**/*
📄 CodeRabbit inference engine (test/AGENTS.md)
Keep fixtures under
test/fixtures/with clear source/provenance notes.
Files:
test/fixtures/explain/values.json
🧠 Learnings (2)
📚 Learning: 2026-08-06T05:37:57.693Z
Learnt from: mobileskyfi
Repo: tikoci/centrs PR: 222
File: src/explain/defects.ts:1-70
Timestamp: 2026-08-06T05:37:57.693Z
Learning: In TypeScript modules under src/explain, retain references to phase-0 lab questions and issue numbers when they document the technical rationale for current implementation behavior or contracts. Remove comments containing dated decisions, delivery status, or future roadmap information when it does not explain current behavior, including in tests.
Applied to files:
src/explain/values.tssrc/explain/args.ts
📚 Learning: 2026-08-06T23:42:32.848Z
Learnt from: mobileskyfi
Repo: tikoci/centrs PR: 227
File: src/explain/args.ts:151-158
Timestamp: 2026-08-06T23:42:32.848Z
Learning: In the RouterOS explain lexer, use the shared `continuationLength(text, at)` helper for backslash-newline handling. Treat only `\n` and `\r\n` as continuations; a lone `\r` after a backslash must not be skipped. In `args.ts`, reject a lone-`\r` escape as invalid so token names and source spans remain accurate.
Applied to files:
src/explain/args.ts
🪛 OpenGrep (1.26.0)
test/integration/explain-values.test.ts
[ERROR] 429-431: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
[ERROR] 455-457: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
[ERROR] 514-514: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
[ERROR] 533-533: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
[ERROR] 547-547: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
[ERROR] 565-567: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
[ERROR] 593-593: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🔇 Additional comments (16)
src/index.ts (1)
317-317: LGTM!Also applies to: 389-389
scripts/explain-value-census.ts (2)
127-157: LGTM!Also applies to: 159-184
100-123: 📐 Maintainability & Code QualityNo change needed for census shape selection.
scripts/explain-value-census.tsalready usessrc/explain/values.tsshape hints with the samebraceArrays,allowBareString, andcontextrules assrc/explain.ts.package.json (1)
98-98: LGTM!docs/MATRIX.md (1)
74-74: LGTM!test/fixtures/explain/values.json (1)
273-311: LGTM!src/explain/values.ts (1)
12-32: LGTM!Also applies to: 61-124, 167-245, 276-304, 316-319
test/unit/explain-values.test.ts (1)
66-113: LGTM!Also applies to: 154-175, 271-279, 364-432, 434-518, 525-532, 541-629, 644-721, 842-845, 862-863
src/explain/args.ts (1)
134-160: LGTM!Also applies to: 173-238, 251-251, 355-355, 374-375, 546-546, 757-968, 980-1028
test/unit/explain-args.test.ts (1)
178-180: LGTM!Also applies to: 193-199, 217-223, 232-234
src/explain.ts (1)
96-96: LGTM!Also applies to: 459-471, 1110-1149, 1564-1566
commands/explain/README.md (1)
548-548: LGTM!Also applies to: 685-742
commands/explain/examples.md (1)
443-445: LGTM!Also applies to: 500-544, 545-568
test/unit/explain.test.ts (1)
361-364: LGTM!Also applies to: 413-424, 426-461
test/integration/explain-values.test.ts (1)
184-187: LGTM!Also applies to: 329-340, 414-627
test/integration/chr.ts (1)
288-293: LGTM!
- README census figures were stale: the fixture and its assertions were regenerated after the comma-spelling change but the prose was not. Now 19,709 / 13,168 / 6,541 / 570 / 814, matching `explain:value-census`. (Copilot ×2, CodeRabbit — the same real defect from three angles.) - `pushArrayMembers` returned a bare boolean, so every withdrawal published "an array literal with an empty member" even when the fault was a `*`/`+` lead. It now returns the reason, and the two causes are named separately. (Copilot.) - Deduplicate the interior fixture: three probe rows appeared twice because two rounds asked the same input. The builder keeps the first reading. (CodeRabbit.) - Declare `name` on the census `Emitted` interface and drop both casts, so `keyedElements` is type-checked rather than asserted. (CodeRabbit.) - Write census output with `Bun.write` instead of `console.log` before `process.exit`, which can truncate a piped stdout in Bun. (CodeRabbit.) - Hoist the duplicated 74-character input literal in example 28's assertion. (CodeRabbit.) - Fix the "Braces a command argument does take are…" fragment in both the README and the examples file. (Copilot ×2.) Verified: `bun run lint:ci`, `bun test` (2,435 pass), `bun run build`, and `test/integration/explain-values.test.ts` 3 pass / 0 fail on CHR 7.23.3. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Meta Muse Spark 1.2 - Review of PR #256 (value interior V1) — gaps & follow-upsReviewed 1. braceArrays gate is path-only, not verb-aware
Ask: enumerate 2. ARRAY_ARG_NAMES is deliberately tiny — needs a grounding note// src/explain/args.ts:206
const ARRAY_ARG_NAMES: ReadonlySet<string> = new Set(["in"]);Comment says "grounded and deliberately tiny". The grounding only cites 3. MAX_MEMBER_DEPTH = 8 is silent and undocumented
Fix: document the bound in 4. Nested invalid array not withdrawnTop-level
The parent Fix: when a member value starts with 5. {a=} / {a= } — silent abstention vs drop-
|
|
The comments from Muse Spark suggest :put {1=1} |
The post-#256 review said `{(1,)}` slips through, and it does: a `(…)` member is a GROUP as often as an array, so it fell to the fallback branch and abstained while the enclosing literal kept its `array` shape. `:parse` rejects the whole statement. Grounded on CHR 7.23.3 (`.scratch/explain-225-nested-literal-probe.ts`, 25 literals) rather than on `highlight`, which is the permissive gate here — it ACCEPTS `{1;2,}`, `{2,}` and `{(1,2),}`, all of which `:parse` rejects. Eight spellings were fabrications: `{(1,)}`, `{a=(1,)}`, `{1;(2,)}`, `{ (1,) ; 2 }`, `{()}`, `{a=()}`, `{(,)}` and `(1,(2,))`. The nine accepted controls (`{(1)}`, `{a=(1)}`, `{1;(2)}`, `{{}}`, `{1;{}}`, `{1;2;}`, `{{1;2};}`, `{1;2}`, `(1,2)`) still read. The probe rows are committed as `interiorGrounding.nestedLiterals`, which makes the previously dead `invalidLiterals` block's job an actual test: rejected literals are scored one-sided (abstaining is fine, claiming a shape is not). Corpus census is unchanged — this family has zero occurrences in all 948 scripts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Triaged all eight items against CHR 7.23.3 before answering, because two of the One item was a real, shipped fabrication and is fixed here (e0d70ce). Item 4 is half right. The other half of item 4 is not a bug:
Item 3 is already tested — Items 1, 2, 5, 6, 7 are real and filed, each with the device evidence:
Item 8's first bullet is a PR-body note, added. The census is unchanged by the The eight fabrications and the nine accepted controls are now committed as |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/explain-value-census.ts (1)
160-176: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winCount only anchors that the strict lexer compares.
Line 162 counts every
keptentry. Lines 163-164 skip everyelement. Array members therefore increasestrictComparableAnchorswithout any strict-token comparison. This makes the metric and its zero-contradiction claim inaccurate.Filter elements before incrementing and iterating. Regenerate the fixture counts and corpus prose after this correction.
Proposed fix
const strict = lexArguments(statement, split.argsAt); if (strict.read) { - result.strictComparableAnchors += kept.length; - for (const entry of kept) { - if (entry.kind === "element") continue; + const comparable = kept.filter((entry) => entry.kind !== "element"); + result.strictComparableAnchors += comparable.length; + for (const entry of comparable) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/explain-value-census.ts` around lines 160 - 176, Update the strict comparison block around lexArguments so strictComparableAnchors counts only non-element entries that are actually examined by the strict lexer. Filter out entries with kind "element" before incrementing the counter and iterating, while preserving the existing token and contradiction checks; then regenerate the fixture counts and corpus prose.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/explain/args.ts`:
- Around line 925-928: Update the depth-limit handling in the member-reading
function to withdraw the enclosing array shape when depth >= MAX_MEMBER_DEPTH
instead of returning null, which callers interpret as successfully reading all
members. Add and use the proposed DEPTH_BOUND_REACHED withdrawal reason
alongside the existing withdrawal constants, preserving null for non-cap paths
where it represents the established successful completion behavior.
---
Outside diff comments:
In `@scripts/explain-value-census.ts`:
- Around line 160-176: Update the strict comparison block around lexArguments so
strictComparableAnchors counts only non-element entries that are actually
examined by the strict lexer. Filter out entries with kind "element" before
incrementing the counter and iterating, while preserving the existing token and
contradiction checks; then regenerate the fixture counts and corpus prose.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a05d8fd1-66b9-4303-91e2-9b57d71b4f39
📒 Files selected for processing (7)
commands/explain/README.mdcommands/explain/examples.mdscripts/explain-value-census.tssrc/explain/args.tstest/fixtures/explain/values.jsontest/unit/explain-values.test.tstest/unit/explain.test.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (7)
commands/*/examples.md
📄 CodeRabbit inference engine (commands/AGENTS.md)
commands/<name>/examples.mdmust contain numbered, runnable examples, with example N corresponding to assertion N intest/integration/<name>.test.ts; these examples define whatCHR-passedis measured against.
Files:
commands/explain/examples.md
commands/*/{README.md,examples.md}
📄 CodeRabbit inference engine (AGENTS.md)
Read the target command's
README.mdandexamples.mdas the executable specification before writing code or tests.
Files:
commands/explain/examples.mdcommands/explain/README.md
{docs/**,.github/instructions/**,.github/**/*.yml,.github/**/*.yaml,**/*.{md,txt,dict}}
📄 CodeRabbit inference engine (AGENTS.md)
Run
bun run lint:ciwhen changing documentation, instructions, security configuration, spelling dictionaries, or workflow files.
Files:
commands/explain/examples.mdcommands/explain/README.md
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Transport or RouterOS-touching code is not done untilbun run test:integrationpasses.
Do not silently fall back to another protocol when the caller pinned--via.
Do not make generated output the hand-edited source of truth.
Do not disable validation to make a test pass; validation is part of the product.
Files:
test/unit/explain.test.tsscripts/explain-value-census.tstest/unit/explain-values.test.tssrc/explain/args.ts
commands/*/README.md
📄 CodeRabbit inference engine (commands/AGENTS.md)
commands/*/README.md: Eachcommands/<name>/README.mdis the executable specification's designed tier: document intent, flags, and behavior; include aDesigned, not implementedtable for spec-only flags; do not duplicate implemented-flag tables generated indocs/CLI.md; link todocs/CONSTITUTION.mdrather than restating constitution-wide rules; document only command-specific behavior; and keep theStatusline consistent withdocs/MATRIX.md.
Implemented flags must be generated fromCliCommandMetadataintodocs/CLI.mdviabun run docs:cli; command READMEs must not maintain duplicate tables of implemented flags.
Files:
commands/explain/README.md
test/fixtures/**/*
📄 CodeRabbit inference engine (test/AGENTS.md)
Keep fixtures under
test/fixtures/with clear source/provenance notes.
Files:
test/fixtures/explain/values.json
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (src/AGENTS.md)
src/**/*.{ts,tsx}: Use Bun-native TypeScript and Web APIs where possible
Errors must be actionable for humans and agents, with next-step guidance when a dependency, protocol, credential, or validation source is missing
Files:
src/explain/args.ts
🧠 Learnings (2)
📚 Learning: 2026-08-06T05:37:57.693Z
Learnt from: mobileskyfi
Repo: tikoci/centrs PR: 222
File: src/explain/defects.ts:1-70
Timestamp: 2026-08-06T05:37:57.693Z
Learning: In TypeScript modules under src/explain, retain references to phase-0 lab questions and issue numbers when they document the technical rationale for current implementation behavior or contracts. Remove comments containing dated decisions, delivery status, or future roadmap information when it does not explain current behavior, including in tests.
Applied to files:
src/explain/args.ts
📚 Learning: 2026-08-06T23:42:32.848Z
Learnt from: mobileskyfi
Repo: tikoci/centrs PR: 227
File: src/explain/args.ts:151-158
Timestamp: 2026-08-06T23:42:32.848Z
Learning: In the RouterOS explain lexer, use the shared `continuationLength(text, at)` helper for backslash-newline handling. Treat only `\n` and `\r\n` as continuations; a lone `\r` after a backslash must not be skipped. In `args.ts`, reject a lone-`\r` escape as invalid so token names and source spans remain accurate.
Applied to files:
src/explain/args.ts
🔇 Additional comments (8)
test/unit/explain.test.ts (1)
385-387: LGTM!src/explain/args.ts (3)
804-840: LGTM!
929-1005: LGTM!
1048-1061: LGTM!test/unit/explain-values.test.ts (1)
66-114: LGTM!Also applies to: 711-758
test/fixtures/explain/values.json (1)
894-1020: LGTM!commands/explain/README.md (1)
718-733: LGTM!commands/explain/examples.md (1)
501-568: LGTM!
…257) Closes the blocking review thread on #256: the `split.path === "/"` gate admitted `:delay {1;2}`, `:beep {1;2}` and `:resolve {1;2}` — CHR 7.23.3 syntax errors this PR was newly publishing member facts under — while refusing accepted slots like `:for from=` and `:local value=`. The gate is now keyed on (verb, slot), from a sweep of every root builtin (enumerated from the device, not by hand) and every slot it completes: 222 rows in `interiorGrounding.braceSlots`, baked into `src/explain/brace-slots.ts`, with a unit test asserting the table still equals the fixture's `array` rows. A slot has FOUR outcomes, and `{1;2}` cannot see the difference — `:local z {1;2}` and `:execute script={1;2}` both lower to `…=1;2`. `{(1,2)}` can: an evaluated slot lowers it to `(, 1 2)`, a script slot echoes `(1,2)` verbatim. Reading only `{1;2}` had put `:execute script=` in the array set, one step from hinting `array` on a script body. So: 72 array, 21 code, 7 text, 92 error, 30 unaskable — and only the 72 are read. Positionals are keyed by index, because `:local {1;2}` is the NAME slot and does not parse while `:local z {1;2}` does. Also from this round: - The member-descent bound withdraws instead of keeping an unverified shape (CodeRabbit). A `(1,)` buried nine levels deep is a syntax error the cap made invisible: `pass` + `array` on a statement `:parse` refuses. Deepest corpus member is 6, so this costs nothing seen. - `strictComparableAnchors` counts only the entries it compares (CodeRabbit, outside-diff). Measured no-op — a statement the strict lexer reads has 0 elements across all 948 scripts — but the counter now matches its published definition. Example 28c and its CHR assertion cover the accepted/rejected controls, the nested rejection, and the depth bound; `test:integration` for this file is 284 device assertions. Census moves by exactly one array (813, the `:execute {…}` script body), regenerated in the fixture, the assertions and the README prose. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
All four items from this round are fixed in The gate is now keyed on (verb, slot), fail closed. Root builtins were A slot has four outcomes, not three, and my first sweep got one wrong. The depth cap was a real fabrication and is fixed. CHR coverage added as example 28c: the accepted and rejected slot controls,
The census moved by exactly one array as a result of the |
Muse Spark 1.2 — Re-review of #256 @
|
…225) The re-review flagged that `braceSlotTakesArray` compares the verb verbatim while `verbsplit` keeps source casing, and suggested normalizing with `toLowerCase()`. The device says not to: `:LOCAL z {1;2}`, `:Local z {1;2}`, `:PUT {1;2}` and `/IP/DNS/set servers=1.1.1.1` all draw `expected command name (line 1 column 2)` on CHR 7.23.3. Lower-casing at the gate would open it on a statement that does not parse — the fabrication class this table exists to remove. So the behavior is already right and stays; what was missing is that it was an accident rather than a stated rule. Now it is written in the module header and in example 28c, with unit and CHR assertions on both halves: a differently-cased DIRECTIVE gets no array, while an argument's casing is free (`:local Z {1;2}` parses and binds `$Z`). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Thanks — nit (a) is worth having, but the suggested fix is the wrong way round, Directive casing. The observation is right — So What was genuinely missing is that this was an accident rather than a stated (b) census drift gate — tracked as #260, which also records why the (c) command-argument diagnostic — tracked as #259, still blocked on the |
Progresses #225 — the V1 interior slice. This does not close the issue: live
observedTypeandschemaTypeproducers remain sequenced with #236.What changed
Each member of a
{…}/(…)literal is now its ownvalues.occurrences[]row:kind: "element", its own document-byte span andshapeHints, aparentnaming the container's result-local id, and
namewhen the brace form spells akey. This is the expression/parser foundation #239 S2 and #255 are sequenced
behind — every gap closes by ADDING a field, none by changing one.
The grounding round overturned the premise twice
Both were hints already shipping since #242 that the device contradicts.
Neither was corpus-reachable enough to have shown up as a failure.
1. A
{…}array literal is a syntax error in a CLI command argument./console/inspectclasses the{byteerrorand:parserefuses thestatement for every row below, while the same bytes one position over parse:
{byte/ip/route/add comment={1;2}error/ip/dns/set servers={1.1.1.1;8.8.8.8}errorserversis list-typed, which rules out a schema-shaped explanation/interface/print .proplist={name;comment}errorip route add comment={1;2}error:log info message={1;2}error:local z {1;2}/:put {1;2}/:foreach i in={1;2}centrs returned
pass+arrayon all of the rejected ones, includingsource={ :return 1 }script bodies. A brace is now anchored only in a rootscripting directive's value slot (positional, or
in=). Note the acceptedbraces are per-attribute, not per-name:
:execute script={…}parses while/system/logging/action/add script={…}is a syntax error.2. There is no class-based wide IPv4 field.
values.tsallowed1.<=16777215/1.1.<=65535; every part is one octet::typeof1.255ip1.0.0.2551.256time00:00:01.2561.16777215time00:00:01.167772150, never1.255.255.2551.1.256str26 corpus literals — GPS coordinates like
54.3520, fractional-second logtimings like
00.000414— were being hintedip.A member is an expression, so it has its own lexicon
Inside a literal RouterOS parses an expression, which changes the answer for
whole classes of spelling:
{abc}lowers to$abc), and so isa full MAC (
{00:11:22:33:44:55}) and any time literal past the signed 64-bitnanosecond range — the cliff is exactly
15250w/15251w,106751d/106752d,9223372036s/9223372037s, in the colon spelling too;*1and+1are hard syntax errors there, though:local x *1isid;0x10is 16,0X10is a variable);;splits braces and,splits parens —{1,2}is one member, a nestedarray — and
(1;2)does not parse;=binds a key in braces ({a=1}) and COMPARES in parens ((a=1,b=2)is twoboolmembers);{;},{;1},{1;;2},(1,)) andwithdraws the enclosing
arrayshape with it;{1;}is legal.The bare comma spelling is the first genuinely plural hint
=1,2,3is a syntax error nowhere, and only the argument's TYPE decides whetherthe device splits it:
:parseILservers=1.1.1.1,8.8.8.8servers=1.1.1.1;8.8.8.8— a listdst-port=80,443dst-port=;80;443— a listcomment=a,bcomment=a,b— one stringinterface=ether1,ether2interface=ether1,ether2— one stringSo a named attribute carries
["array", "str"]— both readings, neithervalidated, which is what
shapeHints.valuesis a list for. A directive's valueslot has no text reading (
:local x 1,2is a two-member array), so the hint issingular there. No members are located for a bare comma run, because whether it
splits at all is a
schemaTypefact; the(1,2)spelling, where the delimitersprove it, is descended into as usual.
Census is re-derivable now
#242's corpus figures were published from a throwaway script, so when this slice
changed the emission there was no way to recompute them under the same
definitions — only to guess which definition produced each number. Added
bun run explain:value-census(scripts/explain-value-census.ts), and thefixture records the command beside the figures.
Evidence
test/fixtures/explain/values.json(
interiorGrounding), generated from the probe JSON rather than retyped, andscored one-sided: an abstention is always allowed, a contradicting type name
never. That test is what caught the IPv4 bug.
(529 keyed, 1,147 nested in another member). The four invariants are all 0 —
boundary contradictions, invalid spans, dangling parents, containment breaks.
bun run lint:cibun test— 2,435 pass, 0 failbun run buildbun run test:integration— 146 pass, 0 fail on CHR 7.23.3, including newexamples 28 and 28b
Not done here
observedType/schemaTypeproducers stay with explain phase 2: live probes — /console/inspect + :parse over rest-api and native-api #236.candidate diagnostic rather than today's silent abstention. Left out
deliberately: the braces a command argument does accept are schema-driven
per attribute (
:execute script={…}vs/system/logging/action/add script={…}), so an error-severity rule needs its own sweep first — explain: a brace array in a command argument is a device error, not a silent abstention #259.review shows the real rule is per (verb, slot) and that the path key is wrong
in both directions, with 0 corpus occurrences either way — explain: the brace-array gate is per (verb, slot), not per path #257.
{a=}is apositional
str;{$a=1}is aboolcomparison) — explain: array members the device has and centrs drops ({a=}, comparison members, key types) #258.valuesas not CHR-passed until explain/Q12: value-type axis — shape hints, observed types, schema types (three facts, three provenances) #225 closes;examples 28/28b are exercised by
test/integration/explain-values.test.ts.256.1is00:04:16.100) stays anabstention rather than a hint, since the reading depends on a failed address
attempt.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests