feat(explain): total, gapless token partition behind --tokens (#289 B1) - #291
Conversation
Implements stage B1 of #264: a new data.tokens[] facet behind --tokens, a total partition of [0, input.bytes) where every byte belongs to exactly one token (sorted, no gaps, no overlaps, join(slice)===input). Byte-exact on analyzed text so it inherits #215/#252 and positionMap applies when normalized. No new classification — reuses existing analyzers (comments + variable occurrences); everything else is unclassified (first-class, ev e0 provisional until B5). data.spans[] untouched so nothing shipped changes without the flag, matching --complete/--schema/--curl pattern. Deliverable is a number: 27.44% of corpus bytes classified (391,474 / 1,426,731 over 948 scripts, 46,580 tokens, avg 49.1/script), reported as a generated block in commands/explain/README.md via test/fixtures/explain/tokens.json → corpus. Census is re-derivable with bun run explain:token-census and drift-gated at both links: - corpus → fixture via bun run explain:token-census:check (CI corpus job) - fixture → README via bun run explain:token-census:readme:check (lint:ci) Each B2 fill (path/menu, arg name/=, value/array, quotes/escapes, operators) will move the classified percentage. Co-Authored-By: Claude Fable 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 PR adds opt-in gapless token partitions to ChangesToken output and census
Estimated code review effort: 4 (Complex) | ~45 minutes Mergeability Score: 🟡 Moderate · up to The PR adds token partition output and corpus drift validation, but malformed ranges can still produce incorrect token reconstruction and check mode can accept an unpinned corpus with matching aggregate metrics. These bounded correctness issues should be fixed before merge. Sequence Diagram(s)sequenceDiagram
participant CLI
participant explainCommand
participant buildTokens
participant Census
participant SQLite
CLI->>explainCommand: run explain --tokens
explainCommand->>buildTokens: analyze input spans
buildTokens-->>explainCommand: return gapless data.tokens[]
Census->>SQLite: load corpus scripts
Census->>explainCommand: request token analysis
explainCommand-->>Census: return token partitions
Census-->>Census: compare fixture and README metrics
Possibly related issues
Possibly related PRs
🚥 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: 3
🤖 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 `@scripts/explain-token-census.ts`:
- Around line 293-297: Update the check-mode flow around resolution.warning so
that when args.includes("--check") and resolution.warning is present, it logs
the warning and returns 1 before opening the database or comparing the fixture;
preserve the existing warning behavior outside check mode.
- Around line 86-104: Update the token validation loop in the explain-token
census checks to validate every token’s start and end offsets before
reconstruction: require integer values and 0 <= start < end <= bytes. Record an
invariant failure for any invalid range and ensure invalid tokens cannot reach
the analyzed.slice reconstruction.
In `@src/explain.ts`:
- Around line 842-846: Update the span validation in buildTokens to reject
ranges where s.end is less than s.start, in addition to the existing bounds
checks. Perform this validation before updating prev so reversed spans cannot
produce invalid token ranges.
🪄 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: bfafd3aa-584a-436b-9911-950b5cca1a2e
📒 Files selected for processing (10)
.github/workflows/ci.yamlGLOSSARY.txtcommands/explain/README.mddocs/CLI.mdpackage.jsonscripts/explain-token-census.tssrc/cli/explain.tssrc/explain.tstest/fixtures/explain/tokens.jsontest/unit/explain-token-census.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: CHR smoke (stable)
- GitHub Check: copilot-pull-request-reviewer
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (6)
test/fixtures/**/*
📄 CodeRabbit inference engine (test/AGENTS.md)
Keep fixtures under
test/fixtures/with clear source/provenance notes.
Files:
test/fixtures/explain/tokens.json
{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.txtdocs/CLI.mdcommands/explain/README.md.github/workflows/ci.yaml
**/*.{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-token-census.test.tssrc/cli/explain.tsscripts/explain-token-census.tssrc/explain.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
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/cli/explain.tssrc/explain.ts
🧠 Learnings (27)
📓 Common learnings
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.
Learnt from: mobileskyfi
Repo: tikoci/centrs PR: 273
File: test/fixtures/explain/corpus-partition.json:4-4
Timestamp: 2026-08-11T17:46:27.539Z
Learning: For durable fixtures in this repository, do not cite in-flight `.scratch/` files as provenance. In `test/fixtures/explain/corpus-partition.json`, the committed groups are the durable artifact, and the `frozen` no-regeneration statement replaces the former `.scratch/explain-lab-partition.ts` generator citation.
Learnt from: mobileskyfi
Repo: tikoci/centrs PR: 287
File: scripts/explain-operator-census.ts:429-454
Timestamp: 2026-08-12T19:54:50.443Z
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`.
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.
📚 Learning: 2026-08-11T17:46:27.539Z
Learnt from: mobileskyfi
Repo: tikoci/centrs PR: 273
File: test/fixtures/explain/corpus-partition.json:4-4
Timestamp: 2026-08-11T17:46:27.539Z
Learning: For durable fixtures in this repository, do not cite in-flight `.scratch/` files as provenance. In `test/fixtures/explain/corpus-partition.json`, the committed groups are the durable artifact, and the `frozen` no-regeneration statement replaces the former `.scratch/explain-lab-partition.ts` generator citation.
Applied to files:
test/fixtures/explain/tokens.jsontest/unit/explain-token-census.test.ts.github/workflows/ci.yamlscripts/explain-token-census.ts
📚 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.jsontest/unit/explain-token-census.test.tscommands/explain/README.md.github/workflows/ci.yamlsrc/cli/explain.tsscripts/explain-token-census.tssrc/explain.ts
📚 Learning: 2026-08-07T16:47:13.907Z
Learnt from: CR
Repo: tikoci/centrs PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-08-07T16:47:13.907Z
Learning: Applies to {docs/**,.github/instructions/**,.github/**/*.yml,.github/**/*.yaml,**/*.{md,txt,dict}} : Run `bun run lint:ci` when changing documentation, instructions, security configuration, spelling dictionaries, or workflow files.
Applied to files:
package.json
📚 Learning: 2026-08-07T16:47:13.907Z
Learnt from: CR
Repo: tikoci/centrs PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-08-07T16:47:13.907Z
Learning: Run `bun run lint && bun run test && bun run build` before finishing code changes.
Applied to files:
package.json
📚 Learning: 2026-08-08T00:38:33.949Z
Learnt from: mobileskyfi
Repo: tikoci/centrs PR: 234
File: .claude/settings.json:0-0
Timestamp: 2026-08-08T00:38:33.949Z
Learning: In `.claude/settings.json`, the `Bash(biome *)`, `Bash(tsc *)`, `Bash(cspell *)`, `Bash(markdownlint-cli2 *)`, `Bash(secretlint *)`, and `Bash(shellcheck *)` permissions intentionally allow valid lint command variants. `Bash(bun *)` already permits the repository’s `bun run lint*` commands. Do not request narrowing only these direct lint-tool permissions unless the broader Bun permission model also changes.
Applied to files:
package.json
📚 Learning: 2026-08-12T19:54:50.443Z
Learnt from: mobileskyfi
Repo: tikoci/centrs PR: 287
File: scripts/explain-operator-census.ts:429-454
Timestamp: 2026-08-12T19:54:50.443Z
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:
package.jsontest/unit/explain-token-census.test.tscommands/explain/README.md.github/workflows/ci.yamlscripts/explain-token-census.ts
📚 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-09T05:13:01.339Z
Learnt from: mobileskyfi
Repo: tikoci/centrs PR: 0
File: :0-0
Timestamp: 2026-08-09T05:13:01.339Z
Learning: RouterOS `.` is an expression concatenation operator with array-distributing semantics. In the current offline value-shape phase, the strict token reader refuses parenthesized, braced, and array forms before `valueShapeHints` runs. Future `observedType` and expression parsing must not treat `a.b` or `1.2` as a single atomic literal without expression-aware parsing.
Applied to files:
GLOSSARY.txt
📚 Learning: 2026-08-11T13:12:33.293Z
Learnt from: mobileskyfi
Repo: tikoci/centrs PR: 268
File: src/explain/args.ts:0-0
Timestamp: 2026-08-11T13:12:33.293Z
Learning: In `src/explain/args.ts`, RouterOS brace-array member keys use the measured grammar `^[A-Za-z0-9./-]+$`: `.`, `-`, and `/` are valid anywhere, including alone or repeated. Examples include `{.=1}`, `{..id=1}`, `{-=1}`, `{--=1}`, `{/=1}`, and `{a/b=1}`. `_` is not valid in this key grammar: `{_a=1}` and `{a_b=1}` are comparisons. Key binding still requires the key to touch `=`; `{.id =1}` is a `bool` comparison, while `{.id=1}` binds key `.id`. When a spelling is not recognized as a key, `pushArrayMembers()` must abstain unless `NOT_IN_MEMBER_NAME` positively proves an expression byte.
Applied to files:
GLOSSARY.txt
📚 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:
docs/CLI.mdcommands/explain/README.md
📚 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:
docs/CLI.mdtest/unit/explain-token-census.test.ts.github/workflows/ci.yamlsrc/cli/explain.tsscripts/explain-token-census.tssrc/explain.ts
📚 Learning: 2026-07-23T00:15:08.643Z
Learnt from: CR
Repo: tikoci/centrs PR: 0
File: commands/AGENTS.md:0-0
Timestamp: 2026-07-23T00:15:08.643Z
Learning: Unknown verbs, sub-verbs, and flags must produce an invalid-command or usage error listing the closest canonical matches using Levenshtein distance, along with accepted aliases.
Applied to files:
docs/CLI.md
📚 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:
test/unit/explain-token-census.test.tssrc/explain.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/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-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/explain-token-census.test.ts
📚 Learning: 2026-08-12T19:55:32.402Z
Learnt from: mobileskyfi
Repo: tikoci/centrs PR: 287
File: test/unit/explain-operators.test.ts:320-360
Timestamp: 2026-08-12T19:55:32.402Z
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:
test/unit/explain-token-census.test.tscommands/explain/README.md
📚 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/explain-token-census.test.tscommands/explain/README.mdsrc/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:
test/unit/explain-token-census.test.ts
📚 Learning: 2026-08-12T21:33:50.821Z
Learnt from: CR
Repo: tikoci/centrs PR: 0
File: scripts/probes/AGENTS.md:0-0
Timestamp: 2026-08-12T21:33:50.821Z
Learning: Applies to scripts/probes/**/*.ts : **`highlight` and `:parse` are different oracles and disagree.** `highlight`
accepts `{1;2,}`, `{2,}` and `{(1,2),}`, all of which `:parse` rejects. Say
which one a row was scored on.
Applied to files:
commands/explain/README.md
📚 Learning: 2026-08-07T16:47:13.907Z
Learnt from: CR
Repo: tikoci/centrs PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-08-07T16:47:13.907Z
Learning: A feature is done only when its examples pass on real CHR via `bun run test:integration`; existing code and passing unit tests alone are insufficient.
Applied to files:
.github/workflows/ci.yaml
📚 Learning: 2026-06-16T10:51:07.659Z
Learnt from: CR
Repo: tikoci/centrs PR: 0
File: test/AGENTS.md:0-0
Timestamp: 2026-06-16T10:51:07.659Z
Learning: Applies to test/**/cli-smoke.test.ts : Network-free CLI smoke tests (`cli-smoke.test.ts`) should not be CHR-gated and should run in the fast push/PR gate (`bun test`).
Applied to files:
.github/workflows/ci.yaml
📚 Learning: 2026-08-11T03:49:52.355Z
Learnt from: mobileskyfi
Repo: tikoci/centrs PR: 265
File: .github/workflows/ci.yaml:171-172
Timestamp: 2026-08-11T03:49:52.355Z
Learning: In this repository's GitHub Actions workflows, SHA-pin `oven-sh/setup-bun` and `actions/setup-node` references and include version comments. Other first-party `actions/*` references may continue using floating major-version tags according to the repository's current convention. Do not request SHA pinning for those first-party actions unless a repository-wide maintainer policy and corresponding Dependabot update policy have been established.
Applied to files:
.github/workflows/ci.yaml
📚 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/cli/explain.tssrc/explain.ts
📚 Learning: 2026-08-12T21:33:50.821Z
Learnt from: CR
Repo: tikoci/centrs PR: 0
File: scripts/probes/AGENTS.md:0-0
Timestamp: 2026-08-12T21:33:50.821Z
Learning: Applies to scripts/probes/**/*.ts : **Resolve the corpus through `../corpus-fetch.ts`.** A hardcoded
`$HOME/GitHub/lsp-routeros-ts` path is the reachability bug `#186` exists to
fix, and the resolver announces which snapshot's bytes were measured.
Applied to files:
scripts/explain-token-census.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:
src/explain.ts
🔇 Additional comments (11)
src/explain.ts (2)
409-433: LGTM!Also applies to: 549-559, 897-904, 1464-1467, 1522-1528
1050-1052: 🎯 Functional CorrectnessRun the required integration test.
No passing
bun run test:integrationresult is included for this RouterOS-touching TypeScript path. Run the command and resolve failures before merge.
src/explain.ts#L1050-L1052: validate token-enabled explanation output.src/cli/explain.ts#L477-L480: validate--tokensCLI propagation.As per coding guidelines:
**/*.{ts,tsx}: Transport or RouterOS-touching code is not done untilbun run test:integrationpasses.Source: Coding guidelines
src/cli/explain.ts (1)
92-96: LGTM!Also applies to: 115-115, 171-173
test/fixtures/explain/tokens.json (1)
1-28: LGTM!test/unit/explain-token-census.test.ts (1)
50-176: LGTM!commands/explain/README.md (1)
1202-1217: LGTM!docs/CLI.md (1)
226-226: LGTM!GLOSSARY.txt (1)
702-704: LGTM!package.json (2)
67-67: 📐 Maintainability & Code QualityRun the required documentation validation.
Run
bun run lint:ciand provide the result before merge. This change includes documentation, glossary, and workflow files.As per coding guidelines, “Run
bun run lint:ciwhen changing documentation, instructions, security configuration, spelling dictionaries, or workflow files.”Source: Coding guidelines
118-121: LGTM!.github/workflows/ci.yaml (1)
161-171: LGTM!Also applies to: 224-239
There was a problem hiding this comment.
Pull request overview
Adds an opt-in --tokens facet to centrs explain that emits data.tokens[] as a total, gapless byte partition of the analyzed input, and introduces a drift-gated token census (fixture + generated README block) wired into CI.
Changes:
- Add
--tokensfacet andExplainTokentypes; build a gapless token partition from existingspans[]plusunclassifiedgaps. - Add corpus-driven token census tooling (
scripts/explain-token-census.ts), a pinned fixture (test/fixtures/explain/tokens.json), and unit tests to drift-gate README rendering. - Wire the census into CI (
ci.yamlcorpus job +lint:ciREADME projection gate) and update glossary/CLI docs.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/explain.ts |
Introduces ExplainToken/data.tokens[], buildTokens(), and threads the facet through explainCommand/explainEnvelope. |
src/cli/explain.ts |
Adds --tokens flag parsing and passes the facet into the envelope. |
scripts/explain-token-census.ts |
New script to compute and gate token census + README block rendering from fixture/corpus. |
test/unit/explain-token-census.test.ts |
Adds invariant tests for the token partition and drift gates for README block rendering. |
test/fixtures/explain/tokens.json |
Pins the token census results for the corpus (the “deliverable number”). |
commands/explain/README.md |
Adds a generated token-census block and updates the narrative around tokens/operators. |
docs/CLI.md |
Documents the new --tokens flag in generated CLI docs. |
package.json |
Adds token-census scripts and adds the README-projection gate to lint:ci. |
GLOSSARY.txt |
Adds project vocabulary entries for “gapless”, “SCIP”, and “unclassified”. |
.github/workflows/ci.yaml |
Extends the corpus census job summary and adds a token-census fixture drift gate. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…k corpus - src/explain.ts: validate buildTokens spans before overlap — bounds, non-empty (end <= start rejected), and integer checks first so a negative start no longer masquerades as "overlapping" - scripts/explain-token-census.ts: validate every token range (integer, 0 <= start < end <= bytes) before gap/overlap and slice reconstruction, so negative offsets can not slip through slice() - scripts/explain-token-census.ts: refuse --check when resolution.warning is set (pinned-corpus gate, matching explain-operator-census) - test/unit/explain-token-census.test.ts: make the unclassified assertion stable across B2 fills (whitespace-only input + fixture counter) instead of pinning a specific command that will become fully classified Addresses CodeRabbit #3771168524, #3771168526, #3771168531 and Copilot #3771177335, #3771177366 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Fixed all 5 review threads in 1d28622:
Verified |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (2)
scripts/explain-token-census.ts:135
- When the token-partition invariant fails, the thrown error does not identify which script caused the failure (or show any snippet/ID). In CI this will be hard to debug because messages like “gap/overlap at 17” aren’t actionable without knowing the input that triggered it.
if (invariantFailures.length > 0) {
throw new Error(
`token partition invariant violated: ${invariantFailures.slice(0, 5).join("; ")}`,
);
scripts/explain-token-census.ts:256
- The
classifiedPctdrift check comment says “tolerance to 2 decimal places”, but the actual tolerance is0.0001(1e-4). This mismatch makes the drift-gate behavior harder to understand/maintain.
// classifiedPct is derived; compare with tolerance to 2 decimal places.
Three findings from an independent review of #291, all in B1's own scope: - `--tokens` was a silent no-op in the DEFAULT text format: output was byte-identical with and without the flag, and only `--json`/`--yaml` carried the partition. Every other opt-in flag changes the text surface (`--curl` adds a `curl:` line; `--complete`/`--schema` emit a tip). Text now gains a `tokens:` section whose header is the #289 deliverable — the per-input coverage number — followed by one row per token, `unclassified` runs included. - An `unclassified` token cited `e0` (`canonicalizeExecuteCommand`). The pass that produced an UNCLAIMED byte is the one that produced the analyzed surface its offsets are on: `e1` (`analyzeCoordinates`). `EV` is documented as "the pass that actually produced the fact"; both ids are always cited, so this is attribution, not a dangling reference. - `buildTokens` is exported and was referenced by no test. The three guards added in 1d28622 (non-integer, out-of-bounds/empty/reversed, overlap) are unreachable from `explainCommand` — 40k fuzzed inputs never trip them — so nothing went red if one were deleted. Added direct tests for each branch plus the unsorted-but-disjoint and empty-input paths; mutation-checked that the overlap test fails when the guard is stubbed out. Also wraps the README paragraph after the generated block and records the B2 constraint found while reviewing: `data.tokens[]`'s only fill source is `data.spans[]`, which is the proof-only facet, so the operator fill (#290) needs a claim seam rather than a new `spans[]` class. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`ExplainSpan` and `ExplainSpanClass` are public; their token counterparts
were not, so a library consumer calling `explainCommand(input, { tokens:
true })` could reach `data.tokens` but could not name its element type.
`src/AGENTS.md`: the TypeScript API is the root contract.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Independent review — 4 findings, all fixed on this branchChecks were green and three bot rounds had landed, so I looked for what a 1.
|
Both items Copilot SUPPRESSED on its second pass, taken on merit — a suppressed finding is still a finding. - `gap/overlap at 17` was unactionable in CI: the corpus is not in this repo, so with no row index and no snippet there was nothing to go look at. Failures now read `script #1 ("/ip address add address=1.1.1.1 # a very long trailing comme"…): gap/overlap at token 1: [0,1) then [0,103)`. Verified by injecting a partition break and reading the message the CI job would print. - The `classifiedPct` comment claimed a "tolerance to 2 decimal places" while the code uses 1e-4. The epsilon is not a drift allowance at all: `classifiedPct` is derived from `classifiedBytes` / `totalBytes`, both already compared exactly, so it only absorbs float round-trip noise through JSON — and must stay far tighter than the 2 decimal places the README renders. Comment now says that. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Addendum — Copilot's two suppressed comments, taken on merit (
|
Closes #289 — stage B1 of #264.
Every byte of the analyzed input now belongs to exactly one token —
data.tokens[]behind--tokens(matching--complete/--schema/--curl), sorted bystart, no gaps, no overlaps,join(slice)===input. Byte-exact on analyzed text so it inherits #215/#252;input.positionMapapplies when normalized.classis explicitly provisional until #264 B5.No new classification: reuses existing analyzers (comments + decided symbol occurrences); everything else is
unclassified(ev: e0provisional, B2 fills carry the claiming pass).data.spans[]untouched so nothing shipped changes without the flag.Deliverable is a number — 27.44% of corpus bytes classified (391,474 / 1,426,731 over 948 scripts, 46,580 tokens, avg 49.1/script), reported as a generated block in
commands/explain/README.mdviatest/fixtures/explain/tokens.json→corpus. Two drift gates:corpus → fixtureviabun run explain:token-census:check(CIcorpusjob, needs DB)fixture → READMEviabun run explain:token-census:readme:check(lint:ci, offline)Each B2 fill (path/menu, arg name/=, value/array, quotes/escapes, operators #290) will move the classified percentage.
Review fixes in this PR: wires
ci.yamlcorpus job, removes deadout.length===0branch inbuildTokens, annotates provisionale0.Verification:
bun run lint:typecheck/biome/cspell 0pass,bun test test/unit/explain-token-census.test.ts17 pass, manual invariant probe over representative inputs green.Summary by CodeRabbit
New Features
--tokensflag to theexplaincommand.unclassifiedranges for uncovered bytes.Documentation
Quality Improvements