From d77c43e6f6c45b4b53cc8f809ec92fd136342d13 Mon Sep 17 00:00:00 2001 From: Dominikus Nold Date: Wed, 18 Feb 2026 12:31:36 +0100 Subject: [PATCH 1/4] feat: add bundle-mapper module with confidence-based spec-to-bundle mapping - BundleMapping model and BundleMapper engine (explicit label, historical, content similarity) - Mapping history persistence and MappingRule (save_user_confirmed_mapping, load_bundle_mapping_config) - Interactive UI (ask_bundle_mapping) with Rich confidence visualization - Unit tests and TDD_EVIDENCE for bundle-mapper-01 (OpenSpec #121) Co-authored-by: Cursor --- modules/bundle-mapper/module-package.yaml | 22 ++ .../src/bundle_mapper/__init__.py | 7 + .../src/bundle_mapper/commands/__init__.py | 1 + .../src/bundle_mapper/mapper/__init__.py | 7 + .../src/bundle_mapper/mapper/engine.py | 204 ++++++++++++++++++ .../src/bundle_mapper/mapper/history.py | 132 ++++++++++++ .../src/bundle_mapper/models/__init__.py | 6 + .../bundle_mapper/models/bundle_mapping.py | 49 +++++ .../src/bundle_mapper/ui/__init__.py | 6 + .../src/bundle_mapper/ui/interactive.py | 90 ++++++++ modules/bundle-mapper/tests/__init__.py | 1 + modules/bundle-mapper/tests/conftest.py | 10 + modules/bundle-mapper/tests/unit/__init__.py | 1 + .../tests/unit/test_bundle_mapper_engine.py | 67 ++++++ .../tests/unit/test_bundle_mapping_model.py | 35 +++ .../tests/unit/test_mapping_history.py | 64 ++++++ .../TDD_EVIDENCE.md | 14 ++ 17 files changed, 716 insertions(+) create mode 100644 modules/bundle-mapper/module-package.yaml create mode 100644 modules/bundle-mapper/src/bundle_mapper/__init__.py create mode 100644 modules/bundle-mapper/src/bundle_mapper/commands/__init__.py create mode 100644 modules/bundle-mapper/src/bundle_mapper/mapper/__init__.py create mode 100644 modules/bundle-mapper/src/bundle_mapper/mapper/engine.py create mode 100644 modules/bundle-mapper/src/bundle_mapper/mapper/history.py create mode 100644 modules/bundle-mapper/src/bundle_mapper/models/__init__.py create mode 100644 modules/bundle-mapper/src/bundle_mapper/models/bundle_mapping.py create mode 100644 modules/bundle-mapper/src/bundle_mapper/ui/__init__.py create mode 100644 modules/bundle-mapper/src/bundle_mapper/ui/interactive.py create mode 100644 modules/bundle-mapper/tests/__init__.py create mode 100644 modules/bundle-mapper/tests/conftest.py create mode 100644 modules/bundle-mapper/tests/unit/__init__.py create mode 100644 modules/bundle-mapper/tests/unit/test_bundle_mapper_engine.py create mode 100644 modules/bundle-mapper/tests/unit/test_bundle_mapping_model.py create mode 100644 modules/bundle-mapper/tests/unit/test_mapping_history.py create mode 100644 openspec/changes/bundle-mapper-01-mapping-strategy/TDD_EVIDENCE.md diff --git a/modules/bundle-mapper/module-package.yaml b/modules/bundle-mapper/module-package.yaml new file mode 100644 index 00000000..e93b39f0 --- /dev/null +++ b/modules/bundle-mapper/module-package.yaml @@ -0,0 +1,22 @@ +name: bundle-mapper +version: "0.1.0" +commands: [] +pip_dependencies: [] +module_dependencies: [] +core_compatibility: ">=0.28.0,<1.0.0" +tier: community +schema_extensions: + project_bundle: {} + project_metadata: + bundle_mapper.mapping_rules: + type: "list | None" + description: "Persistent mapping rules from user confirmations" + bundle_mapper.history: + type: "dict | None" + description: "Auto-populated historical mappings (item_key -> bundle_id counts)" +publisher: + name: nold-ai + url: https://github.com/nold-ai/specfact-cli-modules +integrity: + checksum_algorithm: sha256 +dependencies: [] diff --git a/modules/bundle-mapper/src/bundle_mapper/__init__.py b/modules/bundle-mapper/src/bundle_mapper/__init__.py new file mode 100644 index 00000000..d23cba4a --- /dev/null +++ b/modules/bundle-mapper/src/bundle_mapper/__init__.py @@ -0,0 +1,7 @@ +"""Bundle mapper module: confidence-based spec-to-bundle assignment with interactive review.""" + +from bundle_mapper.mapper.engine import BundleMapper +from bundle_mapper.models.bundle_mapping import BundleMapping + + +__all__ = ["BundleMapper", "BundleMapping"] diff --git a/modules/bundle-mapper/src/bundle_mapper/commands/__init__.py b/modules/bundle-mapper/src/bundle_mapper/commands/__init__.py new file mode 100644 index 00000000..191c5148 --- /dev/null +++ b/modules/bundle-mapper/src/bundle_mapper/commands/__init__.py @@ -0,0 +1 @@ +"""Command hooks for backlog refine/import --auto-bundle (used when module is loaded).""" diff --git a/modules/bundle-mapper/src/bundle_mapper/mapper/__init__.py b/modules/bundle-mapper/src/bundle_mapper/mapper/__init__.py new file mode 100644 index 00000000..12618015 --- /dev/null +++ b/modules/bundle-mapper/src/bundle_mapper/mapper/__init__.py @@ -0,0 +1,7 @@ +"""Bundle mapper engine and history.""" + +from bundle_mapper.mapper.engine import BundleMapper +from bundle_mapper.mapper.history import save_user_confirmed_mapping + + +__all__ = ["BundleMapper", "save_user_confirmed_mapping"] diff --git a/modules/bundle-mapper/src/bundle_mapper/mapper/engine.py b/modules/bundle-mapper/src/bundle_mapper/mapper/engine.py new file mode 100644 index 00000000..1fa30821 --- /dev/null +++ b/modules/bundle-mapper/src/bundle_mapper/mapper/engine.py @@ -0,0 +1,204 @@ +""" +BundleMapper engine: confidence-based mapping from backlog items to bundles. +""" + +from __future__ import annotations + +import re +from pathlib import Path +from typing import Any + +from beartype import beartype +from icontract import ensure, require + +from bundle_mapper.mapper.history import ( + item_key, + item_keys_similar, + load_bundle_mapping_config, +) +from bundle_mapper.models.bundle_mapping import BundleMapping + + +try: + from specfact_cli.models.backlog_item import BacklogItem +except ImportError: + BacklogItem = Any # type: ignore[misc, assignment] + +WEIGHT_EXPLICIT = 0.8 +WEIGHT_HISTORICAL = 0.15 +WEIGHT_CONTENT = 0.05 +HISTORY_CAP = 10.0 + + +def _tokenize(text: str) -> set[str]: + """Lowercase, split by non-alphanumeric.""" + return set(re.findall(r"[a-z0-9]+", text.lower())) + + +def _jaccard(a: set[str], b: set[str]) -> float: + """Jaccard similarity between two sets.""" + if not a and not b: + return 1.0 + if not a or not b: + return 0.0 + return len(a & b) / len(a | b) + + +@beartype +class BundleMapper: + """ + Computes mapping from backlog items to OpenSpec bundle ids using three signals: + explicit labels (bundle:xyz), historical patterns, content similarity. + """ + + def __init__( + self, + available_bundle_ids: list[str] | None = None, + config_path: Path | None = None, + bundle_spec_keywords: dict[str, set[str]] | None = None, + ) -> None: + """ + Args: + available_bundle_ids: Valid bundle ids (for explicit label validation). + config_path: Path to .specfact config for rules/history. + bundle_spec_keywords: Optional map bundle_id -> set of keywords from specs (for content similarity). + """ + self._available_bundle_ids = set(available_bundle_ids or []) + self._config_path = config_path + self._config: dict[str, Any] = {} + self._bundle_keywords = bundle_spec_keywords or {} + + def _load_config(self) -> dict[str, Any]: + if not self._config: + self._config = load_bundle_mapping_config(self._config_path) + return self._config + + @beartype + def _score_explicit_mapping(self, item: BacklogItem) -> tuple[str | None, float]: + """Return (bundle_id, score) for explicit bundle:xyz tag, or (None, 0.0).""" + prefix = self._load_config().get("explicit_label_prefix", "bundle:") + for tag in item.tags: + tag = (tag or "").strip() + if tag.startswith(prefix): + bundle_id = tag[len(prefix) :].strip() + if bundle_id and (not self._available_bundle_ids or bundle_id in self._available_bundle_ids): + return (bundle_id, 1.0) + return (None, 0.0) + + @beartype + def _score_historical_mapping(self, item: BacklogItem) -> tuple[str | None, float]: + """Return (bundle_id, score) from history, or (None, 0.0).""" + key = item_key(item) + history = self._load_config().get("history", {}) + best_bundle: str | None = None + best_count = 0 + for hist_key, entry in history.items(): + if not item_keys_similar(key, hist_key): + continue + counts = entry.get("counts", {}) + for bid, cnt in counts.items(): + if cnt > best_count: + best_count = cnt + best_bundle = bid + if best_bundle is None: + return (None, 0.0) + score = min(1.0, best_count / HISTORY_CAP) + return (best_bundle, score) + + @beartype + def _score_content_similarity(self, item: BacklogItem) -> list[tuple[str, float]]: + """Return list of (bundle_id, score) by keyword overlap with item title/body.""" + text = f"{item.title} {item.body_markdown or ''}" + tokens = _tokenize(text) + if not tokens: + return [] + results: list[tuple[str, float]] = [] + for bundle_id, keywords in self._bundle_keywords.items(): + sim = _jaccard(tokens, keywords) + if sim > 0: + results.append((bundle_id, sim)) + return sorted(results, key=lambda x: -x[1]) + + @beartype + def _explain_score(self, bundle_id: str, score: float, method: str) -> str: + """Human-readable one-line explanation.""" + if method == "explicit_label": + return f"Explicit label → {bundle_id} (confidence {score:.2f})" + if method == "historical": + return f"Historical pattern → {bundle_id} (confidence {score:.2f})" + if method == "content_similarity": + return f"Content similarity → {bundle_id} (confidence {score:.2f})" + return f"{bundle_id} (confidence {score:.2f})" + + @beartype + def _build_explanation( + self, + primary_bundle_id: str | None, + confidence: float, + candidates: list[tuple[str, float]], + reasons: list[str], + ) -> str: + """Build full explanation string.""" + parts = [f"Confidence: {confidence:.2f}"] + if reasons: + parts.append("; ".join(reasons)) + if candidates: + parts.append("Alternatives: " + ", ".join(f"{b}({s:.2f})" for b, s in candidates[:5])) + return ". ".join(parts) + + @beartype + @require(lambda item: item is not None, "Item must not be None") + @ensure( + lambda result: 0.0 <= result.confidence <= 1.0, + "Confidence in [0, 1]", + ) + def compute_mapping(self, item: BacklogItem) -> BundleMapping: + """ + Compute mapping for one backlog item using weighted signals: + 0.8 * explicit + 0.15 * historical + 0.05 * content. + """ + reasons: list[str] = [] + explicit_bundle, explicit_score = self._score_explicit_mapping(item) + hist_bundle, hist_score = self._score_historical_mapping(item) + content_list = self._score_content_similarity(item) + + primary_bundle_id: str | None = None + weighted = 0.0 + + if explicit_bundle and explicit_score > 0: + primary_bundle_id = explicit_bundle + weighted += WEIGHT_EXPLICIT * explicit_score + reasons.append(self._explain_score(explicit_bundle, explicit_score, "explicit_label")) + + if hist_bundle and hist_score > 0: + contrib = WEIGHT_HISTORICAL * hist_score + if primary_bundle_id is None: + primary_bundle_id = hist_bundle + weighted += contrib + reasons.append(self._explain_score(hist_bundle, hist_score, "historical")) + else: + weighted += contrib + + if content_list: + best_content = content_list[0] + contrib = WEIGHT_CONTENT * best_content[1] + weighted += contrib + if primary_bundle_id is None: + primary_bundle_id = best_content[0] + reasons.append(self._explain_score(best_content[0], best_content[1], "content_similarity")) + + confidence = min(1.0, weighted) + candidates: list[tuple[str, float]] = [] + if primary_bundle_id: + seen = {primary_bundle_id} + for bid, sc in content_list: + if bid not in seen: + seen.add(bid) + candidates.append((bid, sc * WEIGHT_CONTENT)) + explanation = self._build_explanation(primary_bundle_id, confidence, candidates, reasons) + return BundleMapping( + primary_bundle_id=primary_bundle_id, + confidence=confidence, + candidates=candidates[:10], + explained_reasoning=explanation, + ) diff --git a/modules/bundle-mapper/src/bundle_mapper/mapper/history.py b/modules/bundle-mapper/src/bundle_mapper/mapper/history.py new file mode 100644 index 00000000..bf224329 --- /dev/null +++ b/modules/bundle-mapper/src/bundle_mapper/mapper/history.py @@ -0,0 +1,132 @@ +""" +Mapping history persistence: save and load user-confirmed mappings from config. +""" + +from __future__ import annotations + +import re +from pathlib import Path +from typing import Any, Protocol, runtime_checkable + +import yaml +from beartype import beartype +from icontract import ensure, require +from pydantic import BaseModel, Field + + +DEFAULT_LABEL_PREFIX = "bundle:" +DEFAULT_AUTO_ASSIGN_THRESHOLD = 0.8 +DEFAULT_CONFIRM_THRESHOLD = 0.5 + + +@runtime_checkable +class _ItemLike(Protocol): + """Minimal interface for backlog item used by history.""" + + id: str + assignees: list[str] + area: str | None + tags: list[str] + + +class MappingRule(BaseModel): + """A single mapping rule (pattern -> bundle_id).""" + + pattern: str = Field(..., description="Pattern: tag=~regex, assignee=exact, area=exact") + bundle_id: str = Field(..., description="Target bundle id") + action: str = Field(default="assign", description="Action: assign") + confidence: float = Field(default=1.0, ge=0.0, le=1.0, description="Rule confidence") + + @beartype + def matches(self, item: _ItemLike) -> bool: + """Return True if this rule matches the item.""" + if self.pattern.startswith("tag=~"): + regex = self.pattern[5:].strip() + try: + pat = re.compile(regex) + except re.error: + return False + return any(pat.search(t) for t in item.tags) + if self.pattern.startswith("assignee="): + val = self.pattern[9:].strip() + return val in item.assignees + if self.pattern.startswith("area="): + val = self.pattern[5:].strip() + return item.area == val + return False + + +def item_key(item: _ItemLike) -> str: + """Build a stable key for history lookup (area, assignee, tags).""" + area = (item.area or "").strip() + assignee = (item.assignees[0] if item.assignees else "").strip() + tags_str = "|".join(sorted(t.strip() for t in item.tags if t)) + return f"area={area}|assignee={assignee}|tags={tags_str}" + + +def item_keys_similar(key_a: str, key_b: str) -> bool: + """Return True if keys share at least 2 of 3 components (area, assignee, tags).""" + + def parts(k: str) -> tuple[str, str, str]: + d: dict[str, str] = {} + for seg in k.split("|"): + if "=" in seg: + name, val = seg.split("=", 1) + d[name.strip()] = val.strip() + return (d.get("area", ""), d.get("assignee", ""), d.get("tags", "")) + + a1, a2, a3 = parts(key_a) + b1, b2, b3 = parts(key_b) + matches = sum([a1 == b1, a2 == b2, a3 == b3]) + return matches >= 2 + + +@beartype +@require(lambda config_path: config_path is None or config_path.exists() or not config_path.exists(), "Path valid") +@ensure(lambda result: result is None, "Returns None") +def save_user_confirmed_mapping( + item: _ItemLike, + bundle_id: str, + config_path: Path | None = None, +) -> None: + """ + Persist a user-confirmed mapping: increment history count and save to config. + + Creates item_key from item metadata, increments mapping count in history, + and writes backlog.bundle_mapping.history to config_path (or default .specfact/config.yaml). + """ + if config_path is None: + config_path = Path.home() / ".specfact" / "config.yaml" + key = item_key(item) + data: dict[str, Any] = {} + if config_path.exists(): + with open(config_path, encoding="utf-8") as f: + data = yaml.safe_load(f) or {} + backlog = data.setdefault("backlog", {}) + bm = backlog.setdefault("bundle_mapping", {}) + history = bm.setdefault("history", {}) + entry = history.setdefault(key, {}) + counts = entry.setdefault("counts", {}) + counts[bundle_id] = counts.get(bundle_id, 0) + 1 + config_path.parent.mkdir(parents=True, exist_ok=True) + with open(config_path, "w", encoding="utf-8") as f: + yaml.safe_dump(data, f, default_flow_style=False, sort_keys=False) + + +@beartype +def load_bundle_mapping_config(config_path: Path | None = None) -> dict[str, Any]: + """Load backlog.bundle_mapping section from config; return dict with rules, history, thresholds.""" + if config_path is None: + config_path = Path.home() / ".specfact" / "config.yaml" + data: dict[str, Any] = {} + if config_path.exists(): + with open(config_path, encoding="utf-8") as f: + data = yaml.safe_load(f) or {} + bm = (data.get("backlog") or {}).get("bundle_mapping") or {} + return { + "rules": bm.get("rules", []), + "history": bm.get("history", {}), + "explicit_label_prefix": bm.get("explicit_label_prefix", DEFAULT_LABEL_PREFIX), + "auto_assign_threshold": float(bm.get("auto_assign_threshold", DEFAULT_AUTO_ASSIGN_THRESHOLD)), + "confirm_threshold": float(bm.get("confirm_threshold", DEFAULT_CONFIRM_THRESHOLD)), + } diff --git a/modules/bundle-mapper/src/bundle_mapper/models/__init__.py b/modules/bundle-mapper/src/bundle_mapper/models/__init__.py new file mode 100644 index 00000000..174dc75a --- /dev/null +++ b/modules/bundle-mapper/src/bundle_mapper/models/__init__.py @@ -0,0 +1,6 @@ +"""Bundle mapper models.""" + +from bundle_mapper.models.bundle_mapping import BundleMapping + + +__all__ = ["BundleMapping"] diff --git a/modules/bundle-mapper/src/bundle_mapper/models/bundle_mapping.py b/modules/bundle-mapper/src/bundle_mapper/models/bundle_mapping.py new file mode 100644 index 00000000..c54493b4 --- /dev/null +++ b/modules/bundle-mapper/src/bundle_mapper/models/bundle_mapping.py @@ -0,0 +1,49 @@ +""" +BundleMapping result model for spec-to-bundle assignment with confidence and explanation. +""" + +from __future__ import annotations + +from beartype import beartype +from icontract import ensure +from pydantic import BaseModel, Field + + +class BundleMapping(BaseModel): + """ + Result of mapping a backlog item to an OpenSpec bundle. + + Attributes: + primary_bundle_id: Best-match bundle id, or None if no mapping. + confidence: Score in [0.0, 1.0]. + candidates: Alternative (bundle_id, score) pairs. + explained_reasoning: Human-readable rationale. + """ + + primary_bundle_id: str | None = Field( + default=None, + description="Assigned bundle id, or None if no mapping", + ) + confidence: float = Field( + default=0.0, + ge=0.0, + le=1.0, + description="Confidence score in [0.0, 1.0]", + ) + candidates: list[tuple[str, float]] = Field( + default_factory=list, + description="Alternative (bundle_id, score) pairs", + ) + explained_reasoning: str = Field( + default="", + description="Human-readable mapping rationale", + ) + + @beartype + @ensure( + lambda result: result is None or (isinstance(result, str) and len(result) >= 0), + "Return type is None or non-negative length str", + ) + def get_primary_or_none(self) -> str | None: + """Return primary_bundle_id (for compatibility with callers expecting str | None).""" + return self.primary_bundle_id diff --git a/modules/bundle-mapper/src/bundle_mapper/ui/__init__.py b/modules/bundle-mapper/src/bundle_mapper/ui/__init__.py new file mode 100644 index 00000000..63666fab --- /dev/null +++ b/modules/bundle-mapper/src/bundle_mapper/ui/__init__.py @@ -0,0 +1,6 @@ +"""Interactive UI for bundle mapping.""" + +from bundle_mapper.ui.interactive import ask_bundle_mapping + + +__all__ = ["ask_bundle_mapping"] diff --git a/modules/bundle-mapper/src/bundle_mapper/ui/interactive.py b/modules/bundle-mapper/src/bundle_mapper/ui/interactive.py new file mode 100644 index 00000000..b9f3a40e --- /dev/null +++ b/modules/bundle-mapper/src/bundle_mapper/ui/interactive.py @@ -0,0 +1,90 @@ +""" +Interactive bundle mapping UI: prompt user with confidence visualization (Rich). +""" + +from __future__ import annotations + +from beartype import beartype +from icontract import ensure, require +from rich.console import Console +from rich.panel import Panel +from rich.prompt import Prompt + +from bundle_mapper.models.bundle_mapping import BundleMapping + + +console = Console() + + +@beartype +@require(lambda mapping: mapping is not None, "Mapping must not be None") +@ensure( + lambda result: result is None or isinstance(result, str), + "Returns bundle_id or None", +) +def ask_bundle_mapping( + mapping: BundleMapping, + available_bundles: list[str] | None = None, + auto_accept_high: bool = False, +) -> str | None: + """ + Prompt user to accept or change bundle assignment. + + Displays confidence (✓ high / ? medium / ! low), suggested bundle, alternatives. + Options: accept, select from candidates, show all bundles (S), skip (Q). + Returns selected bundle_id or None if skipped. + """ + available_bundles = available_bundles or [] + conf = mapping.confidence + primary = mapping.primary_bundle_id + candidates = mapping.candidates + explanation = mapping.explained_reasoning + + if conf >= 0.8: + label = "[green]✓ HIGH CONFIDENCE[/green]" + elif conf >= 0.5: + label = "[yellow]? MEDIUM CONFIDENCE[/yellow]" + else: + label = "[red]! LOW CONFIDENCE[/red]" + + lines = [ + f"{label}", + f"Suggested bundle: [bold]{primary or '—'}[/bold]", + explanation, + ] + if candidates: + lines.append("Alternatives: " + ", ".join(f"{b} ({s:.2f})" for b, s in candidates[:5])) + + console.print(Panel("\n".join(lines), title="Bundle mapping")) + if auto_accept_high and conf >= 0.8 and primary: + return primary + + choice = ( + Prompt.ask( + "Accept (A), choose number from list (1-N), show all (S), skip (Q)", + default="A", + ) + .strip() + .upper() + ) + + if choice == "Q": + return None + if choice == "A" and primary: + return primary + if choice == "S" and available_bundles: + for i, b in enumerate(available_bundles, 1): + console.print(f" {i}. {b}") + idx = Prompt.ask("Enter number", default="1") + try: + i = int(idx) + if 1 <= i <= len(available_bundles): + return available_bundles[i - 1] + except ValueError: + pass + return None + if choice.isdigit() and candidates: + i = int(choice) + if 1 <= i <= len(candidates): + return candidates[i - 1][0] + return primary diff --git a/modules/bundle-mapper/tests/__init__.py b/modules/bundle-mapper/tests/__init__.py new file mode 100644 index 00000000..a420dbfb --- /dev/null +++ b/modules/bundle-mapper/tests/__init__.py @@ -0,0 +1 @@ +"""Bundle mapper tests.""" diff --git a/modules/bundle-mapper/tests/conftest.py b/modules/bundle-mapper/tests/conftest.py new file mode 100644 index 00000000..bfdebd12 --- /dev/null +++ b/modules/bundle-mapper/tests/conftest.py @@ -0,0 +1,10 @@ +"""Pytest conftest: add bundle_mapper src to path.""" + +import sys +from pathlib import Path + + +# modules/bundle-mapper/tests/conftest.py -> src = modules/bundle-mapper/src +_bundle_mapper_src = Path(__file__).resolve().parents[1] / "src" +if _bundle_mapper_src.exists() and str(_bundle_mapper_src) not in sys.path: + sys.path.insert(0, str(_bundle_mapper_src)) diff --git a/modules/bundle-mapper/tests/unit/__init__.py b/modules/bundle-mapper/tests/unit/__init__.py new file mode 100644 index 00000000..f3f1bc4c --- /dev/null +++ b/modules/bundle-mapper/tests/unit/__init__.py @@ -0,0 +1 @@ +"""Unit tests for bundle mapper.""" diff --git a/modules/bundle-mapper/tests/unit/test_bundle_mapper_engine.py b/modules/bundle-mapper/tests/unit/test_bundle_mapper_engine.py new file mode 100644 index 00000000..dfad84e0 --- /dev/null +++ b/modules/bundle-mapper/tests/unit/test_bundle_mapper_engine.py @@ -0,0 +1,67 @@ +"""Unit tests for BundleMapper engine.""" + +from __future__ import annotations + +from bundle_mapper.mapper.engine import BundleMapper + +from specfact_cli.models.backlog_item import BacklogItem + + +def _item( + id_: str = "1", + title: str = "Fix login", + tags: list[str] | None = None, + assignees: list[str] | None = None, + area: str | None = None, + body: str = "", +) -> BacklogItem: + return BacklogItem( + id=id_, + provider="github", + url="https://github.com/r/1", + title=title, + body_markdown=body, + state="open", + tags=tags or [], + assignees=assignees or [], + area=area, + ) + + +def test_explicit_label_valid_bundle() -> None: + mapper = BundleMapper(available_bundle_ids=["backend-services"]) + item = _item(tags=["bundle:backend-services"]) + m = mapper.compute_mapping(item) + assert m.primary_bundle_id == "backend-services" + assert m.confidence >= 0.8 + + +def test_explicit_label_invalid_bundle_ignored() -> None: + mapper = BundleMapper(available_bundle_ids=["backend-services"]) + item = _item(tags=["bundle:nonexistent"]) + m = mapper.compute_mapping(item) + assert m.primary_bundle_id is None + assert m.confidence == 0.0 + + +def test_no_signals_returns_none_zero_confidence() -> None: + mapper = BundleMapper(available_bundle_ids=[]) + item = _item(tags=[], title="Generic task") + m = mapper.compute_mapping(item) + assert m.primary_bundle_id is None + assert m.confidence == 0.0 + + +def test_confidence_in_bounds() -> None: + mapper = BundleMapper(available_bundle_ids=["b"]) + item = _item(tags=["bundle:b"]) + m = mapper.compute_mapping(item) + assert 0.0 <= m.confidence <= 1.0 + + +def test_weighted_calculation_explicit_dominates() -> None: + mapper = BundleMapper(available_bundle_ids=["backend"]) + item = _item(tags=["bundle:backend"]) + m = mapper.compute_mapping(item) + assert m.primary_bundle_id == "backend" + assert m.confidence >= 0.8 diff --git a/modules/bundle-mapper/tests/unit/test_bundle_mapping_model.py b/modules/bundle-mapper/tests/unit/test_bundle_mapping_model.py new file mode 100644 index 00000000..d4a181d5 --- /dev/null +++ b/modules/bundle-mapper/tests/unit/test_bundle_mapping_model.py @@ -0,0 +1,35 @@ +"""Unit tests for BundleMapping model.""" + +from __future__ import annotations + +import pytest +from bundle_mapper.models.bundle_mapping import BundleMapping + + +def test_bundle_mapping_defaults() -> None: + m = BundleMapping() + assert m.primary_bundle_id is None + assert m.confidence == 0.0 + assert m.candidates == [] + assert m.explained_reasoning == "" + + +def test_bundle_mapping_with_values() -> None: + m = BundleMapping( + primary_bundle_id="backend", + confidence=0.9, + candidates=[("api", 0.5)], + explained_reasoning="Explicit label", + ) + assert m.primary_bundle_id == "backend" + assert m.confidence == 0.9 + assert m.get_primary_or_none() == "backend" + + +def test_bundle_mapping_confidence_bounds() -> None: + BundleMapping(confidence=0.0) + BundleMapping(confidence=1.0) + with pytest.raises(ValueError): + BundleMapping(confidence=-0.1) + with pytest.raises(ValueError): + BundleMapping(confidence=1.1) diff --git a/modules/bundle-mapper/tests/unit/test_mapping_history.py b/modules/bundle-mapper/tests/unit/test_mapping_history.py new file mode 100644 index 00000000..3f5b3c43 --- /dev/null +++ b/modules/bundle-mapper/tests/unit/test_mapping_history.py @@ -0,0 +1,64 @@ +"""Unit tests for mapping history persistence.""" + +from __future__ import annotations + +import tempfile +from pathlib import Path + +import pytest +from bundle_mapper.mapper.history import ( + item_key, + item_keys_similar, + load_bundle_mapping_config, + save_user_confirmed_mapping, +) + +from specfact_cli.models.backlog_item import BacklogItem + + +def _item( + assignees: list[str] | None = None, + area: str | None = None, + tags: list[str] | None = None, +) -> BacklogItem: + return BacklogItem( + id="1", + provider="github", + url="https://x/1", + title="T", + state="open", + assignees=assignees or [], + area=area, + tags=tags or [], + ) + + +def test_item_key() -> None: + item = _item(assignees=["alice"], area="backend", tags=["bug"]) + k = item_key(item) + assert "alice" in k + assert "backend" in k + + +def test_item_keys_similar_two_components() -> None: + k1 = "area=be|assignee=alice|tags=a" + k2 = "area=be|assignee=alice|tags=b" + assert item_keys_similar(k1, k2) is True + + +def test_save_user_confirmed_mapping_increments_history() -> None: + with tempfile.TemporaryDirectory() as tmp: + config_path = Path(tmp) / "config.yaml" + item = _item(assignees=["bob"], area="api") + save_user_confirmed_mapping(item, "backend-services", config_path=config_path) + save_user_confirmed_mapping(item, "backend-services", config_path=config_path) + cfg = load_bundle_mapping_config(config_path=config_path) + history = cfg.get("history", {}) + assert len(history) >= 1 + for entry in history.values(): + counts = entry.get("counts", {}) + if "backend-services" in counts: + assert counts["backend-services"] == 2 + break + else: + pytest.fail("Expected backend-services in history counts") diff --git a/openspec/changes/bundle-mapper-01-mapping-strategy/TDD_EVIDENCE.md b/openspec/changes/bundle-mapper-01-mapping-strategy/TDD_EVIDENCE.md new file mode 100644 index 00000000..c05002d9 --- /dev/null +++ b/openspec/changes/bundle-mapper-01-mapping-strategy/TDD_EVIDENCE.md @@ -0,0 +1,14 @@ +# TDD Evidence: bundle-mapper-01-mapping-strategy + +## Pre-implementation (failing run) + +- **Command**: `hatch run pytest modules/bundle-mapper/tests/ -v --no-cov` +- **Timestamp**: 2026-02-18 (session) +- **Result**: Collection errors — `ModuleNotFoundError: No module named 'bundle_mapper'` (resolved by adding `conftest.py` with `sys.path.insert` for module `src`). Then `BeartypeDecorHintPep3119Exception` for `_ItemLike` Protocol (resolved by `@runtime_checkable`). + +## Post-implementation (passing run) + +- **Command**: `hatch run pytest modules/bundle-mapper/tests/ -v --no-cov` +- **Timestamp**: 2026-02-18 +- **Result**: 11 passed in 0.71s +- **Tests**: test_bundle_mapping_model (3), test_bundle_mapper_engine (5), test_mapping_history (3) From b29d44cc03eec4317a70930125616c6dbdf08026 Mon Sep 17 00:00:00 2001 From: Dominikus Nold Date: Wed, 18 Feb 2026 15:16:09 +0100 Subject: [PATCH 2/4] fix(bundle-mapper): address PR review findings (P1/P2) - P1 interactive: no default accept for low-confidence; use default only when conf >= 0.5 - P1 history: ignore empty key fields in item_keys_similar (only count non-empty matches) - P2 engine: add historical weight only when hist_bundle == primary_bundle_id - Add test_item_keys_similar_empty_fields_not_counted to lock empty-key behavior Co-authored-by: Cursor --- .../bundle-mapper/src/bundle_mapper/mapper/engine.py | 2 +- .../bundle-mapper/src/bundle_mapper/mapper/history.py | 10 ++++++++-- .../bundle-mapper/src/bundle_mapper/ui/interactive.py | 3 ++- .../bundle-mapper/tests/unit/test_mapping_history.py | 7 +++++++ 4 files changed, 18 insertions(+), 4 deletions(-) diff --git a/modules/bundle-mapper/src/bundle_mapper/mapper/engine.py b/modules/bundle-mapper/src/bundle_mapper/mapper/engine.py index 1fa30821..67592cbf 100644 --- a/modules/bundle-mapper/src/bundle_mapper/mapper/engine.py +++ b/modules/bundle-mapper/src/bundle_mapper/mapper/engine.py @@ -176,7 +176,7 @@ def compute_mapping(self, item: BacklogItem) -> BundleMapping: primary_bundle_id = hist_bundle weighted += contrib reasons.append(self._explain_score(hist_bundle, hist_score, "historical")) - else: + elif hist_bundle == primary_bundle_id: weighted += contrib if content_list: diff --git a/modules/bundle-mapper/src/bundle_mapper/mapper/history.py b/modules/bundle-mapper/src/bundle_mapper/mapper/history.py index bf224329..9a3bb1a0 100644 --- a/modules/bundle-mapper/src/bundle_mapper/mapper/history.py +++ b/modules/bundle-mapper/src/bundle_mapper/mapper/history.py @@ -65,7 +65,7 @@ def item_key(item: _ItemLike) -> str: def item_keys_similar(key_a: str, key_b: str) -> bool: - """Return True if keys share at least 2 of 3 components (area, assignee, tags).""" + """Return True if keys share at least 2 of 3 non-empty components (area, assignee, tags). Empty fields are ignored to avoid matching unrelated items.""" def parts(k: str) -> tuple[str, str, str]: d: dict[str, str] = {} @@ -77,7 +77,13 @@ def parts(k: str) -> tuple[str, str, str]: a1, a2, a3 = parts(key_a) b1, b2, b3 = parts(key_b) - matches = sum([a1 == b1, a2 == b2, a3 == b3]) + matches = 0 + if a1 and b1 and a1 == b1: + matches += 1 + if a2 and b2 and a2 == b2: + matches += 1 + if a3 and b3 and a3 == b3: + matches += 1 return matches >= 2 diff --git a/modules/bundle-mapper/src/bundle_mapper/ui/interactive.py b/modules/bundle-mapper/src/bundle_mapper/ui/interactive.py index b9f3a40e..01df76ec 100644 --- a/modules/bundle-mapper/src/bundle_mapper/ui/interactive.py +++ b/modules/bundle-mapper/src/bundle_mapper/ui/interactive.py @@ -59,10 +59,11 @@ def ask_bundle_mapping( if auto_accept_high and conf >= 0.8 and primary: return primary + prompt_default: str | None = "A" if conf >= 0.5 else None choice = ( Prompt.ask( "Accept (A), choose number from list (1-N), show all (S), skip (Q)", - default="A", + default=prompt_default, ) .strip() .upper() diff --git a/modules/bundle-mapper/tests/unit/test_mapping_history.py b/modules/bundle-mapper/tests/unit/test_mapping_history.py index 3f5b3c43..089e1015 100644 --- a/modules/bundle-mapper/tests/unit/test_mapping_history.py +++ b/modules/bundle-mapper/tests/unit/test_mapping_history.py @@ -46,6 +46,13 @@ def test_item_keys_similar_two_components() -> None: assert item_keys_similar(k1, k2) is True +def test_item_keys_similar_empty_fields_not_counted() -> None: + """Items with only empty area/assignee/tags must not be considered similar.""" + k1 = "area=|assignee=|tags=" + k2 = "area=|assignee=|tags=" + assert item_keys_similar(k1, k2) is False + + def test_save_user_confirmed_mapping_increments_history() -> None: with tempfile.TemporaryDirectory() as tmp: config_path = Path(tmp) / "config.yaml" From bb98b21467a869978b0045146247a89053efddc6 Mon Sep 17 00:00:00 2001 From: Dominikus Nold Date: Sun, 22 Feb 2026 22:15:33 +0100 Subject: [PATCH 3/4] fix: address bundle-mapper review defects with tdd evidence --- .../src/bundle_mapper/mapper/engine.py | 6 +- .../src/bundle_mapper/mapper/history.py | 69 +++++++++++++++---- .../tests/unit/test_bundle_mapper_engine.py | 50 ++++++++++++++ .../tests/unit/test_mapping_history.py | 26 +++++++ .../CHANGE_VALIDATION.md | 7 ++ .../TDD_EVIDENCE.md | 29 ++++++++ .../specs/bundle-mapping/spec.md | 13 ++++ .../specs/confidence-scoring/spec.md | 13 ++++ .../tasks.md | 11 +++ tests/unit/adapters/test_ado.py | 11 ++- tests/unit/integrations/test_specmatic.py | 14 ++-- 11 files changed, 226 insertions(+), 23 deletions(-) diff --git a/modules/bundle-mapper/src/bundle_mapper/mapper/engine.py b/modules/bundle-mapper/src/bundle_mapper/mapper/engine.py index 67592cbf..95c78b36 100644 --- a/modules/bundle-mapper/src/bundle_mapper/mapper/engine.py +++ b/modules/bundle-mapper/src/bundle_mapper/mapper/engine.py @@ -97,6 +97,8 @@ def _score_historical_mapping(self, item: BacklogItem) -> tuple[str | None, floa continue counts = entry.get("counts", {}) for bid, cnt in counts.items(): + if self._available_bundle_ids and bid not in self._available_bundle_ids: + continue if cnt > best_count: best_count = cnt best_bundle = bid @@ -182,10 +184,12 @@ def compute_mapping(self, item: BacklogItem) -> BundleMapping: if content_list: best_content = content_list[0] contrib = WEIGHT_CONTENT * best_content[1] - weighted += contrib if primary_bundle_id is None: + weighted += contrib primary_bundle_id = best_content[0] reasons.append(self._explain_score(best_content[0], best_content[1], "content_similarity")) + elif best_content[0] == primary_bundle_id: + weighted += contrib confidence = min(1.0, weighted) candidates: list[tuple[str, float]] = [] diff --git a/modules/bundle-mapper/src/bundle_mapper/mapper/history.py b/modules/bundle-mapper/src/bundle_mapper/mapper/history.py index 9a3bb1a0..bb33eb32 100644 --- a/modules/bundle-mapper/src/bundle_mapper/mapper/history.py +++ b/modules/bundle-mapper/src/bundle_mapper/mapper/history.py @@ -7,6 +7,7 @@ import re from pathlib import Path from typing import Any, Protocol, runtime_checkable +from urllib.parse import quote, unquote import yaml from beartype import beartype @@ -58,25 +59,58 @@ def matches(self, item: _ItemLike) -> bool: def item_key(item: _ItemLike) -> str: """Build a stable key for history lookup (area, assignee, tags).""" - area = (item.area or "").strip() - assignee = (item.assignees[0] if item.assignees else "").strip() - tags_str = "|".join(sorted(t.strip() for t in item.tags if t)) - return f"area={area}|assignee={assignee}|tags={tags_str}" + area = quote((item.area or "").strip(), safe="") + assignee = quote((item.assignees[0] if item.assignees else "").strip(), safe="") + # Use comma-separated, URL-encoded tag values to avoid delimiter collisions. + tags = [quote(t.strip(), safe="") for t in sorted(t.strip() for t in item.tags if t)] + tags_str = ",".join(tags) + return f"area={area};assignee={assignee};tags={tags_str}" def item_keys_similar(key_a: str, key_b: str) -> bool: """Return True if keys share at least 2 of 3 non-empty components (area, assignee, tags). Empty fields are ignored to avoid matching unrelated items.""" - def parts(k: str) -> tuple[str, str, str]: - d: dict[str, str] = {} - for seg in k.split("|"): + def _parse_key(k: str) -> tuple[str, str, str]: + # Preferred modern format: area=...;assignee=...;tags=a,b + if ";" in k: + d: dict[str, str] = {} + for seg in k.split(";"): + if "=" in seg: + name, val = seg.split("=", 1) + d[name.strip()] = val.strip() + area = unquote(d.get("area", "")) + assignee = unquote(d.get("assignee", "")) + tags_raw = d.get("tags", "") + tags = [unquote(t) for t in tags_raw.split(",") if t] + return (area, assignee, ",".join(tags)) + + # Legacy format: area=...|assignee=...|tags=a|b + d_legacy: dict[str, str] = {} + segments = k.split("|") + idx = 0 + while idx < len(segments): + seg = segments[idx] if "=" in seg: name, val = seg.split("=", 1) - d[name.strip()] = val.strip() - return (d.get("area", ""), d.get("assignee", ""), d.get("tags", "")) - - a1, a2, a3 = parts(key_a) - b1, b2, b3 = parts(key_b) + name = name.strip() + val = val.strip() + if name == "tags": + tag_parts = [val] if val else [] + j = idx + 1 + while j < len(segments) and "=" not in segments[j]: + if segments[j]: + tag_parts.append(segments[j].strip()) + j += 1 + d_legacy["tags"] = ",".join(tag_parts) + idx = j + continue + d_legacy[name] = val + idx += 1 + + return (d_legacy.get("area", ""), d_legacy.get("assignee", ""), d_legacy.get("tags", "")) + + a1, a2, a3 = _parse_key(key_a) + b1, b2, b3 = _parse_key(key_b) matches = 0 if a1 and b1 and a1 == b1: matches += 1 @@ -129,10 +163,17 @@ def load_bundle_mapping_config(config_path: Path | None = None) -> dict[str, Any with open(config_path, encoding="utf-8") as f: data = yaml.safe_load(f) or {} bm = (data.get("backlog") or {}).get("bundle_mapping") or {} + + def _safe_float(value: Any, default: float) -> float: + try: + return float(value) + except (TypeError, ValueError): + return default + return { "rules": bm.get("rules", []), "history": bm.get("history", {}), "explicit_label_prefix": bm.get("explicit_label_prefix", DEFAULT_LABEL_PREFIX), - "auto_assign_threshold": float(bm.get("auto_assign_threshold", DEFAULT_AUTO_ASSIGN_THRESHOLD)), - "confirm_threshold": float(bm.get("confirm_threshold", DEFAULT_CONFIRM_THRESHOLD)), + "auto_assign_threshold": _safe_float(bm.get("auto_assign_threshold"), DEFAULT_AUTO_ASSIGN_THRESHOLD), + "confirm_threshold": _safe_float(bm.get("confirm_threshold"), DEFAULT_CONFIRM_THRESHOLD), } diff --git a/modules/bundle-mapper/tests/unit/test_bundle_mapper_engine.py b/modules/bundle-mapper/tests/unit/test_bundle_mapper_engine.py index dfad84e0..2bf430cc 100644 --- a/modules/bundle-mapper/tests/unit/test_bundle_mapper_engine.py +++ b/modules/bundle-mapper/tests/unit/test_bundle_mapper_engine.py @@ -2,6 +2,9 @@ from __future__ import annotations +from pathlib import Path + +import yaml from bundle_mapper.mapper.engine import BundleMapper from specfact_cli.models.backlog_item import BacklogItem @@ -65,3 +68,50 @@ def test_weighted_calculation_explicit_dominates() -> None: m = mapper.compute_mapping(item) assert m.primary_bundle_id == "backend" assert m.confidence >= 0.8 + + +def test_historical_mapping_ignores_stale_bundle_ids(tmp_path: Path) -> None: + config_path = tmp_path / "config.yaml" + key = "area=backend;assignee=alice;tags=bug,login" + config_path.write_text( + yaml.safe_dump( + { + "backlog": { + "bundle_mapping": { + "history": { + key: { + "counts": { + "removed-bundle": 50, + "backend-services": 2, + } + } + } + } + } + }, + sort_keys=False, + ), + encoding="utf-8", + ) + + mapper = BundleMapper(available_bundle_ids=["backend-services"], config_path=config_path) + item = _item(assignees=["alice"], area="backend", tags=["bug", "login"]) + mapping = mapper.compute_mapping(item) + + assert mapping.primary_bundle_id == "backend-services" + + +def test_conflicting_content_signal_does_not_increase_primary_confidence() -> None: + mapper = BundleMapper( + available_bundle_ids=["alpha", "beta"], + bundle_spec_keywords={"beta": {"beta"}}, + ) + item = _item( + tags=["bundle:alpha"], + title="beta", + ) + + mapping = mapper.compute_mapping(item) + + assert mapping.primary_bundle_id == "alpha" + assert mapping.confidence == 0.8 diff --git a/modules/bundle-mapper/tests/unit/test_mapping_history.py b/modules/bundle-mapper/tests/unit/test_mapping_history.py index 089e1015..291c65b7 100644 --- a/modules/bundle-mapper/tests/unit/test_mapping_history.py +++ b/modules/bundle-mapper/tests/unit/test_mapping_history.py @@ -69,3 +69,29 @@ def test_save_user_confirmed_mapping_increments_history() -> None: break else: pytest.fail("Expected backend-services in history counts") + + +def test_item_key_similarity_does_not_false_match_tag_lists() -> None: + k1 = item_key(_item(assignees=["alice"], area="api", tags=["a", "b"])) + k2 = item_key(_item(assignees=["alice"], area="web", tags=["a"])) + + assert item_keys_similar(k1, k2) is False + + +def test_load_bundle_mapping_config_malformed_thresholds_use_defaults(tmp_path: Path) -> None: + config_path = tmp_path / "config.yaml" + config_path.write_text( + """ +backlog: + bundle_mapping: + auto_assign_threshold: high + confirm_threshold: medium +""".strip() + + "\n", + encoding="utf-8", + ) + + cfg = load_bundle_mapping_config(config_path=config_path) + + assert cfg["auto_assign_threshold"] == 0.8 + assert cfg["confirm_threshold"] == 0.5 diff --git a/openspec/changes/bundle-mapper-01-mapping-strategy/CHANGE_VALIDATION.md b/openspec/changes/bundle-mapper-01-mapping-strategy/CHANGE_VALIDATION.md index c7980830..46ccf56b 100644 --- a/openspec/changes/bundle-mapper-01-mapping-strategy/CHANGE_VALIDATION.md +++ b/openspec/changes/bundle-mapper-01-mapping-strategy/CHANGE_VALIDATION.md @@ -107,3 +107,10 @@ This change was re-validated after renaming and updating to align with the modul - All old change ID references updated to new module-scoped naming **Result**: Pass — format compliant, module architecture aligned, no breaking changes introduced. + +## Remediation Re-Validation (2026-02-22) + +- Scope: review defect remediation for stale historical bundle IDs, history key encoding ambiguity, conflicting content contribution, and malformed threshold parsing. +- Validation command: `openspec validate bundle-mapper-01-mapping-strategy --strict` +- Result: `Change 'bundle-mapper-01-mapping-strategy' is valid` +- Notes: telemetry flush warnings were emitted due restricted network (`edge.openspec.dev`) but validation completed successfully with exit code 0. diff --git a/openspec/changes/bundle-mapper-01-mapping-strategy/TDD_EVIDENCE.md b/openspec/changes/bundle-mapper-01-mapping-strategy/TDD_EVIDENCE.md index c05002d9..dfa51106 100644 --- a/openspec/changes/bundle-mapper-01-mapping-strategy/TDD_EVIDENCE.md +++ b/openspec/changes/bundle-mapper-01-mapping-strategy/TDD_EVIDENCE.md @@ -1,5 +1,12 @@ # TDD Evidence: bundle-mapper-01-mapping-strategy +## Review findings intake (2026-02-22) + +- Historical scorer may choose stale bundle IDs not present in current `available_bundle_ids`. +- History key format is ambiguous because `|` is used for both field and tag separators. +- Content signal can boost confidence even when it points to a different bundle than the selected primary bundle. +- Threshold parsing crashes on malformed user config values instead of falling back to defaults. + ## Pre-implementation (failing run) - **Command**: `hatch run pytest modules/bundle-mapper/tests/ -v --no-cov` @@ -12,3 +19,25 @@ - **Timestamp**: 2026-02-18 - **Result**: 11 passed in 0.71s - **Tests**: test_bundle_mapping_model (3), test_bundle_mapper_engine (5), test_mapping_history (3) + +## Pre-implementation (review-defect regression tests) + +- **Command**: `hatch run pytest modules/bundle-mapper/tests/unit/test_bundle_mapper_engine.py modules/bundle-mapper/tests/unit/test_mapping_history.py -q` +- **Timestamp**: 2026-02-22 +- **Result**: 4 failed, 9 passed +- **Failure summary**: + - `test_historical_mapping_ignores_stale_bundle_ids`: primary mapping was `None`/invalid due to stale history IDs + - `test_conflicting_content_signal_does_not_increase_primary_confidence`: confidence was `0.85` instead of `0.80` + - `test_item_key_similarity_does_not_false_match_tag_lists`: returned false-positive similarity (`True`) + - `test_load_bundle_mapping_config_malformed_thresholds_use_defaults`: `ValueError` raised for non-numeric thresholds + +## Post-implementation (review-defect regression tests) + +- **Command**: `hatch run pytest modules/bundle-mapper/tests/unit/test_bundle_mapper_engine.py modules/bundle-mapper/tests/unit/test_mapping_history.py -q` +- **Timestamp**: 2026-02-22 +- **Result**: 13 passed in 0.75s +- **Tests**: + - stale historical bundle IDs are ignored during scoring + - unambiguous history key serialization preserves tag semantics + - conflicting content signal does not boost different primary bundle confidence + - malformed thresholds fall back to defaults diff --git a/openspec/changes/bundle-mapper-01-mapping-strategy/specs/bundle-mapping/spec.md b/openspec/changes/bundle-mapper-01-mapping-strategy/specs/bundle-mapping/spec.md index bdc04a04..4c36543d 100644 --- a/openspec/changes/bundle-mapper-01-mapping-strategy/specs/bundle-mapping/spec.md +++ b/openspec/changes/bundle-mapper-01-mapping-strategy/specs/bundle-mapping/spec.md @@ -62,11 +62,24 @@ The system SHALL persist mapping rules learned from user confirmations. - **WHEN** a new item matches historical pattern (same assignee, area, tags) - **THEN** the system uses historical mapping frequency to boost confidence score +#### Scenario: Historical mapping ignores stale bundle ids + +- **GIVEN** history contains bundle ids that are no longer present in available bundles +- **WHEN** historical scoring is computed +- **THEN** stale bundle ids are ignored +- **AND** returned historical bundle ids are always members of current available bundles + #### Scenario: Mapping rules from config - **WHEN** config file contains mapping rules (e.g., "assignee=alice → backend-services") - **THEN** the system applies these rules before computing other signals +#### Scenario: History key encoding is unambiguous + +- **WHEN** item keys are serialized for history matching +- **THEN** field delimiters and tag-value delimiters do not collide +- **AND** round-trip parsing preserves all tag values without truncation + ### Requirement: Interactive Mapping UI The system SHALL provide an interactive prompt for bundle selection with confidence visualization and candidate options. diff --git a/openspec/changes/bundle-mapper-01-mapping-strategy/specs/confidence-scoring/spec.md b/openspec/changes/bundle-mapper-01-mapping-strategy/specs/confidence-scoring/spec.md index a6f0d975..ff011847 100644 --- a/openspec/changes/bundle-mapper-01-mapping-strategy/specs/confidence-scoring/spec.md +++ b/openspec/changes/bundle-mapper-01-mapping-strategy/specs/confidence-scoring/spec.md @@ -62,6 +62,13 @@ The system SHALL score content similarity between item text and existing specs i - **WHEN** item text has no keywords in common with bundle specs - **THEN** the system assigns score 0.0 for that bundle +#### Scenario: Conflicting content signal does not increase confidence + +- **GIVEN** explicit or historical scoring selected a primary bundle +- **AND** top content similarity points to a different bundle +- **WHEN** final confidence is calculated +- **THEN** the content contribution is not added to the selected primary bundle confidence + #### Scenario: Tokenization for matching - **WHEN** content similarity is computed @@ -90,3 +97,9 @@ The system SHALL use configurable confidence thresholds for routing decisions. - **WHEN** user configures custom thresholds in `.specfact/config.yaml` - **THEN** the system uses custom thresholds instead of defaults + +#### Scenario: Malformed thresholds fall back to defaults + +- **WHEN** config contains non-numeric threshold values +- **THEN** mapper initialization does not fail +- **AND** default threshold values are used diff --git a/openspec/changes/bundle-mapper-01-mapping-strategy/tasks.md b/openspec/changes/bundle-mapper-01-mapping-strategy/tasks.md index 9c33127e..bcccb23d 100644 --- a/openspec/changes/bundle-mapper-01-mapping-strategy/tasks.md +++ b/openspec/changes/bundle-mapper-01-mapping-strategy/tasks.md @@ -134,6 +134,17 @@ - [ ] 12.4.4 Run `hatch test --cover -v` one final time - [ ] 12.4.5 Verify no errors remain (formatting, linting, type-checking, tests) +## 12R. Review Defect Remediation (2026-02-22) + +- [x] 12R.1 Add regression tests first (must fail before implementation) + - [x] 12R.1.1 Historical scoring ignores stale bundle IDs not present in available bundles + - [x] 12R.1.2 History key encoding is unambiguous and does not lose tag values + - [x] 12R.1.3 Conflicting content signal does not boost confidence for another primary bundle + - [x] 12R.1.4 Malformed threshold config values fall back to defaults without crashing +- [x] 12R.2 Record failing run in `TDD_EVIDENCE.md` with command, timestamp, and failure summary +- [x] 12R.3 Implement production fixes in mapper/history modules +- [x] 12R.4 Re-run regression tests and record passing run in `TDD_EVIDENCE.md` + ## 13. OpenSpec Validation - [ ] 13.1 Validate change proposal diff --git a/tests/unit/adapters/test_ado.py b/tests/unit/adapters/test_ado.py index 8292f31a..cdf8679c 100644 --- a/tests/unit/adapters/test_ado.py +++ b/tests/unit/adapters/test_ado.py @@ -245,11 +245,20 @@ def test_update_work_item_status( @beartype @patch("specfact_cli.adapters.ado.requests.patch") @patch("specfact_cli.adapters.ado.requests.get") - def test_missing_api_token(self, mock_get: MagicMock, mock_patch: MagicMock, bridge_config: BridgeConfig) -> None: + @patch("specfact_cli.adapters.ado.get_token") + def test_missing_api_token( + self, + mock_get_token: MagicMock, + mock_get: MagicMock, + mock_patch: MagicMock, + bridge_config: BridgeConfig, + ) -> None: """Test error when API token is missing.""" # Clear environment variable BEFORE creating adapter old_token = os.environ.pop("AZURE_DEVOPS_TOKEN", None) try: + # Ensure adapter cannot resolve token from persisted auth cache. + mock_get_token.return_value = None adapter = AdoAdapter(org="test-org", project="test-project", api_token=None) # Mock process template API call (called by _get_work_item_type) diff --git a/tests/unit/integrations/test_specmatic.py b/tests/unit/integrations/test_specmatic.py index 7f38a48b..0d977179 100644 --- a/tests/unit/integrations/test_specmatic.py +++ b/tests/unit/integrations/test_specmatic.py @@ -128,7 +128,7 @@ def test_to_json(self): class TestValidateSpecWithSpecmatic: """Test suite for validate_spec_with_specmatic function.""" - @pytest.mark.asyncio + @pytest.mark.anyio @patch("specfact_cli.integrations.specmatic._get_specmatic_command") @patch("specfact_cli.integrations.specmatic.asyncio.to_thread") async def test_validate_success(self, mock_to_thread, mock_get_cmd, tmp_path): @@ -150,7 +150,7 @@ async def test_validate_success(self, mock_to_thread, mock_get_cmd, tmp_path): assert result.examples_valid is True assert mock_to_thread.call_count == 2 # Schema validation + examples - @pytest.mark.asyncio + @pytest.mark.anyio @patch("specfact_cli.integrations.specmatic._get_specmatic_command") async def test_validate_specmatic_not_available(self, mock_get_cmd, tmp_path): """Test when Specmatic is not available.""" @@ -166,7 +166,7 @@ async def test_validate_specmatic_not_available(self, mock_get_cmd, tmp_path): assert result.examples_valid is False assert "Specmatic" in result.errors[0] and "not available" in result.errors[0] - @pytest.mark.asyncio + @pytest.mark.anyio @patch("specfact_cli.integrations.specmatic._get_specmatic_command") @patch("specfact_cli.integrations.specmatic.asyncio.to_thread") async def test_validate_with_previous_version(self, mock_to_thread, mock_get_cmd, tmp_path): @@ -193,7 +193,7 @@ async def test_validate_with_previous_version(self, mock_to_thread, mock_get_cmd class TestCheckBackwardCompatibility: """Test suite for check_backward_compatibility function.""" - @pytest.mark.asyncio + @pytest.mark.anyio @patch("specfact_cli.integrations.specmatic._get_specmatic_command") @patch("specfact_cli.integrations.specmatic.asyncio.to_thread") async def test_backward_compatible(self, mock_to_thread, mock_get_cmd, tmp_path): @@ -213,7 +213,7 @@ async def test_backward_compatible(self, mock_to_thread, mock_get_cmd, tmp_path) assert is_compatible is True assert breaking_changes == [] - @pytest.mark.asyncio + @pytest.mark.anyio @patch("specfact_cli.integrations.specmatic._get_specmatic_command") @patch("specfact_cli.integrations.specmatic.asyncio.to_thread") async def test_backward_incompatible(self, mock_to_thread, mock_get_cmd, tmp_path): @@ -242,7 +242,7 @@ async def test_backward_incompatible(self, mock_to_thread, mock_get_cmd, tmp_pat class TestGenerateSpecmaticTests: """Test suite for generate_specmatic_tests function.""" - @pytest.mark.asyncio + @pytest.mark.anyio @patch("specfact_cli.integrations.specmatic._get_specmatic_command") @patch("specfact_cli.integrations.specmatic.asyncio.to_thread") async def test_generate_tests_success(self, mock_to_thread, mock_get_cmd, tmp_path): @@ -264,7 +264,7 @@ async def test_generate_tests_success(self, mock_to_thread, mock_get_cmd, tmp_pa class TestCreateMockServer: """Test suite for create_mock_server function.""" - @pytest.mark.asyncio + @pytest.mark.anyio @patch("builtins.__import__") @patch("specfact_cli.integrations.specmatic._get_specmatic_command") @patch("specfact_cli.integrations.specmatic.asyncio.to_thread") From d2842a604cf13951694d1ad7dcfa29c850e34abd Mon Sep 17 00:00:00 2001 From: Dominikus Nold Date: Sun, 22 Feb 2026 22:47:13 +0100 Subject: [PATCH 4/4] test: make specmatic integration tests plugin-agnostic --- tests/unit/integrations/test_specmatic.py | 38 +++++++++-------------- 1 file changed, 15 insertions(+), 23 deletions(-) diff --git a/tests/unit/integrations/test_specmatic.py b/tests/unit/integrations/test_specmatic.py index 0d977179..efa51ab7 100644 --- a/tests/unit/integrations/test_specmatic.py +++ b/tests/unit/integrations/test_specmatic.py @@ -1,9 +1,8 @@ """Unit tests for Specmatic integration.""" +import asyncio from unittest.mock import MagicMock, patch -import pytest - from specfact_cli.integrations.specmatic import ( SpecValidationResult, check_backward_compatibility, @@ -128,10 +127,9 @@ def test_to_json(self): class TestValidateSpecWithSpecmatic: """Test suite for validate_spec_with_specmatic function.""" - @pytest.mark.anyio @patch("specfact_cli.integrations.specmatic._get_specmatic_command") @patch("specfact_cli.integrations.specmatic.asyncio.to_thread") - async def test_validate_success(self, mock_to_thread, mock_get_cmd, tmp_path): + def test_validate_success(self, mock_to_thread, mock_get_cmd, tmp_path): """Test successful validation.""" # Mock specmatic command mock_get_cmd.return_value = ["specmatic"] @@ -143,33 +141,31 @@ async def test_validate_success(self, mock_to_thread, mock_get_cmd, tmp_path): spec_path = tmp_path / "openapi.yaml" spec_path.write_text("openapi: 3.0.0\n") - result = await validate_spec_with_specmatic(spec_path) + result = asyncio.run(validate_spec_with_specmatic(spec_path)) assert result.is_valid is True assert result.schema_valid is True assert result.examples_valid is True assert mock_to_thread.call_count == 2 # Schema validation + examples - @pytest.mark.anyio @patch("specfact_cli.integrations.specmatic._get_specmatic_command") - async def test_validate_specmatic_not_available(self, mock_get_cmd, tmp_path): + def test_validate_specmatic_not_available(self, mock_get_cmd, tmp_path): """Test when Specmatic is not available.""" mock_get_cmd.return_value = None spec_path = tmp_path / "openapi.yaml" spec_path.write_text("openapi: 3.0.0\n") - result = await validate_spec_with_specmatic(spec_path) + result = asyncio.run(validate_spec_with_specmatic(spec_path)) assert result.is_valid is False assert result.schema_valid is False assert result.examples_valid is False assert "Specmatic" in result.errors[0] and "not available" in result.errors[0] - @pytest.mark.anyio @patch("specfact_cli.integrations.specmatic._get_specmatic_command") @patch("specfact_cli.integrations.specmatic.asyncio.to_thread") - async def test_validate_with_previous_version(self, mock_to_thread, mock_get_cmd, tmp_path): + def test_validate_with_previous_version(self, mock_to_thread, mock_get_cmd, tmp_path): """Test validation with previous version for backward compatibility.""" mock_get_cmd.return_value = ["specmatic"] # Mock successful subprocess runs @@ -183,7 +179,7 @@ async def test_validate_with_previous_version(self, mock_to_thread, mock_get_cmd previous_path = tmp_path / "openapi.v1.yaml" previous_path.write_text("openapi: 3.0.0\n") - result = await validate_spec_with_specmatic(spec_path, previous_path) + result = asyncio.run(validate_spec_with_specmatic(spec_path, previous_path)) assert result.is_valid is True assert result.backward_compatible is True @@ -193,10 +189,9 @@ async def test_validate_with_previous_version(self, mock_to_thread, mock_get_cmd class TestCheckBackwardCompatibility: """Test suite for check_backward_compatibility function.""" - @pytest.mark.anyio @patch("specfact_cli.integrations.specmatic._get_specmatic_command") @patch("specfact_cli.integrations.specmatic.asyncio.to_thread") - async def test_backward_compatible(self, mock_to_thread, mock_get_cmd, tmp_path): + def test_backward_compatible(self, mock_to_thread, mock_get_cmd, tmp_path): """Test when specs are backward compatible.""" mock_get_cmd.return_value = ["specmatic"] # Mock successful backward compatibility check @@ -208,15 +203,14 @@ async def test_backward_compatible(self, mock_to_thread, mock_get_cmd, tmp_path) new_spec = tmp_path / "new.yaml" new_spec.write_text("openapi: 3.0.0\n") - is_compatible, breaking_changes = await check_backward_compatibility(old_spec, new_spec) + is_compatible, breaking_changes = asyncio.run(check_backward_compatibility(old_spec, new_spec)) assert is_compatible is True assert breaking_changes == [] - @pytest.mark.anyio @patch("specfact_cli.integrations.specmatic._get_specmatic_command") @patch("specfact_cli.integrations.specmatic.asyncio.to_thread") - async def test_backward_incompatible(self, mock_to_thread, mock_get_cmd, tmp_path): + def test_backward_incompatible(self, mock_to_thread, mock_get_cmd, tmp_path): """Test when specs are not backward compatible.""" mock_get_cmd.return_value = ["specmatic"] # Mock failed backward compatibility check with breaking changes in output @@ -232,7 +226,7 @@ async def test_backward_incompatible(self, mock_to_thread, mock_get_cmd, tmp_pat new_spec = tmp_path / "new.yaml" new_spec.write_text("openapi: 3.0.0\n") - is_compatible, breaking_changes = await check_backward_compatibility(old_spec, new_spec) + is_compatible, breaking_changes = asyncio.run(check_backward_compatibility(old_spec, new_spec)) assert is_compatible is False assert len(breaking_changes) > 0 @@ -242,10 +236,9 @@ async def test_backward_incompatible(self, mock_to_thread, mock_get_cmd, tmp_pat class TestGenerateSpecmaticTests: """Test suite for generate_specmatic_tests function.""" - @pytest.mark.anyio @patch("specfact_cli.integrations.specmatic._get_specmatic_command") @patch("specfact_cli.integrations.specmatic.asyncio.to_thread") - async def test_generate_tests_success(self, mock_to_thread, mock_get_cmd, tmp_path): + def test_generate_tests_success(self, mock_to_thread, mock_get_cmd, tmp_path): """Test successful test generation.""" mock_get_cmd.return_value = ["specmatic"] mock_result = MagicMock(returncode=0, stderr="") @@ -255,7 +248,7 @@ async def test_generate_tests_success(self, mock_to_thread, mock_get_cmd, tmp_pa spec_path.write_text("openapi: 3.0.0\n") output_dir = tmp_path / "tests" - output = await generate_specmatic_tests(spec_path, output_dir) + output = asyncio.run(generate_specmatic_tests(spec_path, output_dir)) assert output == output_dir mock_to_thread.assert_called_once() @@ -264,12 +257,11 @@ async def test_generate_tests_success(self, mock_to_thread, mock_get_cmd, tmp_pa class TestCreateMockServer: """Test suite for create_mock_server function.""" - @pytest.mark.anyio @patch("builtins.__import__") @patch("specfact_cli.integrations.specmatic._get_specmatic_command") @patch("specfact_cli.integrations.specmatic.asyncio.to_thread") @patch("specfact_cli.integrations.specmatic.asyncio.sleep") - async def test_create_mock_server(self, mock_sleep, mock_to_thread, mock_get_cmd, mock_import, tmp_path): + def test_create_mock_server(self, mock_sleep, mock_to_thread, mock_get_cmd, mock_import, tmp_path): """Test mock server creation.""" import socket as real_socket @@ -307,7 +299,7 @@ def import_side_effect(name, *args, **kwargs): spec_path = tmp_path / "openapi.yaml" spec_path.write_text("openapi: 3.0.0\n") - mock_server = await create_mock_server(spec_path, port=9000, strict_mode=True) + mock_server = asyncio.run(create_mock_server(spec_path, port=9000, strict_mode=True)) assert mock_server.port == 9000 assert mock_server.spec_path == spec_path