-
Notifications
You must be signed in to change notification settings - Fork 646
Fix(cli): detect resolve intent and report intent_source #1006
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,7 @@ | |
|
|
||
| import hashlib | ||
| import json | ||
| import re | ||
| from typing import Any | ||
|
|
||
| from domain.ontology import ( | ||
|
|
@@ -172,6 +173,119 @@ | |
| } | ||
|
|
||
|
|
||
| # --- Deterministic intent detection ----------------------------------------- | ||
| # A dependency-free keyword/phrase matcher over the free-text task, mirroring | ||
| # the "no-guess" discipline of ``domain.ontology_classifier``: when nothing | ||
| # matches confidently it returns ``None`` (the caller surfaces that as the | ||
| # neutral ``unknown`` intent) rather than guessing an intent. The tables are | ||
| # seeded from the natural-language trigger phrases in each recipe's ``"when"`` | ||
| # field (see :data:`CONTEXT_RESOLVE_RECIPES`). ``unknown`` is intentionally | ||
| # absent — it is the fallback, never a detected result. | ||
| # | ||
| # Detection runs *before* ``normalize_context_intent`` and does not modify the | ||
| # intent vocabulary, recipes, or default-include map. | ||
| _INTENT_KEYWORDS: dict[str, tuple[str, ...]] = { | ||
| "security": ( | ||
| "vulnerability", | ||
| "vulnerabilities", | ||
| "cve", | ||
| "exploit", | ||
| "security", | ||
| "audit", | ||
| "hardening", | ||
| "injection", | ||
| "auth bypass", | ||
| ), | ||
| "debugging": ( | ||
| "bug", | ||
| "error", | ||
| "errors", | ||
| "crash", | ||
| "crashing", | ||
| "incident", | ||
| "failing", | ||
| "failure", | ||
| "exception", | ||
| "traceback", | ||
| "stack trace", | ||
| "flaky", | ||
| "500", | ||
| "throwing", | ||
| "broken", | ||
| ), | ||
| "operations": ( | ||
| "deploy", | ||
| "deployment", | ||
| "runbook", | ||
| "production", | ||
| "rollback", | ||
| "outage", | ||
| "on-call", | ||
| "oncall", | ||
| "alert", | ||
| ), | ||
| "refactor": ( | ||
| "refactor", | ||
| "technical debt", | ||
| "tech debt", | ||
| "migrate", | ||
| "migration", | ||
| "cleanup", | ||
| "clean up", | ||
| "restructure", | ||
| ), | ||
| "test": ("test", "tests", "pytest", "coverage", "test suite"), | ||
| "review": ( | ||
| "review", | ||
| "pull request", | ||
| "what changed", | ||
| "recently", | ||
| "recent changes", | ||
| "latest changes", | ||
| "risky", | ||
| ), | ||
| "docs": ("documentation", "docs", "readme", "document"), | ||
| "onboarding": ( | ||
| "onboard", | ||
| "onboarding", | ||
| "unfamiliar", | ||
| "getting started", | ||
| "new to", | ||
| "up to speed", | ||
| ), | ||
| "planning": ("roadmap", "sprint", "architecture", "design doc", "milestone"), | ||
| "feature": ("feature", "implement", "add support", "endpoint", "behavior change"), | ||
| } | ||
|
|
||
|
|
||
| _INTENT_DETECTION_PRIORITY: tuple[str, ...] = tuple(_INTENT_KEYWORDS) | ||
|
|
||
| _INTENT_MATCHERS: dict[str, re.Pattern[str]] = { | ||
| intent: re.compile( | ||
| r"|".join(r"\b" + re.escape(kw) + r"\b" for kw in keywords), | ||
| re.IGNORECASE, | ||
| ) | ||
| for intent, keywords in _INTENT_KEYWORDS.items() | ||
| } | ||
|
|
||
|
|
||
| def detect_context_intent(task: str | None) -> str | None: | ||
| """Map a free-text task to a canonical intent, or ``None`` if unsure. | ||
|
|
||
| Deterministic, case-insensitive, word-boundary keyword match. When the | ||
| task triggers more than one intent, :data:`_INTENT_DETECTION_PRIORITY` | ||
| breaks the tie (most specific / highest-risk first). Returns ``None`` when | ||
| nothing matches — favouring "no guess" over a wrong guess — which the CLI | ||
| surfaces as the neutral ``unknown`` intent with ``intent_source=default``. | ||
| """ | ||
| if not task or not task.strip(): | ||
| return None | ||
| for intent in _INTENT_DETECTION_PRIORITY: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice work! I noticed there's good coverage for representative mappings and the fallback behavior, but I couldn't find a regression test for a task matching multiple intents. Since
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch, added. It pins four multi-match phrases ( security > debugging > operations > review > feature), and each case also guards-asserts the lower-priority intents genuinely match the phrase, so a future keyword change can't silently reduce the test to a single-match. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for adding it! I like the extra guard assertions—they make the priority behavior much clearer and help ensure the test stays meaningful over time. |
||
| if _INTENT_MATCHERS[intent].search(task): | ||
| return intent | ||
| return None | ||
|
|
||
|
|
||
| def normalize_context_intent(intent: str | None) -> str: | ||
| value = (intent or "unknown").strip().lower() | ||
| return value if value in CONTEXT_INTENTS else "unknown" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| """CLI ``resolve`` intent selection + intent_source surfacing (issue #996).""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from types import SimpleNamespace | ||
|
|
||
| import pytest | ||
|
|
||
| from adapters.inbound.cli.commands.query import ( | ||
| _envelope_human, | ||
| _envelope_payload, | ||
| _select_intent, | ||
| ) | ||
|
|
||
| pytestmark = pytest.mark.unit | ||
|
|
||
|
|
||
| def _fake_env(*, intent: str, intent_source: str) -> SimpleNamespace: | ||
| return SimpleNamespace( | ||
| pot_id="pot_1", | ||
| intent=intent, | ||
| overall_confidence="low", | ||
| items=(), | ||
| coverage=(), | ||
| unsupported_includes=(), | ||
| metadata={"intent_source": intent_source}, | ||
| ) | ||
|
|
||
|
|
||
| def test_select_intent_explicit_flag_wins() -> None: | ||
| assert _select_intent("debugging", "some unrelated task") == ( | ||
| "debugging", | ||
| "explicit", | ||
| ) | ||
|
|
||
|
|
||
| def test_select_intent_detects_from_task() -> None: | ||
| assert _select_intent(None, "the service is throwing a 500 error") == ( | ||
| "debugging", | ||
| "detected", | ||
| ) | ||
|
|
||
|
|
||
| def test_select_intent_falls_back_to_unknown_default() -> None: | ||
| assert _select_intent(None, "the quick brown fox") == ("unknown", "default") | ||
|
|
||
|
|
||
| def test_envelope_payload_exposes_intent_source() -> None: | ||
| payload = _envelope_payload(_fake_env(intent="debugging", intent_source="detected")) | ||
| assert payload["intent"] == "debugging" | ||
| assert payload["intent_source"] == "detected" | ||
|
|
||
|
|
||
| def test_envelope_human_header_shows_source() -> None: | ||
| header = _envelope_human(_fake_env(intent="unknown", intent_source="default")) | ||
| assert "intent=unknown (source=default)" in header | ||
|
|
||
|
|
||
| def test_envelope_payload_defaults_source_when_metadata_absent() -> None: | ||
| env = _fake_env(intent="feature", intent_source="detected") | ||
| env.metadata = {} | ||
| assert _envelope_payload(env)["intent_source"] == "default" |
Uh oh!
There was an error while loading. Please reload this page.