Add per-article BCQuality coverage for code-review dataset (AB#646153)

notebooks/code-review-coverage.ipynb

@@ -0,0 +1,145 @@
+{
+ "cells": [
+  {
+   "cell_type": "markdown",
+   "id": "cb467b33",
+   "metadata": {},
+   "source": [
+    "# Per-article BCQuality coverage (code-review)\n",
+    "\n",
+    "Ad-hoc analysis of how the code-review gold dataset maps onto BCQuality\n",
+    "knowledge articles. Every finding is annotated with the article it derives\n",
+    "from (`ReviewComment.article`), and false-positive-guard entries carry their\n",
+    "association at entry level (`metadata.articles`). This notebook aggregates\n",
+    "those annotations via `bcbench.dataset.coverage`.\n",
+    "\n",
+    "Set `BCQUALITY_ROOT` (or edit the cell below) to point at a BCQuality checkout\n",
+    "to also surface articles with **zero** gold coverage; without it the report\n",
+    "covers declared articles only."
+   ]
+  },
+  {
+   "cell_type": "code",
+   "execution_count": null,
+   "id": "fcfeca75",
+   "metadata": {},
+   "outputs": [],
+   "source": [
+    "import os\n",
+    "\n",
+    "from bcbench.dataset import CodeReviewEntry\n",
+    "from bcbench.dataset.coverage import build_coverage_report, enumerate_inventory, resolve_bcquality_root\n",
+    "from bcbench.types import EvaluationCategory\n",
+    "\n",
+    "entries = CodeReviewEntry.load(EvaluationCategory.CODE_REVIEW.dataset_path)\n",
+    "\n",
+    "root = resolve_bcquality_root(os.environ.get(\"BCQUALITY_ROOT\"))\n",
+    "inventory = enumerate_inventory(root) if root is not None else None\n",
+    "\n",
+    "report = build_coverage_report(entries, inventory)\n",
+    "summary = f\"{len(report.covered)} articles covered across {report.annotated_entries}/{report.total_entries} annotated entries\"\n",
+    "if report.inventory_available:\n",
+    "    summary += f\"; {len(report.zero_coverage)} of {report.inventory_size} articles have zero coverage\"\n",
+    "print(summary)"
+   ]
+  },
+  {
+   "cell_type": "markdown",
+   "id": "1486b0a2",
+   "metadata": {},
+   "source": [
+    "## Coverage by domain"
+   ]
+  },
+  {
+   "cell_type": "code",
+   "execution_count": null,
+   "id": "119d9656",
+   "metadata": {},
+   "outputs": [],
+   "source": [
+    "import pandas as pd\n",
+    "\n",
+    "domains = sorted({c.domain for c in report.covered} | {a.split(\"/\", 1)[0] for a in report.zero_coverage})\n",
+    "rows = []\n",
+    "for domain in domains:\n",
+    "    covered = [c for c in report.covered if c.domain == domain]\n",
+    "    zero = [a for a in report.zero_coverage if a.split(\"/\", 1)[0] == domain]\n",
+    "    row = {\"Domain\": domain, \"Covered\": len(covered), \"Gold entries\": sum(c.count for c in covered)}\n",
+    "    if report.inventory_available:\n",
+    "        row[\"Inventory\"] = len(covered) + len(zero)\n",
+    "        row[\"Zero-cov\"] = len(zero)\n",
+    "    rows.append(row)\n",
+    "\n",
+    "coverage_df = pd.DataFrame(rows)\n",
+    "print(coverage_df.to_string(index=False))"
+   ]
+  },
+  {
+   "cell_type": "markdown",
+   "id": "7140d74b",
+   "metadata": {},
+   "source": [
+    "## Covered articles and the entries that exercise them"
+   ]
+  },
+  {
+   "cell_type": "code",
+   "execution_count": null,
+   "id": "0f717de6",
+   "metadata": {},
+   "outputs": [],
+   "source": [
+    "article_df = (\n",
+    "    pd.DataFrame([{\"Article\": c.article, \"Entries\": c.count, \"Instance IDs\": \", \".join(c.entry_ids)} for c in report.covered]).sort_values([\"Article\"])\n",
+    "    if report.covered\n",
+    "    else pd.DataFrame(columns=[\"Article\", \"Entries\", \"Instance IDs\"])\n",
+    ")\n",
+    "print(article_df.to_string(index=False))"
+   ]
+  },
+  {
+   "cell_type": "markdown",
+   "id": "106f03ae",
+   "metadata": {},
+   "source": [
+    "## Gaps: unknown slugs, zero-coverage articles, unannotated entries"
+   ]
+  },
+  {
+   "cell_type": "code",
+   "execution_count": null,
+   "id": "fa857f5b",
+   "metadata": {},
+   "outputs": [],
+   "source": [
+    "if report.unknown_articles:\n",
+    "    print(f\"Unknown article slugs not in inventory ({len(report.unknown_articles)}):\")\n",
+    "    for a in report.unknown_articles:\n",
+    "        print(f\"  - {a}\")\n",
+    "\n",
+    "if report.zero_coverage:\n",
+    "    print(f\"\\nZero-coverage articles ({len(report.zero_coverage)}):\")\n",
+    "    for a in report.zero_coverage:\n",
+    "        print(f\"  - {a}\")\n",
+    "\n",
+    "if report.unannotated_entry_ids:\n",
+    "    print(f\"\\nUnannotated entries ({len(report.unannotated_entry_ids)}):\")\n",
+    "    for e in report.unannotated_entry_ids:\n",
+    "        print(f\"  - {e}\")"
+   ]
+  }
+ ],
+ "metadata": {
+  "kernelspec": {
+   "display_name": "Python 3",
+   "language": "python",
+   "name": "python3"
+  },
+  "language_info": {
+   "name": "python"
+  }
+ },
+ "nbformat": 4,
+ "nbformat_minor": 5
+}

src/bcbench/collection/collect_codereview.py

@@ -33,8 +33,7 @@
 from typing import Any
 
 from bcbench.collection.gh_client import GHClient
-from bcbench.dataset import CodeReviewEntry, ReviewComment, Severity
-from bcbench.dataset.dataset_entry import EntryMetadata
+from bcbench.dataset import CodeReviewEntry, CodeReviewEntryMetadata, ReviewComment, Severity
 from bcbench.exceptions import CollectionError
 from bcbench.logger import get_logger
 
@@ -215,7 +214,7 @@ def _make_entry(
         created_at=created_at,
         environment_setup_version=environment_setup_version,
         patch=patch,
-        metadata=EntryMetadata(area=area),
+        metadata=CodeReviewEntryMetadata(area=area),
         expected_comments=expected_comments,
     )
 

src/bcbench/dataset/__init__.py

@@ -1,13 +1,24 @@
 """Dataset module for querying, validating and analyze dataset entries."""
 
-from bcbench.dataset.codereview import CodeReviewEntry, ReviewComment, Severity
+from bcbench.dataset.codereview import CodeReviewEntry, CodeReviewEntryMetadata, ReviewComment, Severity
+from bcbench.dataset.coverage import (
+    ArticleCoverage,
+    CoverageReport,
+    build_coverage_report,
+    collect_declared_articles,
+    enumerate_inventory,
+    resolve_bcquality_root,
+)
 from bcbench.dataset.dataset_entry import BaseDatasetEntry, BugFixEntry, NL2ALEntry, RepoGroundedEntry, TestEntry, TestGenEntry
 from bcbench.dataset.extensibility_request import ExtRequestImplementEntry, ExtRequestTriageEntry, ManagedLabel
 
 __all__ = [
+    "ArticleCoverage",
     "BaseDatasetEntry",
     "BugFixEntry",
     "CodeReviewEntry",
+    "CodeReviewEntryMetadata",
+    "CoverageReport",
     "ExtRequestImplementEntry",
     "ExtRequestTriageEntry",
     "ManagedLabel",
@@ -17,4 +28,8 @@
     "Severity",
     "TestEntry",
     "TestGenEntry",
+    "build_coverage_report",
+    "collect_declared_articles",
+    "enumerate_inventory",
+    "resolve_bcquality_root",
 ]

src/bcbench/dataset/codereview.py

@@ -1,11 +1,14 @@
 from __future__ import annotations
 
 from enum import StrEnum
-from typing import Annotated
+from typing import Annotated, Self
 
-from pydantic import BaseModel, ConfigDict, Field, field_validator
+from pydantic import BaseModel, ConfigDict, Field, field_validator, model_validator
 
-from bcbench.dataset.dataset_entry import RepoGroundedEntry
+from bcbench.dataset.dataset_entry import EntryMetadata, RepoGroundedEntry
+
+# BCQuality knowledge article id, formatted as `<domain>/<slug>` (e.g. `security/hardcoded-secret`).
+ArticleId = Annotated[str, Field(pattern=r"^[a-z0-9][a-z0-9-]*/[a-z0-9][a-z0-9-]*$")]
 
 
 class Severity(StrEnum):
@@ -58,6 +61,10 @@ class ReviewComment(BaseModel):
     domain: str | None = None
     body: Annotated[str, Field(min_length=1)]
     severity: Severity | None = None
+    # BCQuality knowledge article this finding derives from, as `<domain>/<slug>`
+    # (e.g. `security/hardcoded-secret`). Optional and backward-compatible; drives
+    # per-article coverage tracking. Older entries leave it unset (counted as unannotated).
+    article: ArticleId | None = None
 
     @field_validator("severity", mode="before")
     @classmethod
@@ -77,9 +84,20 @@ def __str__(self) -> str:
         return f"[{self.severity_label}] {loc}: {self.body}"
 
 
+class CodeReviewEntryMetadata(EntryMetadata):
+    """Code-review-specific entry metadata."""
+
+    # BCQuality knowledge articles this entry exercises as `<domain>/<slug>`. Primarily
+    # for false-positive-guard entries (expected_comments=[]) that test an article by
+    # omission and thus have no per-comment `article` to carry the association.
+    articles: list[ArticleId] = Field(default_factory=list)
+
+
 class CodeReviewEntry(RepoGroundedEntry):
     """Dataset entry for the code-review category."""
 
+    metadata: CodeReviewEntryMetadata = Field(default_factory=CodeReviewEntryMetadata)
+
     expected_comments: list[ReviewComment] = Field(default_factory=list)
     # Comments that are acceptable but not required. If the agent raises a matching
     # comment it is neither rewarded (recall) nor penalized (precision) -- it is dropped
@@ -88,8 +106,38 @@ class CodeReviewEntry(RepoGroundedEntry):
     # precedence, so a generated comment is only ever neutralized after expected matching.
     ignored_comments: list[ReviewComment] = Field(default_factory=list)
 
+    @model_validator(mode="after")
+    def _validate_article_annotations(self) -> Self:
+        """Keep per-comment and entry-level article annotations complementary.
+
+        `metadata.articles` carries the article association for findings no expected
+        comment covers (e.g. false-positive-guard entries whose `expected_comments` is
+        empty). An article already declared on a comment must not be repeated at entry
+        level, so the two annotation sources cannot silently drift apart.
+        """
+        comment_articles = {c.article for c in self.expected_comments if c.article}
+        overlap = comment_articles & set(self.metadata.articles)
+        if overlap:
+            raise ValueError(
+                f"{self.instance_id}: article(s) {sorted(overlap)} declared both per-comment "
+                "and in metadata.articles; entry-level metadata.articles is only for articles "
+                "no expected comment already carries"
+            )
+        return self
+
     def get_task(self) -> str:
         return self.patch
 
     def get_expected_output(self) -> str:
         return "\n".join(str(c) for c in self.expected_comments)
+
+    def declared_articles(self) -> set[str]:
+        """BCQuality articles this entry is annotated against.
+
+        Union of every expected comment's `article` and the entry-level
+        `metadata.articles` (which carries the association for false-positive-guard
+        entries whose `expected_comments` is empty).
+        """
+        articles = {c.article for c in self.expected_comments if c.article}
+        articles.update(self.metadata.articles)
+        return articles