feat(explain): operator fill on the token partition seam (#290 B2) - #292
Merged
Conversation
The B1 partition (591) shipped a total, gapless [0,bytes) token stream behind --tokens with only comment + variable-* claimed. B2 fills holes from existing analyzers without touching the proof-only ExplainSpanClass / data.spans[] contract. Seam: split the vocabularies (ExplainTokenClass = ExplainSpanClass | "operator" | "unclassified", provisional until B5), factor residualRanges() out of buildTokens, and make buildTokens take an ordered list of fills — argument order IS fill order (#290 design decision 1). Cross-fill overlap is a hard throw; callers achieve impossibility by offering only residual. EV operators (e10) added, citedEvidence now seeds it when tokens carry that pass. Fill: src/explain/operator-tokens.ts scans the residual left by spans, longest-match over the grounded table (operators.ts + lowered aliases, minus re-lexed <>), word-bounded for and/or/in/ any, with dot traps (1. variable, 1.2 IP, (.1) time, .. second dot), slash second-of-//, and top-level conservatism for , / = - (depth>0 inside parens/brackets only, -> allowed everywhere). Quoted strings are skipped via shared scanQuotedString. Every emitted span is sorted, non-overlapping, fully inside residual, and carries class operator + ev e10. Census: 1,426,731 bytes, 27.44% → 28.62% classified (+16,833 operator bytes), 46,580 → 72,264 tokens (+13,907 operator runs), README block re-derived, both checks gated. Tests borrow the review lessons from 291: direct residual scanner traps, explainCommand masking, multi-fill overlap, and fill-order conservatism. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 56 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe explain token pipeline now computes residual ranges, fills them with operator tokens, validates multiple fills, and records operator provenance. The operator scanner handles literals, strings, boundaries, nesting, and punctuation cases. Tests and census documentation reflect the new token class. ChangesOperator token fill
Estimated code review effort: 4 (Complex) | ~45 minutes Mergeability Score: 🔵 Low · up to The PR adds operator classification and currently performs that scan even when token output is not requested, which can add unnecessary processing to explain requests; the documentation also contains a broken source link. The change is mergeable with explicit owner awareness or follow-up for these bounded issues. Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant ExplainCommand
participant BuildTokens
participant OperatorSpans
participant EvidenceCollection
ExplainCommand->>BuildTokens: existing spans and residual ranges
BuildTokens->>OperatorSpans: residual ranges
OperatorSpans-->>BuildTokens: operator tokens with e10
BuildTokens-->>ExplainCommand: combined token result
ExplainCommand->>EvidenceCollection: token provenance
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 1220-1223: Fix the `buildTokens` source link in the README to use
`../../src/explain.ts` instead of the path that traverses above the repository
root.
In `@src/explain.ts`:
- Around line 1129-1136: In the token-building flow around residualRanges,
operatorSpans, and buildTokens, only compute residual0 and opSpans when
options.tokens === true. Keep fills and buildTokens inside that branch, and
return undefined without scanning analyzed documents when tokens are not
requested.
🪄 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: df6d5868-2f02-46c6-be2c-d415488b3eb0
📒 Files selected for processing (7)
GLOSSARY.txtcommands/explain/README.mdsrc/explain.tssrc/explain/operator-tokens.tstest/fixtures/explain/tokens.jsontest/unit/explain-token-census.test.tstest/unit/operator-tokens.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (5)
- GitHub Check: CHR smoke (stable)
- GitHub Check: Corpus census
- GitHub Check: copilot-pull-request-reviewer
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (6)
{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:
GLOSSARY.txtcommands/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/tokens.json
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
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/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/explain/operator-tokens.tssrc/explain.ts
**/*.{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:
src/explain/operator-tokens.tstest/unit/operator-tokens.test.tstest/unit/explain-token-census.test.tssrc/explain.ts
🧠 Learnings (21)
📓 Common learnings
Learnt from: mobileskyfi
Repo: tikoci/centrs PR: 287
File: test/unit/explain-operators.test.ts:320-360
Timestamp: 2026-08-12T19:55:35.352Z
Learning: For the RouterOS operator sweep, `test/fixtures/explain/operators.json` stores captures from RouterOS 7.21.5 long-term, 7.23.3 stable, and 7.24rc4 testing. `scripts/explain-operator-slice.ts` compares every published sweep axis, including unary precedence, across these captures and emits `sweep.versionDifferences`. The `any` operator has no version differences in these captures.
📚 Learning: 2026-08-01T22:09:14.680Z
Learnt from: mobileskyfi
Repo: tikoci/centrs PR: 195
File: GLOSSARY.txt:548-561
Timestamp: 2026-08-01T22:09:14.680Z
Learning: In this repository, CSpell accepts a digit-suffixed token such as `toip6` when its alphabetic base token, `toip`, is present in `GLOSSARY.txt`. Do not add redundant digit-suffixed glossary entries solely for CSpell coverage.
Applied to files:
GLOSSARY.txt
📚 Learning: 2026-08-05T20:06:21.299Z
Learnt from: mobileskyfi
Repo: tikoci/centrs PR: 217
File: src/explain/symbols.ts:0-0
Timestamp: 2026-08-05T20:06:21.299Z
Learning: In `src/explain/symbols.ts`, comparative corpus measurements that explain a semantic modeling trade-off are rule provenance and follow the established F1/F2 documentation convention. Treat run-specific bookkeeping, fuzz counts, scratch-script paths, method pointers, and per-version missed-tail details as status data that should not remain in source comments.
Applied to files:
test/fixtures/explain/tokens.jsoncommands/explain/README.mdsrc/explain/operator-tokens.tstest/unit/operator-tokens.test.tstest/unit/explain-token-census.test.tssrc/explain.ts
📚 Learning: 2026-08-12T19:55:35.352Z
Learnt from: mobileskyfi
Repo: tikoci/centrs PR: 287
File: test/unit/explain-operators.test.ts:320-360
Timestamp: 2026-08-12T19:55:35.352Z
Learning: For the RouterOS operator sweep, `test/fixtures/explain/operators.json` stores captures from RouterOS 7.21.5 long-term, 7.23.3 stable, and 7.24rc4 testing. `scripts/explain-operator-slice.ts` compares every published sweep axis, including unary precedence, across these captures and emits `sweep.versionDifferences`. The `any` operator has no version differences in these captures.
Applied to files:
commands/explain/README.mdsrc/explain/operator-tokens.tstest/unit/operator-tokens.test.tstest/unit/explain-token-census.test.tssrc/explain.ts
📚 Learning: 2026-08-12T19:54:53.875Z
Learnt from: mobileskyfi
Repo: tikoci/centrs PR: 287
File: scripts/explain-operator-census.ts:429-454
Timestamp: 2026-08-12T19:54:53.875Z
Learning: In the centrs census commands, a successful fixture-drift check is valid only when the resolved corpus is the pinned corpus. `scripts/explain-operator-census.ts` rejects `--check` when `resolution.warning` is set. `scripts/explain-value-census.ts` has the same warning-only resolution shape but was deliberately left unchanged in PR `#287` to keep the RouterOS operator-surface work scoped to issue `#255`.
Applied to files:
commands/explain/README.mdsrc/explain/operator-tokens.tstest/unit/operator-tokens.test.tstest/unit/explain-token-census.test.tssrc/explain.ts
📚 Learning: 2026-08-06T14:17:50.227Z
Learnt from: mobileskyfi
Repo: tikoci/centrs PR: 224
File: commands/explain/examples.md:11-16
Timestamp: 2026-08-06T14:17:50.227Z
Learning: For the phase-1 offline explain API, offline behavior is tested through unit and fixture tests rather than CHR-backed integration tests. In `test/unit/explain-envelope.test.ts`, the CLI-spelling gate-versus-analysis behavior described by `commands/explain/examples.md` example `1b` and the resolved-menu behavior described by example `18b` have focused unit coverage. The CLI `explain` command surface is deferred and is not implemented as `src/cli/explain.ts` in this PR.
Applied to files:
commands/explain/README.mdsrc/explain/operator-tokens.tstest/unit/operator-tokens.test.tstest/unit/explain-token-census.test.tssrc/explain.ts
📚 Learning: 2026-08-01T23:46:54.392Z
Learnt from: mobileskyfi
Repo: tikoci/centrs PR: 197
File: src/explain/verbsplit.ts:383-387
Timestamp: 2026-08-01T23:46:54.392Z
Learning: In `src/explain/verbsplit.ts`, `VerbSplit` is the ratified Q6 boundary shape. Do not add document-scale context-certainty metadata to it independently; add that signal with the planned `ambiguous`/`unknown` verdict-vocabulary and phase-1 envelope work tracked in GitHub issue `#192`. Until then, `resolveVerbs` may correctly return root-based resolved paths for context-independent statements after context loss, but callers cannot observe that certainty was lost.
Applied to files:
commands/explain/README.mdsrc/explain.ts
📚 Learning: 2026-08-06T05:38:01.223Z
Learnt from: mobileskyfi
Repo: tikoci/centrs PR: 222
File: src/explain/defects.ts:1-70
Timestamp: 2026-08-06T05:38:01.223Z
Learning: In the TypeScript `src/explain` modules, references to phase-0 lab questions and issue numbers are an established documentation idiom when they explain the technical basis for a current contract. Keep these rationale references when they support implementation behavior. Remove dated decisions, delivery status, and future roadmap text from implementation and test comments.
Applied to files:
commands/explain/README.mdtest/unit/explain-token-census.test.tssrc/explain.ts
📚 Learning: 2026-08-06T05:37:57.342Z
Learnt from: mobileskyfi
Repo: tikoci/centrs PR: 222
File: src/explain/pathresolve.ts:895-901
Timestamp: 2026-08-06T05:37:57.342Z
Learning: In `src/explain/pathresolve.ts`, over-depth defects must use `regionIn(loc, start, end)` rather than `spanIn` when the scanned extent can be empty. `regionIn` widens empty bracket or block-body extents to the enclosing statement span, preserving the `Defect` contract that `end > start` without fabricating a precise byte location.
Applied to files:
src/explain/operator-tokens.tstest/unit/operator-tokens.test.tstest/unit/explain-token-census.test.tssrc/explain.ts
📚 Learning: 2026-08-09T00:35:47.021Z
Learnt from: mobileskyfi
Repo: tikoci/centrs PR: 241
File: src/explain/transport.ts:0-0
Timestamp: 2026-08-09T00:35:47.021Z
Learning: In `src/explain/transport.ts`, transport input arguments must use `ExplainArguments` from `src/explain.ts`, narrowed with `Extract<ExplainArguments, { read: true }>`. Do not substitute `ArgumentsRead` from `src/explain/args.ts`: it is a distinct lexer-level type that includes `args: Record<string, string>` and uses a different span representation, while `src/explain.ts` keeps command arguments separately in `command.args`.
Applied to files:
src/explain/operator-tokens.tstest/unit/explain-token-census.test.tssrc/explain.ts
📚 Learning: 2026-08-06T05:38:13.259Z
Learnt from: mobileskyfi
Repo: tikoci/centrs PR: 222
File: test/fixtures/explain/defects.json:3-73
Timestamp: 2026-08-06T05:38:13.259Z
Learning: In `src/explain/pathresolve.ts`, `Loc.base = -1` widening applies to defects raised during nested walker traversal, such as `over-depth`, when their local offsets cannot be mapped safely from UTF-16 string indexes to analyzed UTF-8 byte offsets. Defects emitted by the top-level segmenter are already in analyzed-document byte space and therefore retain precise offsets even when their enclosing statement contains non-ASCII text. The behavior is covered by `test/fixtures/explain/defects.json` and `test/unit/explain-defects.test.ts`.
Applied to files:
src/explain/operator-tokens.tstest/unit/operator-tokens.test.tstest/unit/explain-token-census.test.tssrc/explain.ts
📚 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/operator-tokens.ts
📚 Learning: 2026-08-05T03:19:54.280Z
Learnt from: mobileskyfi
Repo: tikoci/centrs PR: 209
File: test/unit/explain-menus.test.ts:146-152
Timestamp: 2026-08-05T03:19:54.280Z
Learning: In `test/unit/explain-write.test.ts`, keep separate test anchors for `isDanglingBarePath` and menu-table rejection. Known menu paths such as `/ip/firewall/filter` and `/log` reach `isDanglingBarePath` at document end, while `/system/reboot` is rejected earlier by `isConfirmedNav` because `isMenuPath` returns false. Do not combine these inputs in one test when asserting coverage of either mechanism.
Applied to files:
test/unit/operator-tokens.test.tstest/unit/explain-token-census.test.ts
📚 Learning: 2026-08-09T05:23:19.794Z
Learnt from: mobileskyfi
Repo: tikoci/centrs PR: 242
File: src/explain/values.ts:83-90
Timestamp: 2026-08-09T05:23:19.794Z
Learning: In `src/explain/values.ts`, RouterOS time literals are order-independent and additive on RouterOS 7.23.3 and 7.24rc3. The `isTimeShape` function must use full fragment coverage rather than enforce descending time-unit order or reject repeated units. Examples such as `1s1m`, `1m1m`, `1h1h1h`, and `1s1ms` are valid `time` values.
Applied to files:
test/unit/operator-tokens.test.tstest/unit/explain-token-census.test.ts
📚 Learning: 2026-07-03T21:35:09.559Z
Learnt from: CR
Repo: tikoci/centrs PR: 0
File: commands/api/AGENTS.md:0-0
Timestamp: 2026-07-03T21:35:09.559Z
Learning: Applies to commands/api/src/protocols/adapter.ts : Map REST `.query` words exactly to native `?` words without the `?` prefix, including eq, comparisons, has-property, absence, and stack operators; multiple words without an explicit stack op are implicitly AND-ed.
Applied to files:
test/unit/operator-tokens.test.ts
📚 Learning: 2026-08-09T00:35:51.834Z
Learnt from: mobileskyfi
Repo: tikoci/centrs PR: 241
File: src/explain/transport.ts:0-0
Timestamp: 2026-08-09T00:35:51.834Z
Learning: In `src/explain/transport.ts`, offline `print where` transport classification must fail closed unless each query expression is an attribute token with a valid property name and a non-empty decoded value. The runtime-exercised Q8 shape is `name=value`; bare property names, empty values, infix comparisons such as `address>1.1.1.1`, REST comparison words such as `>name=value`, and repeated `where` tokens must not produce a REST `.query` request.
Applied to files:
test/unit/operator-tokens.test.tssrc/explain.ts
📚 Learning: 2026-08-10T23:41:13.189Z
Learnt from: mobileskyfi
Repo: tikoci/centrs PR: 256
File: src/explain/args.ts:936-945
Timestamp: 2026-08-10T23:41:13.189Z
Learning: In `src/explain/args.ts`, `pushArrayMembers` must withdraw the enclosing array shape with `DEPTH_BOUND_REACHED` when `depth >= MAX_MEMBER_DEPTH`; returning `null` incorrectly reports the unverified literal as a valid array. CHR 7.23.3 accepts valid array nesting to at least depth 64, while invalid `(1,)` members remain syntax errors at all tested depths. The 948-script corpus has a maximum observed array-member depth of 6, so the depth-8 analysis bound does not drop observed valid corpus literals.
Applied to files:
test/unit/operator-tokens.test.ts
📚 Learning: 2026-08-06T21:57:06.372Z
Learnt from: mobileskyfi
Repo: tikoci/centrs PR: 226
File: test/integration/cli-smoke.test.ts:333-347
Timestamp: 2026-08-06T21:57:06.372Z
Learning: For the `centrs explain` CLI, ambient stdin is read only when no positional argument can provide the offline input. When a positional input is present and stdin is supplied, the explain result includes the `usage/stdin-ignored` warning. In `test/integration/cli-smoke.test.ts`, no-input coverage must test both child stdin shapes: no `stdin` option (`/dev/null`) and `stdin: ""` (an empty pipe).
Applied to files:
test/unit/operator-tokens.test.ts
📚 Learning: 2026-08-06T14:17:38.964Z
Learnt from: mobileskyfi
Repo: tikoci/centrs PR: 224
File: commands/explain/README.md:0-0
Timestamp: 2026-08-06T14:17:38.964Z
Learning: For the `explain` command documentation, `commands/explain/examples.md` specifies the finished target contract, not the current implementation phase. `commands/explain/README.md` contains the phase inventory and must map omitted phase fields to the issues and examples that will make them green. Future phase gaps must add fields without changing the finished example contract.
Applied to files:
test/unit/explain-token-census.test.ts
📚 Learning: 2026-06-11T06:18:36.306Z
Learnt from: mobileskyfi
Repo: tikoci/centrs PR: 13
File: commands/transfer/examples.md:5-9
Timestamp: 2026-06-11T06:18:36.306Z
Learning: In the `centrs` project (tikoci/centrs), `commands/transfer/examples.md` uses a multi-prefix example numbering scheme: `1-21` for core REST examples, `N*` for native-api, `S*` for SSH/SFTP, and `P*` for pending/gated examples. The test↔example mapping is NOT positional (not "example N → assertion N"). Instead, `test/integration/transfer.test.ts` annotates each test block with its example ID and records coverage via `recordIntegrationEvidence({ exampleIds })`. This is the intended, unambiguous mapping mechanism for this project.
Applied to files:
test/unit/explain-token-census.test.ts
📚 Learning: 2026-08-02T01:55:57.600Z
Learnt from: mobileskyfi
Repo: tikoci/centrs PR: 197
File: src/explain/pathresolve.ts:0-0
Timestamp: 2026-08-02T01:55:57.600Z
Learning: In `src/explain/pathresolve.ts`, `isUnreadablePath` identifies unreadable menu paths in both relative and absolute spelling. However, document-context poisoning applies only to unreadable `/`-led statements (`isUnreadableAbsolute`); relative unreadable paths do not poison document context by declared design.
Applied to files:
src/explain.ts
🔇 Additional comments (2)
src/explain.ts (1)
108-108: 🎯 Functional CorrectnessRun the required RouterOS integration suite.
Run
bun run test:integrationbefore merge. This changed RouterOS-touching TypeScript code is not complete until that command passes.As per coding guidelines:
**/*.{ts,tsx}: Transport or RouterOS-touching code is not done untilbun run test:integrationpasses.Source: Coding guidelines
commands/explain/README.md (1)
1209-1223: 📐 Maintainability & Code QualityRun the required documentation lint.
Run
bun run lint:ciafter updating this README.As per coding guidelines:
{docs/**,.github/instructions/**,.github/**/*.yml,.github/**/*.yaml,**/*.{md,txt,dict}}: Runbun run lint:ciwhen changing documentation, instructions, security configuration, spelling dictionaries, or workflow files.Source: Coding guidelines
There was a problem hiding this comment.
Pull request overview
Adds the first B2 “operator” fill to the --tokens gapless token partition for explain, consuming the grounded operator surface and enforcing explicit multi-fill ordering without expanding the proof-only spans[] vocabulary.
Changes:
- Introduces
src/explain/operator-tokens.tsto claim operator bytes on the residual (longest-match + trap rules) and citee10only when operator tokens exist. - Refactors token building to accept an ordered list of fills (
buildTokens(..., fills)) and factorsresidualRanges()out for reuse/testing. - Updates corpus census fixture + README docs and adds unit tests for operator traps, residual seam, and fill-order safety.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/explain.ts |
Adds ordered-fill token construction, exports residualRanges(), extends token class with operator, and wires operator fill + evidence citation. |
src/explain/operator-tokens.ts |
New residual-only operator scanner implementing longest-match, word-boundary, dot/slash traps, and depth conservatism. |
test/unit/operator-tokens.test.ts |
Adds focused tests for operator scanner traps and end-to-end behavior via explainCommand --tokens. |
test/unit/explain-token-census.test.ts |
Adds seam + fill-order tests for residualRanges() and multi-fill buildTokens. |
test/fixtures/explain/tokens.json |
Updates the pinned corpus census numbers and introduces operator counts/bytes. |
commands/explain/README.md |
Updates token-census prose to reflect the operator fill and the split between spans[] and tokens[]. |
GLOSSARY.txt |
Adds test-vocabulary tokens used by the operator-fill unit tests. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
#292) CodeRabbit: - commands/explain/README.md: `../..//../src/explain.ts` traversed above repo root — fix to `../../src/explain.ts` (also covers the duplicate Copilot thread on same line). - src/explain.ts: `residualRanges` + `operatorSpans` ran on every `explainCommand` even when `options.tokens` was not set and `tokens` was discarded. Gate the whole fill — residual + scan + buildTokens — on `options.tokens === true`; future B2 fills belong inside the same branch so the residual work is pay-for-what-you-ask. Copilot: - src/explain/operator-tokens.ts:112 comment said "single quote" while handling `"` — fix to "double-quoted string". - src/explain/operator-tokens.ts:202 hot-loop `[",","/","=","-"].includes` allocated a fresh array per spelling attempt — hoist to a module constant `TOP_LEVEL_CONSERVATIVE` Set. - src/explain/operator-tokens.ts:222 `out.sort` was redundant: `i` only moves forward so `out` is already by `start`. Return `out` directly and avoid O(n log n) on every corpus census scan. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…es (#290) Three false-positive classes the bot rounds did not reach, each grounded on the corpus device oracle (`parseil_results.il_text`) rather than argued. Together they were 4,489 of the fill's 13,907 corpus claims (32%). 1. `[` was counted as expression grouping alongside `(`, so the `, / = -` conservatism switched OFF inside command substitution — the one place it is most needed. Inside `[ … ]` the IL shows a COMMAND: `[ /system/identity/get value-name=name ]` is `(evl /system/identity/get value-name=name)` — path separators and an argument separator, no division and no comparison node. Measured on the corpus: 1,225 `/` (path roots, path separators, CIDR masks such as `dst-address=0.0.0.0/0`), 1,105 `=` (`to=json`, `output=user`, `bpm=150`), 907 `-` (`global-config-overlay`, `default-name`, `back-to-home`) and 231 `,` were claimed as operators. Replace the depth counter with a delimiter stack: only `(` opens an expression; `[` is a command and `{` a block or array literal. 2. A spelling glued immediately after an argument `=` is inside the VALUE — `in-interface-list=!LAN` (108), `admin-mac=*…`/`.id=*2` (49), `oid=.1.3.6.1.2.1` (17). 3. A spelling glued into an argument NAME is a name byte. `:foreach x in=$list` is `/foreach counter=$x` in the IL with no `(in …)` node anywhere — 234 of the 376 `in` claims were this. Corpus-wide, 121 scripts contain `in=` and exactly one has an `(in …)` node. Same for the dotted names: the IL keeps `security.authentication-types=wpa2-psk` as one name and renders `.id` as the single symbol `$.id`, never `(. …)` — 193 claims. The first abstention has a measured cost, stated rather than hidden: `find where name="x"` inside `[ … ]` DOES lower to a real `(= $name x)` node, so 224 of the 1,259 bracket-interior `=` bytes are genuine comparisons that now stay `unclassified`, against 1,035 that were plain `arg=value`. A `where`-aware fill can take them back later; claiming all 1,259 is 82% wrong. Under-claiming is the recoverable direction — `unclassified` is a first-class answer (#289 B1) and a later fill can still claim the byte, while an over-claim is permanent under fill order. Census re-derived through the ritual: `classifiedBytes` 408307 → 403516 (28.62% → 28.28%), `operator` 13907 → 9418 spans and 16833 → 12042 bytes. The PR still moves B1's number (27.44% → 28.28%) — it moves it by less, on bytes that are actually operators. `test/unit/operator-tokens.test.ts` asserted `[a,b]` → `[","]` with the comment "Inside brackets (command substitution)", so the defect was pinned as intended behavior; that row is now the opposite assertion plus per-rule coverage. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The worked example for the next B2 fill put `pathSpansOnResidual` *after* `opSpans`, which inverts #290 design decision 1 — the whole point is that `pathresolve.ts` has claimed the `/` and `args.ts` the `=` before the operator scanner runs, so it only ever sees bytes nobody else wanted. That ordering is also what lets the operator fill's three abstentions relax: they exist because the fills that own those bytes do not run yet. A fill inserted after the operator scanner would leave them permanently load-bearing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…]` end to end Two gaps in #290's own seam tests: - "buildTokens with ordered fills concatenates in argument order" asserted a throw and nothing about order. Fill order decides who may CLAIM a byte, not the emitted order — the partition is always sorted by `start`. Rename to what is true and assert it: the same two fills either way round produce the same stream, byte-ordered, with the trailing gap `unclassified` on `e1`. - The conservatism test covered `:put 1,2` vs `:put (1,2)` but never a bracket, which is where the fill was actually wrong. Add the `explainCommand`-level case: a whole `[ /ip/route/find where … ]` line yields zero operator tokens. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Author
|
@coderabbitai review |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #290. Bumps #289 B1 → B2 on #264.
591 proved the [0,bytes) partition behind
--tokenswith only comment + variable-* claimed (27.44%). This PR fills the first B2 hole — operator bytes — without widening the proof-onlyExplainSpanClasscontract (#289 scope: "not touched" +src/explain.tsheader).Seam —
#290amendment:ExplainTokenClasswasExplainSpanClass | "unclassified"andbuildTokens(analyzed, spans)had one claim source. Instead of adding another member toExplainSpanClass, split the vocabularies (ExplainTokenClass = ExplainSpanClass | "operator" | "unclassified", provisional until 264 B5), factorresidualRanges()out of the gap walk, and makebuildTokenstake an ordered list of fills — argument order is fill order (#290 design-decision 1). Cross-fill overlap is a hard throw (fail-closed); callers achieve impossibility by offering only residual. EV:e10operators,citedEvidenceseeds it when tokens carry that pass.spansvstokenssplit documented where enforced (explainCommandcall site +commands/explain/README.md).Fill —
src/explain/operator-tokens.tsconsumesoperators.ts(grounded 26 ops, 14 precedence, PR #287, no re-probe) on the residual only — longest-match, word-bounded (and/or/in/any), with dot traps (1.variable,1.2IP,(.1)time,..///second-byte) and slash second-of-//. SharesscanQuotedString, sorted/non-overlapping/fully-inside-residual invariant.Abstentions — a byte that is structurally path or argument is left for the later fills. Three rules, each grounded on the corpus device oracle (
parseil_results.il_text), added in4525be2after the first review round:, / = -outside a( )group —(opens an expression,[opens a command substitution and{a block/array literal[ /system/identity/get value-name=name ]is(evl /system/identity/get value-name=name)— path separators + an argument separator, no division and no comparison node=(the value)in-interface-list=!LAN,.id=*2,oid=.1.3.6.1.2.1:foreach x in=$listis/foreach counter=$xwith no(in …)node;security.authentication-types=wpa2-pskstays one name;.idrenders as the single symbol$.id, never(. …)The first rule has a measured cost, stated rather than hidden:
find where name="x"inside[ … ]does lower to a real(= $name x)node, so 224 of the corpus's 1,259 bracket-interior=bytes are genuine comparisons that now stayunclassified— against 1,035 that were plainarg=value. Awhere-aware fill can take them back later; claiming all 1,259 is 82% wrong. Under-claiming is the recoverable direction:unclassifiedis a first-class answer (#289 B1), while an over-claim is permanent under fill order.Census — re-derived ritual (
explain:token-census → explain:token-census:readme → explain-token-census.test.tspin): 1,426,731 bytes,classifiedBytes391474 → 403516 (27.44% → 28.28%),totalTokens46580 → 63416,operator9418 spans / 12042 bytes.commands/explain/README.mdGENERATED block + prose paragraph updated; both checks plusexplain:operator-readme:checkgreen.Tests borrow 291 review lessons —
operator-tokens.test.tsper-trap,explain-token-census.test.tsseam+fill-order.Two-hop note: bytes where operator abstains at top-level stay
unclassifiednow and later becomepath/arg— don't misread the later B2 diff as stealing.Co-Authored-By: Claude Fable 5 noreply@anthropic.com
Summary by CodeRabbit
New Features
--tokensoutput.xand,andx, andalnum.Bug Fixes
Documentation