feat(catalog): itemize drift report (#237) - #279
Conversation
|
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 catalog check now itemizes catalog drift, including row counts, path changes, provenance, kind, and gate metadata. CI publishes the structured report or an expanded fallback summary. The glossary and spell-check configuration were updated. ChangesCatalog drift reporting
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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.yaml:
- Around line 170-182: Replace the multiline fallback prose in the QA workflow
with a single concise summary line that points reviewers to the `bun run
explain:catalog` command. Remove the duplicated explanatory wording and leave
detailed drift messaging owned by `buildDriftReport` in
`scripts/gen-explain-catalog.ts`.
In `@scripts/gen-explain-catalog.ts`:
- Around line 227-236: Emit the fixed sentinel “<!-- explain-catalog-drift -->”
in every report path of the generator, including the committed === null branch
in scripts/gen-explain-catalog.ts (lines 227-236). Update
.github/workflows/qa.yaml (lines 164-168) to detect existing reports with
fixed-string grep for that sentinel instead of “Row counts”.
- Around line 318-366: Update the provenance flip collection in the loop over
fresh and committed rows to retain structured records containing the path, from,
to, and kind values instead of pre-rendered strings. Add a formatFlip helper for
markdown rendering, categorize with from === "both" and to === "published", and
use it for both categorized and remaining output while preserving sorting,
capping, and counts.
- Around line 412-419: Update the summary-writing logic in the report-generation
flow to append rather than overwrite existing GitHub step summaries. Add
appendFileSync to the existing node:fs import and replace Bun.write in the
GITHUB_STEP_SUMMARY block, preserving the current report newline and best-effort
error handling.
🪄 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: 16715ac4-d13f-45bc-b8ed-f152cd257fd1
📒 Files selected for processing (4)
.github/workflows/qa.yamlGLOSSARY.txtcspell.jsonscripts/gen-explain-catalog.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 (2)
{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:
.github/workflows/qa.yamlGLOSSARY.txt
**/*.{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:
scripts/gen-explain-catalog.ts
🧠 Learnings (13)
📚 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:
cspell.json
📚 Learning: 2026-06-08T22:23:31.252Z
Learnt from: CR
Repo: tikoci/centrs PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-06-08T22:23:31.252Z
Learning: Read `../AGENTS.md` first for agent configuration and guidelines
Applied to files:
cspell.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: Applies to **/AGENTS.md : Use directory-level `AGENTS.md` files only for local constraints.
Applied to files:
cspell.json
📚 Learning: 2026-06-15T16:06:55.935Z
Learnt from: CR
Repo: tikoci/centrs PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-06-15T16:06:55.935Z
Learning: Applies to test/**/AGENTS.md : Document agent configurations and behavior expectations in AGENTS.md
Applied to files:
cspell.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: Applies to **/*.{ts,tsx} : Do not make generated output the hand-edited source of truth.
Applied to files:
cspell.json
📚 Learning: 2026-06-08T22:26:00.538Z
Learnt from: CR
Repo: tikoci/centrs PR: 0
File: src/data/AGENTS.md:0-0
Timestamp: 2026-06-08T22:26:00.538Z
Learning: Applies to src/data/**/*{device,persist,credential,cache,storage}*.{ts,tsx,js} : Do not silently persist credentials or discovered devices; persistence must be controlled by a typed setting and explained in interactive output
Applied to files:
cspell.json
📚 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/qa.yaml
📚 Learning: 2026-08-06T23:42:39.898Z
Learnt from: mobileskyfi
Repo: tikoci/centrs PR: 227
File: src/explain/args.ts:151-158
Timestamp: 2026-08-06T23:42:39.898Z
Learning: For the RouterOS explain argument lexer corpus checks, a green frozen corpus measures coverage and risk but does not validate lexical rules absent from the corpus. The frozen corpus contains no lone-`\r` continuation case, so edge-case probes and targeted regression tests remain required for continuation behavior.
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:
GLOSSARY.txtscripts/gen-explain-catalog.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: Prefer `rosetta` MCP tools for RouterOS facts before using web search.
Applied to files:
GLOSSARY.txt
📚 Learning: 2026-06-08T22:26:06.293Z
Learnt from: CR
Repo: tikoci/centrs PR: 0
File: src/protocols/AGENTS.md:0-0
Timestamp: 2026-06-08T22:26:06.293Z
Learning: Prefer rosetta MCP lookups and related tikoci project evidence over general web search for RouterOS-specific facts
Applied to files:
GLOSSARY.txt
📚 Learning: 2026-06-08T22:26:06.293Z
Learnt from: CR
Repo: tikoci/centrs PR: 0
File: src/protocols/AGENTS.md:0-0
Timestamp: 2026-06-08T22:26:06.293Z
Learning: Applies to src/protocols/commands/**/*.{js,ts} : Surface missing local tools with platform-aware install or remediation advice
Applied to files:
scripts/gen-explain-catalog.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:
scripts/gen-explain-catalog.ts
🔇 Additional comments (4)
scripts/gen-explain-catalog.ts (2)
28-28: LGTM!Also applies to: 200-214, 238-317, 370-390
165-198: 🗄️ Data Integrity & IntegrationNo parser/emitter mismatch exists.
renderRowemits fields in parser order, and the generatedROWSblock uses the required delimiters.> Likely an incorrect or invalid review comment.GLOSSARY.txt (1)
678-678: LGTM!cspell.json (1)
17-18: LGTM!
There was a problem hiding this comment.
Pull request overview
Enhances the catalog drift check to emit an itemized, actionable drift report (paths added/removed, provenance flips, gate-string changes, counts) rather than a binary “out of date” message, and wires that structured report into the GitHub Actions QA summary.
Changes:
- Add structured drift parsing + diffing to
scripts/gen-explain-catalog.tsand emit a Markdown report to stderr /.scratch/ step summary on--checkfailure. - Update
qa.yamlto prefer the structured drift artifact in the job summary (with duplicate-avoidance). - Update spelling configuration/glossary entries related to repo terminology.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| scripts/gen-explain-catalog.ts | Builds and outputs a structured catalog drift report during --check, including counts and itemized deltas. |
| .github/workflows/qa.yaml | Adjusts the “Report drift” step to consume the structured drift report file and avoid duplicate summaries. |
| GLOSSARY.txt | Adds a project vocabulary entry. |
| cspell.json | Updates cspell ignore paths to include .agents/**. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…ured flips, append, fallback
- parseCommittedCatalog handles CRLF (\r?\n) and strips trailing \r (copilot 168)
- separate provenance/kind tables with blank line (copilot 260)
- provenanceFlips as structured {path,from,to,kind} + formatFlip, filter by from/to not substring (coderabbit 366)
- use appendFileSync for GITHUB_STEP_SUMMARY to avoid overwrite (coderabbit 419 + copilot 419)
- emit <!-- explain-catalog-drift --> sentinel in every report and grep sentinel in qa.yaml (coderabbit 236)
- qa fallback: single concise line, detailed wording owned by buildDriftReport (coderabbit 182)
- cspell: ignore .claude/** (was .claude/settings*.json) to cover scheduled_tasks.lock etc.
- gitignore: add .agents/** (was missing on branch)
5733218 to
ce6dca6
Compare
Closes #237
Summary by CodeRabbit
Improvements
Documentation
Maintenance