#53: retrieval-quality gate hardening — golden set 24→35 + direct-surface eval matrix#57
Conversation
…face eval matrix Widen the Phase 0 retrieval net so an unknown Confluence→Docusaurus regression surfaces before 0.11.0 stable. Closes the zero-coverage gap on property/changelog/ video retrieval and adds a grounded 0.10.0 compare-and-contrast. Golden set (fixtures/eval/queries.json): 24 → 35, all verified against a fresh Docusaurus build. +2 nl (pppoe, dns), +4 property, +3 changelog, +2 video. Runner (src/eval/retrieval.ts): new per-query `surface` field. `search` (default) still drives the durable routeros_search recall/MRR/classifier gate — the entry point most exposed to the swap; `property`/`changelog`/`video` dispatch to the dedicated tools (lookupProperty/searchChangelogs/searchVideos) and feed a separate informational `surface_matrix` (per-surface hit@5) kept OUT of the gated aggregates, plus a soft "was passing, now misses" surface-regression warning. Thresholds recomputed for 27 search queries (0.88/0.82/0.75/0.90 — the old 0.85/0.70/0.65 had drifted loose); 2pp regression check vs baseline stays the tight forward guard. Both baseline.json and the stale April-2026 Confluence-era self-supervised-baseline.json regenerated against the Docusaurus corpus. Grounded audit (briefings/B-0020): no net regression from the swap (14/16 topic-hit on both corpora; 91.7% Recall@5 on Docusaurus); #26's two "regressions" confirmed to miss on 0.10.0 too (pre-existing query-core weakness, not swap-induced). New one-off src/eval/corpus-compare.ts drives the cross-corpus comparison. ETL Bug Ledger (BL-1 property-link gap pinned by a known-failing anchor, etc.) recorded, not hidden — no fixture loosened to green the board. 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:
📝 WalkthroughWalkthroughThe PR expands retrieval evaluation to dedicated property, changelog, and video surfaces, keeps them outside the search gate, adds corpus comparison tooling, documents the 0.11 audit, and adds a pull request reviewer gate. ChangesRetrieval quality evaluation
Copilot reviewer gate
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant GoldenQueries
participant runEval
participant RetrievalTools
participant Report
GoldenQueries->>runEval: provide surface-tagged queries
runEval->>RetrievalTools: dispatch search, property, changelog, or video retrieval
RetrievalTools-->>runEval: return retrieval result
runEval->>Report: record surface result and hit status
Report-->>runEval: separate search gate metrics from surface_matrix
Suggested labels: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
There was a problem hiding this comment.
Pull request overview
This pull request hardens Rosetta’s retrieval-quality evaluation for the 0.11.0 Docusaurus corpus by expanding the golden query set, introducing per-query retrieval “surfaces” (search vs direct tool surfaces), and regenerating baselines so CI gates reflect the current corpus.
Changes:
- Expanded the golden query set from 24 → 35 and recalibrated search-surface floors while keeping the baseline regression guard tight.
- Added
surface-aware evaluation insrc/eval/retrieval.ts, producing an informational per-surface hit@5 matrix for property/changelog/video without diluting the gated search aggregates. - Regenerated eval baselines and added a one-off cross-corpus comparator plus supporting documentation/changelog entries.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| VALIDATION.md | Updates V-retrieval-floor / V-retrieval-self definitions to reflect the expanded golden set and new surface-matrix reporting. |
| src/eval/retrieval.ts | Adds per-query surface dispatch and an informational surface_matrix, keeping search metrics as the only hard gate. |
| src/eval/corpus-compare.ts | Introduces a one-off script to compare 0.10.0 vs current corpus behavior using corpus-agnostic topic tokens. |
| project-words.txt | Adds “routerboot” to project dictionary. |
| fixtures/eval/queries.json | Expands to 35 queries, adds surface-specific expectations, and recomputes search floors. |
| fixtures/eval/baseline.json | Regenerates baseline for the expanded set and stores surface-matrix results. |
| CHANGELOG.md | Documents the retrieval-quality hardening and baseline regeneration work under Unreleased. |
| briefings/B-0020-0.11-retrieval-quality-audit.md | Adds an audit note capturing the measurement methodology, findings, and bug ledger references for #53. |
![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 `@src/eval/retrieval.ts`:
- Around line 351-370: Update the video-surface hit check in the retrieval
evaluation flow around searchVideos so a query is only considered valid when at
least one of expected_video_id or expected_video_contains is provided. Remove
the fallback that treats an unspecified expected_video_id as a match, while
preserving the existing ID-or-title/chapter/excerpt matching behavior when
criteria are present.
🪄 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: 64aeba09-fa7a-4c61-80de-0044df42cb01
⛔ Files ignored due to path filters (3)
fixtures/eval/baseline.jsonis excluded by!fixtures/**fixtures/eval/queries.jsonis excluded by!fixtures/**fixtures/eval/self-supervised-baseline.jsonis excluded by!fixtures/**
📒 Files selected for processing (6)
CHANGELOG.mdVALIDATION.mdbriefings/B-0020-0.11-retrieval-quality-audit.mdproject-words.txtsrc/eval/corpus-compare.tssrc/eval/retrieval.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: CodeRabbit
- GitHub Check: copilot-pull-request-reviewer
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (6)
*
📄 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:
project-words.txtCHANGELOG.mdVALIDATION.md
*.md
📄 CodeRabbit inference engine (CLAUDE.md)
*.md: Keep each project documentation file focused on its canonical role; prefer the canonical document instead of creating a new top-level Markdown file.
Put durable project rationale inDESIGN.md, operator or release procedures inMANUAL.md, and user-facing quick-start content inREADME.md.
Files:
CHANGELOG.mdVALIDATION.md
CHANGELOG.md
📄 CodeRabbit inference engine (CLAUDE.md)
Record user-visible shipped changes in
CHANGELOG.mdunder[Unreleased]or release sections.
Files:
CHANGELOG.md
briefings/B-*.md
📄 CodeRabbit inference engine (CLAUDE.md)
briefings/B-*.md: Usebriefings/B-*.mdfor grounded research, decision support, and research notes.
Usebriefings/B-*.mdfor grounded research and decision support rather than placing it in issues or general documentation.
Files:
briefings/B-0020-0.11-retrieval-quality-audit.md
**/*.{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
bunandbun testrather than Node/npm-oriented substitutes.
Files:
src/eval/corpus-compare.tssrc/eval/retrieval.ts
VALIDATION.md
📄 CodeRabbit inference engine (CLAUDE.md)
Keep load-bearing invariants and the way CI proves them in
VALIDATION.md.
Files:
VALIDATION.md
🔇 Additional comments (6)
src/eval/retrieval.ts (1)
27-42: LGTM!Also applies to: 53-145, 175-175, 272-350, 371-656
src/eval/corpus-compare.ts (1)
1-127: LGTM!VALIDATION.md (1)
25-26: LGTM!Also applies to: 73-73
briefings/B-0020-0.11-retrieval-quality-audit.md (1)
1-91: LGTM!CHANGELOG.md (1)
27-27: LGTM!project-words.txt (1)
317-317: LGTM!
Copilot + CodeRabbit flagged that evalSurfaceQuery's changelog and video branches were vacuously true when the expected fields were omitted — a misconfigured fixture would pass while asserting nothing (the video predicate collapsed to `!expected_video_id` = true; changelog fell back to `: true`). A surface query with no expectation now HARD-FAILS loudly (MISCONFIGURED note, surface_hit=false) rather than passing for free or quietly skipping, so silent coverage loss can't merge. changelog requires expected_changelog_contains OR expected_version; video requires expected_video_contains OR expected_video_id; property's missing-expected_property guard made consistent (fail, not skip). Real fixtures unchanged (all carry required fields). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Addressed the review comments (Copilot ×2, CodeRabbit ×1 — all the same vacuous-pass issue) in e2a803b. Fix:
Verified: typecheck + biome clean; real fixtures unchanged (property 2/3, changelog 3/3, video 2/2); no |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…arator, scope gate token Address remaining PR #57 review comments: - retrieval.ts: unknown-surface MISCONFIGURED note now reports the computed effective surface, not the raw q.surface (which can be unset when the surface is inferred from shape). - corpus-compare.ts: mirror retrieval.ts's effectiveSurface() so a fixture row that sets shape (property/changelog/video) but forgets surface can't leak into the search-corpus comparison. - copilot-gate.yml: add a minimal top-level permissions block (contents: read, pull-requests: read) — CodeQL flagged the unscoped GITHUB_TOKEN; the gate only reads PR requested-reviewer state. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@coderabbitai full review |
✅ Action performedFull review finished. |
✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 59 minutes. |
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/copilot-gate.yml:
- Around line 25-26: Update the reviewer check in the PENDING validation to use
fixed-string, whole-line matching for the exact login
copilot-pull-request-reviewer, preventing partial or substring matches while
preserving the existing blocking behavior.
- Around line 13-15: Add PR-scoped concurrency to the check-pending job in the
workflow, using a group keyed by the pull request identity and enabling
cancel-in-progress so newer review-state runs replace stale ones.
- Around line 3-6: Add ready_for_review to the pull_request trigger in the
workflow. Add concurrency configuration with cancel-in-progress enabled to
prevent stale runs from publishing results. Update the
copilot-pull-request-reviewer gate check to require an exact match using grep -x
or equivalent equality logic instead of substring matching.
In `@src/eval/corpus-compare.ts`:
- Around line 54-59: Export the existing effectiveSurface and
directSurfaceFromShape helpers from retrieval.ts, then import and reuse them in
corpus-compare.ts instead of maintaining the local directShapes set and
effectiveSurface implementation. Preserve the current fallback to "search" and
remove the duplicated logic.
🪄 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: c9d8a16b-cb1b-4f05-b7a7-c5bd6032b899
⛔ Files ignored due to path filters (3)
fixtures/eval/baseline.jsonis excluded by!fixtures/**fixtures/eval/queries.jsonis excluded by!fixtures/**fixtures/eval/self-supervised-baseline.jsonis excluded by!fixtures/**
📒 Files selected for processing (7)
.github/workflows/copilot-gate.ymlCHANGELOG.mdVALIDATION.mdbriefings/B-0020-0.11-retrieval-quality-audit.mdproject-words.txtsrc/eval/corpus-compare.tssrc/eval/retrieval.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (6)
*
📄 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:
project-words.txtCHANGELOG.mdVALIDATION.md
*.md
📄 CodeRabbit inference engine (CLAUDE.md)
*.md: Keep each project documentation file focused on its canonical role; prefer the canonical document instead of creating a new top-level Markdown file.
Put durable project rationale inDESIGN.md, operator or release procedures inMANUAL.md, and user-facing quick-start content inREADME.md.
Files:
CHANGELOG.mdVALIDATION.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)
Keep load-bearing invariants and the way CI proves them in
VALIDATION.md.
Files:
VALIDATION.md
briefings/B-*.md
📄 CodeRabbit inference engine (CLAUDE.md)
briefings/B-*.md: Usebriefings/B-*.mdfor grounded research, decision support, and research notes.
Usebriefings/B-*.mdfor grounded research and decision support rather than placing it in issues or general documentation.
Files:
briefings/B-0020-0.11-retrieval-quality-audit.md
**/*.{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
bunandbun testrather than Node/npm-oriented substitutes.
Files:
src/eval/corpus-compare.tssrc/eval/retrieval.ts
🧠 Learnings (6)
📚 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/copilot-gate.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/copilot-gate.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/copilot-gate.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/copilot-gate.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/copilot-gate.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/copilot-gate.yml
🪛 zizmor (1.26.1)
.github/workflows/copilot-gate.yml
[warning] 11-11: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment
(undocumented-permissions)
[info] 14-14: workflow or action definition without a name (anonymous-definition): this job
(anonymous-definition)
[warning] 2-8: insufficient job-level concurrency limits (concurrency-limits): workflow is missing concurrency setting
(concurrency-limits)
🔇 Additional comments (7)
.github/workflows/copilot-gate.yml (1)
1-1: LGTM!Also applies to: 9-11, 17-24, 27-30
src/eval/retrieval.ts (1)
27-27: LGTM!Also applies to: 32-42, 53-68, 78-91, 114-122, 142-145, 168-184, 281-281, 295-406, 429-449, 520-540, 596-625, 639-647, 679-695
src/eval/corpus-compare.ts (1)
1-134: LGTM! Verified thenew Map(runChild(oldDb).map((r) => [r.id, r]))pattern (Line 86) does not trip TypeScript's tuple inference — contextual typing fromMap's constructor correctly infers the callback's return as[string, EmitRow].VALIDATION.md (1)
25-26: LGTM! Cross-checked the 35/27/8 query split and theV-retrieval-selfaudit note againstbriefings/B-0020-0.11-retrieval-quality-audit.md— figures reconcile.Also applies to: 73-73
briefings/B-0020-0.11-retrieval-quality-audit.md (1)
1-91: LGTM! Numeric claims (16 TOPICS entries, 24→35 golden-set breakdown, Bug Ledger items) are internally consistent and reconcile withCHANGELOG.md,VALIDATION.md, andsrc/eval/corpus-compare.ts.CHANGELOG.md (1)
27-27: LGTM!project-words.txt (1)
317-317: LGTM!
- Add `ready_for_review` trigger so draft→ready transitions run the gate. - Add PR-scoped `concurrency` with cancel-in-progress so a superseded run can't publish a stale gate result after the reviewer state changes. - Match the reviewer login exactly (`grep -qxF`) instead of by substring. - Quote the `gh api` path vars (shellcheck SC2086); actionlint now clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Closes #53.
Part of the 0.11.0 release quality bar (milestone) and umbrella #28's "did we regress from the HTML corpus" concern. Sibling of #26.
What this does
fixtures/eval/queries.json), closing the zero-coverage gap on property/changelog/video retrieval that the Confluence→Docusaurus swap left untested.src/eval/retrieval.tsgains a per-querysurfacefield.search(default) still drives the durablerouteros_searchrecall/MRR/classifier gate — the entry point most exposed to the swap, so it carries the weight.property/changelog/videodispatch to the dedicated tools (lookupProperty/searchChangelogs/searchVideos) and feed a separate, informationalsurface_matrix(per-surface hit@5) kept out of the gated aggregates, plus a soft "was passing, now misses" surface-regression warning. Grows into a coverage board over time.src/query.tsuntouched (ranking fixes belong to Retrieval ranking regressions from Docusaurus corpus swap (nl-firewall-filter, nl-bgp golden queries) #26)._thresholdsrecomputed for 27 search queries (0.88/0.82/0.75/0.90 — the pre-0.11 retrieval-quality gate hardening: expand golden set + blocking self-eval audit #53 0.85/0.70/0.65 had drifted loose over the larger set; rationale inline in_doc). The 2pp regression check vsbaseline.jsonstays the tight forward guard.self-supervised-baseline.json(never regenerated after the swap, so every post-swap run showed a phantom regression).All verified against a fresh local
make extract(live manual.mikrotik.com): 363 pages / 4402 props / 1311 changelogs / 1953 video segments.New queries — why each page is the right answer
nl-pppoedocs/virtual-private-networks/pppoenl-dnsdocs/network-management/dnsprop-search-mtudocs/hardware/mtu-in-routeros+classifier.property=mtu+related.propertiesrouteros_search.prop-wireguard-listen-port/interface/wireguardHIGHprop-vxlan-vni/interface/vxlanHIGHprop-firewall-filter-action/ip/firewall/filteractionexists on the Filter page + the path exists, but they aren't linked → 0 HIGH. Pins the core-menu linkage gap; not loosened.changelog-cvechangelog-rose-storagechangelog-wireguard-7-4video-containervideo-wireguardResults (current Docusaurus corpus)
nl-firewall-filter,nl-bgp(Retrieval ranking regressions from Docusaurus corpus swap (nl-firewall-filter, nl-bgp golden queries) #26),prop-firewall-filter-action(BL-1).Audit deliverables (see
briefings/B-0020+ the #53 comment)src/eval/corpus-compare.ts(one-off) scores 14/16 topic-hit@5 on BOTH 0.10.0 and current.V-retrieval-selfpromotion path).Verification
bun run typecheck·make lint·bun test(768 pass / 0 fail) ·DB_PATH=<fresh> bun run src/eval/retrieval.tsall green. Recommend aqa.ymldispatch (db_source=local-build) to confirm baseline parity against a CI build.🤖 Generated with Claude Code
Summary by CodeRabbit
Quality Improvements
Documentation
Chores