CLI-Reference overlay: CI publish wiring + close V-cliref-link-drift / V-cliref-db-integrity gaps (#127)#128
Conversation
Wire the overlay into published builds and close the two GAP validation rows (V-cliref-link-drift, V-cliref-db-integrity) from #124. - release.yml: run extract-cliref + link-cliref (link after "Extract command tree" populates schema_nodes), so published DBs carry cliref_* + the crosswalk. - V-cliref-link-drift: `link-cliref --check` re-links the built DB and byte-compares the exact/alias/manual-only crosswalk against a committed, reviewable cli-reference-links.tsv (24 alias + 120 manual-only rows, counts header), and asserts every alias names a KNOWN_ALIAS_SEGMENTS member. `--write-baseline` / `make link-cliref-baseline` regenerates it. - V-cliref-db-integrity: new src/check-cliref-db.ts — integrity/FK/orphans/ enums/occurrence-order uniqueness + the settable-only field-view invariant. - qa.yml db-content: run both gates against the built DB (local-build/artifact), and extend the content floors with cliref_entries/fields/links. - VALIDATION.md: flip both rows GAP -> blocking; V-cliref-parse-reconcile is now release-wired. release.test.ts anchors the new steps/floors. Also fix a real parser bug the #124 fail-loud surfaced against the live corpus: <ArgTableRow> attributes were truncated at a '>' inside a quoted value (e.g. typ="iface_enum { <l2tp>:0xfffffffe }" on interface/pppoe-server), storing an empty raw_type. The tag matcher now skips whole quoted values. Verified against a full local build (59 restraml versions + live cliref): 228 pages / 1051 entries / 10118 fields / 948 flags; 907 exact / 24 alias / 120 manual-only; --check passes and exits 1 on drift; integrity gate clean. Co-Authored-By: Claude Opus 4.8 <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:
📝 WalkthroughWalkthroughRelease builds now populate the CLI-reference overlay and inspection crosswalk. New baseline and database-integrity checks run through QA gates, with workflow tests, parser regression coverage, documentation, and validation-matrix updates. ChangesCLI-Reference overlay hardening
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
link-cliref fails fast on an empty schema_nodes, but the command tree is only populated on the full-versions path — a single-version build (make extract / non-dispatch release trigger) runs extract-commands, not extract-all-versions. Guard so the link + drift gate run iff a crosswalk can form: - release.yml: link-cliref runs only when schema_nodes has dir/cmd nodes. - qa.yml: the CLIREF_LINKS floor and link-cliref-check skip when the tree is absent (e.g. the default local-build rehearsal). cliref-db-check still runs — it's valid with an empty crosswalk (the field view yields zero rows). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Wires the existing CLI-Reference overlay pipeline into CI-published DB builds and promotes the previously deferred CLI-Reference validation gaps into blocking, reproducible QA gates (plus fixes a live-corpus parser edge case).
Changes:
- Adds
extract-cliref+link-clirefto the release build pipeline and extendsqa.ymlDB-content floors to require populatedcliref_*tables. - Introduces a committed crosswalk baseline (
cli-reference-links.tsv) andlink-cliref --checkdrift gate (V-cliref-link-drift) enforced inqa.yml. - Adds
src/check-cliref-db.tsstructural/semantic full-DB integrity gate (V-cliref-db-integrity) and fixes<ArgTableRow>attribute parsing when a quoted value contains a literal>.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
VALIDATION.md |
Flips V-cliref-link-drift / V-cliref-db-integrity from GAP to blocking and updates V-cliref-parse-reconcile to reflect CI wiring. |
src/release.test.ts |
Asserts release.yml and qa.yml include the new CLI-Reference wiring, floors, and gates (order + presence checks). |
src/link-cliref.ts |
Adds baseline TSV generation + --check mode for drift detection and alias allowlist auditing. |
src/link-cliref.test.ts |
Adds unit coverage for baseline generation and alias-segment auditing (using the db.ts singleton). |
src/extract-cliref.ts |
Fixes <ArgTableRow> parsing to avoid truncating attributes when > appears inside quoted attribute values. |
src/extract-cliref.test.ts |
Adds regression test for the >-inside-quoted-attribute parser bug. |
src/check-cliref-db.ts |
New full-DB CLI-Reference integrity gate script (PRAGMA checks, orphans, enums, uniqueness, view semantics). |
MANUAL.md |
Documents that release builds now include the overlay and that qa.yml runs the two new blocking CLI-Reference gates. |
Makefile |
Adds targets for baseline regeneration/check and the full-DB integrity gate (link-cliref-baseline, link-cliref-check, cliref-db-check). |
cli-reference-links.tsv |
New committed baseline crosswalk (alias + manual-only rows + pinned counts header). |
CHANGELOG.md |
Records user-visible release/QA behavior change (overlay now shipped + CI gates) and the parser fix. |
.github/workflows/release.yml |
Adds Extract CLI-Reference overlay and Link CLI-Reference to inspect steps in the build job. |
.github/workflows/qa.yml |
Extends DB-content floors for cliref_* presence and adds V-cliref-link-drift / V-cliref-db-integrity steps. |
- release.yml comment "iff" → "only when" (cspell dictionary). - link-cliref.test.ts header no longer claims the file never touches the db.ts singleton — the baseline tests seed it (and clean up in afterAll). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 @.github/workflows/qa.yml:
- Around line 236-239: Add a cliref_flags row count alongside CLIREF_ENTRIES,
CLIREF_FIELDS, and CLIREF_LINKS, define and document an appropriate minimum
floor, and enforce that floor in the QA content gate. Include the cliref_flags
count in the database summary output while preserving the existing checks and
failure behavior.
🪄 Autofix (Beta)
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: 9f91729c-fbb7-4cf1-b7a3-e62c6f6180d0
⛔ Files ignored due to path filters (1)
cli-reference-links.tsvis excluded by!**/*.tsv,!*.tsv
📒 Files selected for processing (12)
.github/workflows/qa.yml.github/workflows/release.ymlCHANGELOG.mdMANUAL.mdMakefileVALIDATION.mdsrc/check-cliref-db.tssrc/extract-cliref.test.tssrc/extract-cliref.tssrc/link-cliref.test.tssrc/link-cliref.tssrc/release.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: copilot-pull-request-reviewer
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Bun and TypeScript, including Bun-native runtime APIs such as
bun:sqlite,Bun.serve, andbunx.Rosetta uses Bun and TypeScript; prefer
bun,bun test, andmake verifywhere applicable, rather than Node/npm-oriented substitutes.
Files:
src/extract-cliref.test.tssrc/extract-cliref.tssrc/check-cliref-db.tssrc/release.test.tssrc/link-cliref.test.tssrc/link-cliref.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Rosetta is read-only documentation/schema context and must not connect to or modify a user's router.
Files:
src/extract-cliref.test.tssrc/extract-cliref.tssrc/check-cliref-db.tssrc/release.test.tssrc/link-cliref.test.tssrc/link-cliref.ts
**/*.{ts,tsx,md}
📄 CodeRabbit inference engine (AGENTS.md)
Keep the attribution boundary visible when community RouterOS skills surface, because they are supplemental rather than official MikroTik documentation.
Files:
src/extract-cliref.test.tssrc/extract-cliref.tsMANUAL.mdCHANGELOG.mdVALIDATION.mdsrc/check-cliref-db.tssrc/release.test.tssrc/link-cliref.test.tssrc/link-cliref.ts
*
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Keep this Copilot instructions file short; put substantive rules in narrow instruction files under
.github/instructions/*.instructions.md.
Files:
MANUAL.mdCHANGELOG.mdVALIDATION.mdMakefile
**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.md: Keep each project documentation file limited to its canonical role; prefer the documented canonical home instead of creating a new top-level Markdown file.
Do not duplicate content from this routing index; move operational detail, schema blocks, or long rule lists to their canonical documentation or instruction files.
Apply the repository's Markdown linting and fenced-code conventions, while excluding LLM instruction files where the repository explicitly requires that exclusion.
Files:
MANUAL.mdCHANGELOG.mdVALIDATION.md
MANUAL.md
📄 CodeRabbit inference engine (CLAUDE.md)
Store installation, operations, release or re-extraction procedures, and schema reference material in
MANUAL.md.
Files:
MANUAL.md
{README.md,MANUAL.md,src/setup.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Keep public client setup snippets aligned across
README.md,src/setup.ts, andMANUAL.md, includingcodex mcp add rosetta -- bunx@tikoci/rosetta``.
Files:
MANUAL.md
CHANGELOG.md
📄 CodeRabbit inference engine (CLAUDE.md)
Record user-visible shipped changes in
CHANGELOG.mdunder[Unreleased]or release sections.
Files:
CHANGELOG.md
VALIDATION.md
📄 CodeRabbit inference engine (CLAUDE.md)
Document load-bearing invariants and how CI proves them in
VALIDATION.md.
Files:
VALIDATION.md
🧠 Learnings (8)
📚 Learning: 2026-07-12T20:33:29.883Z
Learnt from: mobileskyfi
Repo: tikoci/rosetta PR: 52
File: .github/workflows/qa.yml:101-101
Timestamp: 2026-07-12T20:33:29.883Z
Learning: In tikoci/rosetta, GitHub Actions workflow steps that reference official actions (e.g., `actions/checkout`, `actions/download-artifact`) may intentionally use version tags (e.g., `v4`, `v3`) instead of full commit SHAs. During code review, do not flag these unpinned `actions/*` tag references as a GitHub Actions security issue for this repository, because Dependabot’s `github_actions` updates tagged refs and GitHub surfaces action security advisories against tagged refs.
Applied to files:
.github/workflows/release.yml.github/workflows/qa.yml
📚 Learning: 2026-07-12T20:33:35.489Z
Learnt from: mobileskyfi
Repo: tikoci/rosetta PR: 52
File: .github/workflows/codeql.yml:29-29
Timestamp: 2026-07-12T20:33:35.489Z
Learning: In this repo’s GitHub Actions workflows, preserve the maintainer’s security/maintenance intent: action references under `actions/*` may use version tags (e.g., `actions/checkoutv7`) instead of full commit SHAs, and `actions/checkout` may intentionally keep `persist-credentials: true`. During review, do not raise issues suggesting SHA pinning for `actions/*` or proposing `persist-credentials: false` unless the project adopts an org-wide policy that enforces SHA pinning.
Applied to files:
.github/workflows/release.yml.github/workflows/qa.yml
📚 Learning: 2026-07-12T20:33:29.883Z
Learnt from: mobileskyfi
Repo: tikoci/rosetta PR: 52
File: .github/workflows/qa.yml:101-101
Timestamp: 2026-07-12T20:33:29.883Z
Learning: In this repository’s GitHub Actions workflows (under .github/workflows), do NOT flag actions/checkout steps for missing `persist-credentials: false` when the workflow is intentionally relying on the persisted GitHub token for later authenticated steps in the same job (e.g., release or subsequent steps that require auth). This exception applies to workflows where the checked-out workspace is not being uploaded as an artifact and the token persistence is an intentional part of the job’s authentication flow.
Applied to files:
.github/workflows/release.yml.github/workflows/qa.yml
📚 Learning: 2026-07-12T20:33:31.549Z
Learnt from: mobileskyfi
Repo: tikoci/rosetta PR: 52
File: .github/workflows/release.yml:433-433
Timestamp: 2026-07-12T20:33:31.549Z
Learning: In this repo, GitHub Actions used in workflow files should be pinned to version tags (e.g., actions/checkoutvX, not full commit SHAs). During code review, do not flag or fail checks that look like “unpinned-uses”/“SHA pinning required” (e.g., zizmor unpinned-uses) for actions/checkout, actions/download-artifact, or other third-party actions referenced from `.github/workflows`—keep the existing tag-pinning policy so Dependabot’s `github_actions` tracking and GitHub’s action security advisory visibility against tagged refs are preserved.
Applied to files:
.github/workflows/release.yml.github/workflows/qa.yml
📚 Learning: 2026-07-12T20:33:42.621Z
Learnt from: mobileskyfi
Repo: tikoci/rosetta PR: 52
File: .github/workflows/test.yml:19-19
Timestamp: 2026-07-12T20:33:42.621Z
Learning: In tikoci/rosetta workflows, follow the maintainer policy: do not require GitHub Actions step refs under `actions/*` (e.g., `actions/checkout`) to be pinned to full commit SHAs, and do not require `persist-credentials: false` to be set on `actions/checkout` (or other `actions/*`) by default. Only flag missing SHA pinning or missing `persist-credentials: false` if the maintainer explicitly indicates the policy has changed (for example, adopting an org-wide SHA-pinning enforcement).
Applied to files:
.github/workflows/release.yml.github/workflows/qa.yml
📚 Learning: 2026-07-12T20:33:36.397Z
Learnt from: mobileskyfi
Repo: tikoci/rosetta PR: 52
File: .github/workflows/release.yml:74-74
Timestamp: 2026-07-12T20:33:36.397Z
Learning: For this repo (tikoci/rosetta), workflow files in .github/workflows should keep GitHub Actions `actions/*` pinned to version tags (e.g., `v3`, `v4`) rather than full commit SHAs. Do not raise/retain findings that would otherwise complain about unpinned uses (e.g., zizmor `unpinned-uses`) or SHA pinning, unless the maintainer later enables an org-wide enforcement policy requiring SHA pinning.
Applied to files:
.github/workflows/release.yml.github/workflows/qa.yml
📚 Learning: 2026-07-12T20:33:31.549Z
Learnt from: mobileskyfi
Repo: tikoci/rosetta PR: 52
File: .github/workflows/release.yml:433-433
Timestamp: 2026-07-12T20:33:31.549Z
Learning: In tikoci/rosetta release-related GitHub Actions workflows, do not treat the default `actions/checkout` behavior for `persist-credentials` (i.e., not explicitly setting `persist-credentials: false`) as a security issue when the checked-out workspace is not uploaded as an artifact. The jobs rely on the persisted token for later authenticated steps within the same job. Only raise/require changing `persist-credentials` if the workflow uploads the checked-out workspace (or otherwise packages the token-bearing credential context) as an artifact, making the persisted credentials usable outside the job.
Applied to files:
.github/workflows/release.yml
📚 Learning: 2026-07-12T20:33:36.397Z
Learnt from: mobileskyfi
Repo: tikoci/rosetta PR: 52
File: .github/workflows/release.yml:74-74
Timestamp: 2026-07-12T20:33:36.397Z
Learning: In this repo’s release-related GitHub Actions workflows, do not flag `actions/checkout` steps for missing `persist-credentials: false`. The maintainer intentionally relies on the default `persist-credentials: true` because the job does not upload the checked-out workspace as an artifact and later authenticated steps in the same job require the persisted token. Only require changing `persist-credentials` if the job structure does not rely on the persisted credentials for subsequent steps.
Applied to files:
.github/workflows/release.yml
🔇 Additional comments (18)
src/extract-cliref.ts (1)
263-267: LGTM!src/extract-cliref.test.ts (1)
151-163: LGTM!.github/workflows/release.yml (1)
337-345: LGTM!src/release.test.ts (2)
704-716: LGTM!
1070-1089: LGTM!src/link-cliref.ts (3)
20-35: LGTM!
182-251: LGTM!
269-292: LGTM!src/link-cliref.test.ts (2)
2-11: LGTM!
126-178: LGTM!Makefile (2)
7-7: LGTM!
176-190: LGTM!src/check-cliref-db.ts (2)
14-15: 🎯 Functional CorrectnessReject or explicitly rule out
NULLvalues in these checks.
NOT IN,NOT BETWEEN, and<>evaluate toUNKNOWNforNULL, so nullable enum fields or view rows can bypass this gate. Add explicitIS NULLviolations, or verify the schema’sNOT NULLconstraints and add a regression test.Also applies to: 55-63, 75-82
1-13: LGTM!Also applies to: 16-54, 65-74, 84-90
.github/workflows/qa.yml (1)
304-317: LGTM!VALIDATION.md (1)
56-60: LGTM!MANUAL.md (1)
102-102: LGTM!CHANGELOG.md (1)
29-33: LGTM!
The db-content gate floored entries/fields/links but not cliref_flags, so a regression dropping all print-output flags could pass. Add a CLIREF_FLAGS count, summary line, and ≥800 floor (observed ~948); anchor it in release.test.ts. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
CoPilot got removed as reviewer by Claude. However, that does not stop a review, only its output in issue. This is a waste since if CoPilot triggers, it worth checking the results. Repo has CoPilot, again, configured for only initial push for automatic review. I did a manual request for CoPilot after df39ec3 git as a final double-check. Claude should not remove CoPilot as a reviewer, if fix the repo defaults. But Claude should not that if I manually request a review on an incremental push that is occasionally to be expected. So review https://github.com/tikoci/rosetta/tasks/bba4172f-62c4-4a37-94fc-cb7e4fedca7d?session_id=59aec810-fe49-4b12-beec-f1690fa93a26 did have one comment, that essentially went nowhere since the reviewer was just removed - that does not cancel a started review agent in GitHub. {
"comment_content": "`readCrosswalk()` orders only by `source_path`/`source_type`, but `cliref_entries` identity is `(page_id, source_order)` (see `src/db.ts:563-580`) and the design explicitly allows duplicate heading paths (`DESIGN.md:209`). With duplicates, the relative order among ties is undefined, so `buildBaselineTsv()` can produce non-deterministic TSV output and trip `--check` via harmless reorderings. Add a stable tie-breaker (e.g. `page_id, source_order`) to make the baseline generation deterministic.",
"comment_type": "bug",
"file_location": "src/link-cliref.ts",
"start_line": 199,
"end_line": 199,
"fixed": false,
"suggested_fix": " ORDER BY e.source_path, e.source_type, e.page_id, e.source_order`,"
}
192. .query(
193. `SELECT e.source_path AS source_path,
194. e.source_type AS source_type,
195. COALESCE(l.match_kind, 'manual-only') AS match_kind,
196. COALESCE(l.match_detail, '') AS match_detail
197. FROM cliref_entries e
198. LEFT JOIN cliref_entry_schema_links l ON l.entry_id = e.id
199. ORDER BY e.source_path, e.source_type`,
200. )
201. .all() as CrosswalkRow[]; |
readCrosswalk() ordered only by (source_path, source_type), but the corpus allows duplicate heading paths, so tied rows had undefined order — the baseline TSV could reorder harmlessly and trip link-cliref --check. Add (page_id, source_order), the entry identity, as a full tie-breaker. The current corpus has no such ties (committed baseline unchanged) but generation is now order-stable. Credit: Copilot review of df39ec3. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Addressing the Copilot review of df39ec3 (whose output I inadvertently suppressed by removing the reviewer — my mistake; I won't do that again): Valid finding — fixed in d33274c. |
Follow-on to #124/#126. Wires the CLI-Reference overlay into published builds and closes the two GAP validation rows deferred from #124. Verified against a full local build (59 restraml versions + live cli-reference fetch): 228 pages / 1,051 entries / 10,118 fields / 948 flags; crosswalk 907 exact / 24 alias / 120 manual-only.
1. CI publish wiring
release.ymlbuild job: adds Extract CLI-Reference overlay (extract-cliref) and Link CLI-Reference to inspect (link-cliref) — the link step runs after "Extract command tree" populatesschema_nodes(it fails fast on an empty tree by design). Published DBs now carrycliref_*+ the crosswalk, so thecli-reference/*export datasets are populated.qa.ymldb-contentfloors extended to require populatedcliref_entries/cliref_fields/cliref_entry_schema_links.2.
V-cliref-link-drift(GAP → blocking)link-cliref --checkre-links the built DB and byte-compares the exact/alias/manual-only crosswalk against a committed, reviewablecli-reference-links.tsv(a# countsheader + the 24 alias + 120 manual-only rows), and asserts every aliasmatch_detailnames aKNOWN_ALIAS_SEGMENTSmember.make link-cliref-baseline.qa.ymldb-content (db_source=artifact, the release path).3.
V-cliref-db-integrity(GAP → blocking)src/check-cliref-db.ts(make cliref-db-check):integrity_check/foreign_key_checkclean, no orphancliref_*rows, enum/CHECK valid, occurrence-order uniqueness, and the field view exposes only settableArgument→argmappings (everyRead-only Argumenthas zero view rows). Size-independent (counts stay in the floors). Runs inqa.ymldb-content.Bonus: a real parser bug the #124 fail-loud surfaced
Building the overlay against the live corpus tripped #126's
requireAttroninterface/pppoe-server:<ArgTableRow>attributes were truncated at a>inside a quoted value (typ="iface_enum { <l2tp>:0xfffffffe }"), sotyplost its closing quote and read as missing — previously stored as an emptyraw_typewith a garbled description. The tag matcher now skips whole quoted attribute values. Anchored by a new test; corpus counts are unchanged (values corrected, not rows).Validation
bun testgreen (1,043 pass). Biome, tsc, markdownlint clean.link-cliref --check: passes on match, exits 1 on drift (both verified locally).check-cliref-db.ts: clean against the full 1,051-entry build.VALIDATION.mdrows flipped;release.test.tsanchors the new steps + floors.Note: the PR's
test.ymlrunsbun testonly — the two new gates need a built DB, so they run onqa.ymldispatch / release. I'm dispatching aqa.yml(local-build, full_versions) run on this branch to exercise them end-to-end in CI.Closes #127.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
>characters.Documentation