busya/ss-tools
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

tasks 033 updated

Browse Source
This commit is contained in:
busya
2026-06-30 19:05:17 +03:00
parent f1810090b6
commit e174c11d4a
23 changed files with 1976 additions and 213 deletions
Download Patch File Download Diff File Expand all files Collapse all files

422
docs/guardrails/intent-keyword-guardrail-algorithm.md Normal file
View File

@@ -0,0 +1,422 @@
# Intent Keyword Guardrail Algorithm — Audit Document
> **Purpose**: Full specification of the substring-based intent-matching algorithm used by the superset-tools agent chat to select LangChain tools. This document captures the algorithm, all known vulnerabilities, the applied fix, remaining risks, and the orthogonal test matrix — for LLM audit and architectural review.
**Created**: 2026-06-30
**Last revised**: 2026-06-30 (full-tool-set architecture)
**Status**: Active — `get_tools_for_query` retired from agent handler, kept for HITL fallback
**Affected files**:
- `backend/src/agent/tools.py` — `get_tools_for_query()`
- `backend/src/agent/_tool_resolver.py` — `infer_tool_from_text()`, `fast_confirmation_tool()`
- `backend/src/agent/app.py` — caller (fix applied)
- `backend/src/api/routes/assistant/_command_parser.py` — `_parse_command()` (legacy)
- `backend/src/agent/_persistence.py` — `prefetch_dashboards()`, `detect_message_state()`
---
## 1. Algorithm Overview
### 1.1 Purpose
~~The agent uses keyword substring matching to select which LangChain `@tool` functions to expose to the LLM.~~ **ARCHITECTURE CHANGE (2026-06-30):** Gemma's context window is now sufficient to accommodate all 24 tool schemas. The agent handler (`app.py`) now passes the full tool catalog via `get_all_tools()` directly to `create_agent()`. Intent-based subset filtering (`get_tools_for_query`) is **retired from the main agent flow**.
**What remains active:**
- `infer_tool_from_text()` / `fast_confirmation_tool()` — HITL fast-track confirmation for read-only tools
- `prefetch_dashboards()` — pre-loads dashboard data into context so the LLM doesn't need to call `search_dashboards`
- `get_tools_for_query()` — code preserved, not called from handler. Available for future context-constrained scenarios.
- `_parse_command()` — legacy REST parser (separate code path)
### 1.2 Architecture
```
User message (text)
│
▼
┌─────────────────────────────────────────────────────────────┐
│ app.py: Handler │
│ │
│ 1. Truncation (>100K chars) │
│ 2. File upload parsing → text += file content │
│ 3. fast_confirmation_tool(user_message_text) ← fast-track │
│ 4. Dashboard prefetch → text += [PRE-FETCHED DATA] │
│ 5. agent_tools = get_all_tools() ← ALL 24 tools │
│ 6. create_agent(agent_tools) → astream_events() │
│ │
│ user_message_text isolates original user intent from │
│ system-injected text (prefetch, file content, truncation) │
└─────────────────────────────────────────────────────────────┘
```
### 1.3 Three Intent-Matching Functions
| Function | File | Pattern | Purpose |
|----------|------|---------|---------|
| `get_tools_for_query()` | `tools.py:1071` | Independent `if`s | Select tool subset for agent creation |
| `infer_tool_from_text()` | `_tool_resolver.py:122` | `elif` chain | Infer single tool from user text (fallback) |
| `_parse_command()` | `_command_parser.py:28` | `if`/`elif` chain | Legacy REST parser (separate code path) |
---
## 2. Keyword Lists — Full Canonical Reference
### 2.1 `get_tools_for_query` (tools.py:1071-1150)
Independent `if` blocks — ALL matching intents accumulate. Exception: `show_capabilities` early return.
```python
# EARLY RETURN — if matched, returns ONLY [show_capabilities]
["инструмент", "tool", "capabilit", "умеешь", "можешь"]
# Independent ifs — all that match are added:
["дашборд", "dashboard", "dashboards", "дашборды"] → search_dashboards (if !prefetch)
["здоров", "health", "статус системы", "system status"] → get_health_summary
["окруж", "environment", "env"] → list_environments
["задач", "task", "таск"] → get_task_status
["llm", "provider", "провайдер", "модель"] → list_llm_providers, get_llm_status
["branch", "ветк"] → create_branch
["commit", "коммит"] → commit_changes
["deploy", "депло", "разверн"] → deploy_dashboard
["миграц", "migration", "migrate"] → execute_migration
["backup", "бэкап", "резерв"] → run_backup
["валидац", "validation", "validate"] → run_llm_validation
["документ", "documentation", "docs"] → run_llm_documentation
["maintenance", "обслуж", "баннер"] → list_maintenance_events, start_maintenance, end_maintenance
["sql", "запрос", "select", "query"] → superset_execute_sql
["форматировать sql", "format sql", "формат sql"] → superset_format_sql
["схем", "schema", "таблиц", "table", "колонк", "column",
"select star", "метаданные", "metadata"] → superset_explore_database
["аудит", "audit", "прав", "permission", "доступ", "access"]→ superset_audit_permissions
["создать дашборд", "create dashboard",
"новый дашборд", "new dashboard"] → superset_create_dashboard
["копировать дашборд", "copy dashboard",
"дублировать дашборд"] → superset_copy_dashboard
["создать датасет", "create dataset",
"новый датасет", "new dataset"] → superset_create_dataset
# Fallback if no intent matched:
→ [search_dashboards, get_health_summary, list_environments, get_task_status]
```
### 2.2 `infer_tool_from_text` (_tool_resolver.py:122-163)
`elif` chain — FIRST match wins, rest are skipped.
```
Line 125: "окруж", "environment", "env" → list_environments
Line 127: "maintenance", "обслуж", "баннер"
sub: "start", "созда", "запусти", "начни" → start_maintenance
sub: "end", "закрой", "заверши", "останов" → end_maintenance
else → list_maintenance_events
Line 134: "дашборд", "dashboard", "dashboards", "дашборды" → search_dashboards
Line 136: "здоров", "health", "статус системы",
"system status" → get_health_summary
Line 138: "задач", "task", "таск" → get_task_status
Line 140: "llm", "provider", "провайдер", "модель" → list_llm_providers
Line 142: "branch", "ветк" → create_branch
Line 144: "commit", "коммит" → commit_changes
Line 146: "deploy", "депло", "разверн" → deploy_dashboard
Line 148: "миграц", "migration", "migrate" → execute_migration
Line 150: "backup", "бэкап", "резерв" → run_backup
Line 152: "валидац", "validation", "validate" → run_llm_validation
Line 154: "документ", "documentation", "docs" → run_llm_documentation
Line 156: "инструмент", "tool", "capabilit",
"умеешь", "можешь" → show_capabilities
```
### 2.3 `_parse_command` (_command_parser.py:28-103)
Legacy REST parser — uses its own `if`/`elif` chain with different keywords. Separate code path, NOT used by the Gradio agent. Included for completeness.
### 2.4 `fast_confirmation_tool` (_tool_resolver.py:217-219)
```python
def fast_confirmation_tool(text: str) -> str | None:
tool_name = infer_tool_from_text(text)
return tool_name if tool_name in _FAST_CONFIRM_TOOLS else None
```
`_FAST_CONFIRM_TOOLS`:
```python
{"show_capabilities", "list_environments", "list_llm_providers",
"get_llm_status", "list_maintenance_events",
"superset_explore_database", "superset_audit_permissions", "superset_format_sql"}
```
### 2.5 `detect_message_state` (_persistence.py:116-124)
Separate concern (conversation list badges). Uses substring matching:
```python
error_markers = ["недоступен", "unavailable", "ошибка", "error", "произошла", "try again"]
cancel_markers = ["отменен", "cancelled", "отклонен", "denied"]
```
---
## 3. System Text Injection Points
User message text (`text` variable) is augmented at 6 points in `app.py` Handler:
| # | Line | Injection | Content | Contains keywords? |
|---|------|-----------|---------|-------------------|
| I1 | 193 | Truncation | `[...truncated]` | No ✓ |
| I2 | 215 | File upload | `--- Uploaded file content ---\n{parsed}` | **Yes** — parsed file content is uncontrolled |
| I3 | 281 | Prefetch marker | `[PRE-FETCHED DATA — use this directly, do NOT call tools]` | **Yes** — `tools` (⊂ `tool`) |
| I4 | 281 | Prefetch header | `Available dashboards in environment 'ss-dev' (260 total):` | **Yes** — `environment` (⊂ `env`), `dashboards` (⊂ `dashboard`) |
| I5 | 281 | Prefetch body | Dashboard titles × 260 from Superset API | **Yes** — ANY keyword could be a dashboard title |
| I6 | 354 | LLM retry prefix | `Respond with valid JSON only...` | No ✓ |
**Prefetch data source** (`_persistence.py:296-332`):
```python
async def prefetch_dashboards(env_id: str) -> str:
# GET /api/dashboards → extracts dashboard titles up to AGENT_PREFETCH_DASHBOARD_LIMIT (default 25)
# Format: "Available dashboards in environment '{env_id}' ({total} total):\n- {title} (id: {id}, modified: {date})"
```
---
## 4. Substring Collision Matrix
### 4.1 Discovered Collisions
All keyword lists use Python `any(word in text for word in [...])` — pure substring matching.
| # | Severity | Keyword | Collides with | Source | Affected function | Impact |
|---|----------|---------|--------------|--------|-------------------|--------|
| **P0** | **BLOCKER** | `tool` | ⊂ `tools` | I3: prefetch marker `"do NOT call tools"` | `get_tools_for_query` | Early return → only `show_capabilities`. ALL other tools stripped. |
| P1 | HIGH | `env` | ⊂ `environment` | I4: prefetch header `"in environment 'ss-dev'"` | `get_tools_for_query` | Spurious `list_environments` selection (masked by P0) |
| P2 | MEDIUM | `доступ` | ⊂ `доступные` | User query `"доступные дашборды"` (available dashboards) | `get_tools_for_query` | Spurious `superset_audit_permissions` (доступ = access, but доступные = available) |
| P3 | MEDIUM | `table` | ⊂ `TABLE` | SQLi-like user input `"DROP TABLE"` | `get_tools_for_query` | Spurious `superset_explore_database` |
| P4 | LOW | `select` | ⊂ `selected` | File content with "selected" text | `get_tools_for_query` | Spurious `superset_execute_sql` |
| D1-D11 | MEDIUM | All keywords | ⊂ Dashboard titles | I5: 260 uncontrolled dashboard titles | `get_tools_for_query` | Any dashboard named "Health Dashboard" → `get_health_summary`, etc. |
| F1-F13 | MEDIUM | All keywords | ⊂ File content | I2: uploaded file content | `fast_confirmation_tool` + `get_tools_for_query` | File content keywords pollute intent detection |
### 4.2 Intentional Substring Matches (Russian stemming)
These are DESIGNED to be substring matches — the stem captures multiple word forms:
| Stem | Matches (examples) | Design intent |
|------|-------------------|---------------|
| `обслуж` | обслуживание, обслуживания, обслуживании | Maintenance intent |
| `дашборд` | дашборда, дашборде, дашборды, дашбордов | Dashboard intent |
| `окруж` | окружение, окружения, окружении, окружений | Environment intent |
| `задач` | задача, задачи, задачу, задачей | Task intent |
| `валидац` | валидация, валидации, валидацией | Validation intent |
| `ветк` | ветка, ветки, ветку, веткой | Branch intent |
| `коммит` | коммита, коммиту, коммитом | Commit intent |
| `депло` | деплой, деплоя, деплоем | Deploy intent |
| `миграц` | миграция, миграции, миграцией | Migration intent |
| `бэкап` | бэкапа, бэкапу, бэкапом | Backup intent |
### 4.3 Intentional English Suffix Matches
| Keyword | Matches | Design intent |
|---------|---------|---------------|
| `dashboard` | dashboards | English plural |
| `environment` | environments | English plural |
| `migration` | migrations | English plural |
---
## 5. Applied Fixes
### 5.1 Fix 1 — Original Text Isolation (retained)
**File**: `backend/src/agent/app.py`
The original user message is captured BEFORE any augmentation (truncation, file upload, prefetch):
```python
# Line 176: Parse message
text = message.get("text", "") if isinstance(message, dict) else str(message)
user_message_text = text # Preserved for intent detection
```
This variable is used for:
- `fast_confirmation_tool(user_message_text)` — HITL fast-track
- `text_lower = user_message_text.lower()` — prefetch trigger check
- `confirmation_payload(conv_id, state, user_message_text)` — HITL metadata
### 5.2 Fix 2 — Full Tool Catalog (architecture change)
**File**: `backend/src/agent/app.py`
```python
# BEFORE (intent-based subset — RETIRED):
agent_tools = get_tools_for_query(user_message_text, prefetch_available=prefetch_available)
# AFTER (all 24 tools):
agent_tools = get_all_tools()
```
**Rationale**: Gemma's context window is now sufficient for the full 24-tool schema. Intent-based subset filtering was a workaround for the previous 4096-token limit (FR-030/FR-031). Sending all tools:
- Eliminates the entire class of substring-matching bugs (P0-P4, D1-D11)
- Lets the LLM decide which tool to call — the standard LangChain/LangGraph pattern
- Simplifies the code path (no `prefetch_available` flag, no tool selection logic)
**What's removed**:
- `prefetch_available` variable (dead code)
- `get_tools_for_query()` call from the handler
- Intent-based tool suppression (`search_dashboards` when prefetch available)
**What's retained**:
- `get_tools_for_query()` — code preserved in `tools.py`, not called. Available for future context-constrained deployments.
- `infer_tool_from_text()` / `fast_confirmation_tool()` — active for HITL fast-track
- `user_message_text` isolation — active for `fast_confirmation_tool` and prefetch trigger
- Prefetch mechanism — still pre-loads dashboard data into context
---
## 6. Remaining Vulnerabilities (Documented, Not Yet Addressed)
### 6.1 V1 — Cross-language prefix ambiguity
**Location**: `tools.py:1076`, `_tool_resolver.py:156`
```python
["инструмент", "tool", "capabilit", "умеешь", "можешь"]
```
**Issue**: The word `tool` is checked as a substring. Users asking legitimate questions containing "tool" (e.g., "which tool should I use") trigger the `show_capabilities` early return, stripping all other tools.
**Risk**: Low — the user IS asking about tools, so returning only `show_capabilities` is the correct behavior. But it prevents multi-intent queries like "which tool can run maintenance" → should get `show_capabilities` + maintenance tools.
### 6.2 V2 — `elif` ordering in `infer_tool_from_text`
**Issue**: The `elif` chain has a fixed priority order. When a query contains keywords for multiple intents, only the FIRST match wins.
**Example**: `"сделай deploy дашборда"` → `search_dashboards` (line 134 matches before line 146 `deploy`).
**Risk**: Low — `infer_tool_from_text` is a FALLBACK for the HITL confirmation system, not the primary tool selector. `get_tools_for_query` (independent `if`s) handles multi-intent correctly.
### 6.3 V3 — `llm` keyword doesn't distinguish providers vs status
**Location**: `_tool_resolver.py:140`, `tools.py:1092`
**Issue**: Both `list_llm_providers` and `get_llm_status` share the same keyword check. In `infer_tool_from_text`, only `list_llm_providers` is returned. In `get_tools_for_query`, both are included.
**Risk**: Low — providing both tools is acceptable. The LLM can choose the correct one.
### 6.4 V4 — `доступ` ⊂ `доступные` (access ⊂ available)
**Location**: `tools.py:1130`
```python
["аудит", "audit", "прав", "permission", "доступ", "access"]
```
**Issue**: Russian word `доступные` (available) contains `доступ` (access) as a prefix. User asking "покажи доступные дашборды" (show available dashboards) triggers `superset_audit_permissions` spuriously.
**Proposed fix**: Either add word boundaries for this keyword, or use `"доступ "` (with trailing space) to require a word break.
### 6.5 V5 — Superset tools not in `infer_tool_from_text`
**Issue**: The new Superset tools (SQL, explore, audit, create/copy dashboard, dataset) are present in `get_tools_for_query` but absent from `infer_tool_from_text`. The HITL fallback cannot infer these tools.
**Risk**: Low — HITL uses LangGraph checkpoint state first, `infer_tool_from_text` is last resort.
### 6.6 V6 — File upload contamination before the fix
**Status**: Mitigated by original text isolation.
Before the fix, file content was appended to `text` BEFORE `fast_confirmation_tool` and `get_tools_for_query`. Uploading a file with keywords could trigger false positive tool selection. The original text isolation fix (using `user_message_text` captured before file upload) eliminates this vector.
---
## 7. Test Coverage
### 7.1 Test File
`backend/tests/test_agent/test_intent_keyword_edges.py` — 163 tests, 11 categories.
### 7.2 Coverage Matrix
| Category | Tests | Coverage |
|----------|-------|----------|
| A — Prefetch contamination | 4 | P0 regression, P1 env, clean text verification |
| B — Dashboard title injection | 11 | All 11 keyword families × simulated dashboard titles |
| C — File upload contamination | 4 | File content → fast_confirm, infer_tool, get_tools |
| D — Empty/Null/Special | 13 | `""`, `None`, SQLi-like, emoji, 200K chars |
| E — Order sensitivity | 6 | `elif` priority chain, `if` accumulation |
| F — Multi-intent | 10 | All intent combinations |
| G — Language edges | 37 | RU stems (21 forms), EN suffixes, mixed RU/EN (5 queries × 2 functions) |
| H — Cross-function consistency | 15 | `infer_tool` vs `get_tools`, `fast_confirm` contracts |
| I — Maintenance intent (P0 scenario) | 7 | All variations of the original bug scenario |
| J — Keyword boundaries | 10 | Case, whitespace, garbage, sub-actions |
| K — Superset tools | 26 | All new Superset tools × 2 functions |
### 7.3 Existing Tests
- `tests/test_agent/test_langchain_tools.py` — Tool contracts, dual auth, intent matching (2 tests)
- `tests/test_agent/test_app.py` — Handler, confirmations, HITL (50 tests)
- `tests/test_agent/test_superset_tools.py` — Superset tool integration (25+ tests)
- `tests/test_agent/test_agent_handler.py` — Agent handler integration
- `tests/test_agent/test_confirmations.py` — HITL confirmation workflow
**Total agent test suite**: 375 tests, all passing.
---
## 8. Design Decisions & Rationale
### 8.1 Why full tool catalog now (was: why substring matching)
- **Context budget**: Gemma previously had a 4096 token limit — 24 tool schemas with Pydantic models would consume ~5000+ tokens. Intent-based subset filtering was necessary.
- **Current state**: Gemma context window is now sufficient for all 24 tools. The standard LangChain/LangGraph pattern is to give the LLM all tools and let it decide.
- **Simplicity**: Eliminates the entire class of substring-matching bugs and the complex keyword maintenance burden.
### 8.2 Why keep `infer_tool_from_text` / `fast_confirmation_tool`?
- **HITL fast-track**: Read-only tools can skip the full agent run and go directly to a confirmation dialog. This is a UX optimization, not a context-saving measure.
- **Deterministic**: Substring matching is 100% deterministic — no LLM hallucination risk for fast-track decisions.
### 8.3 Why keep `user_message_text` isolation?
- **HITL metadata**: `confirmation_payload` shows the user's original query, not augmented text.
- **Prefetch trigger**: Only prefetch dashboards when the USER asks about dashboards, not when system text mentions them.
- **Future-proof**: Any new system text injections won't affect HITL or prefetch decisions.
---
## 9. Edge Case Catalog (for future LLM review)
### 9.1 Input types
| Input | `get_tools_for_query` | `infer_tool_from_text` | `_parse_command` |
|-------|----------------------|------------------------|------------------|
| `""` (empty) | 5 fallback tools | `None` | `domain="unknown"` |
| `None` | 5 fallback tools | `None` | N/A |
| `"'; DROP TABLE users;--"` | `superset_explore_database` (table keyword) | `None` | `domain="unknown"` |
| `"🐛🔥💥"` | 5 fallback tools | `None` | `domain="unknown"` |
| `"..."` | 5 fallback tools | `None` | `domain="unknown"` |
| 200K chars of "dashboard " | `search_dashboards` (matches) | `search_dashboards` | N/A |
| `"xyzzy123!@#$%^&*()"` | 5 fallback tools | `None` | N/A |
### 9.2 Query → Expectation mapping (sample)
| Query | `get_tools_for_query` (no prefetch) | `infer_tool_from_text` |
|-------|-------------------------------------|------------------------|
| "Запусти обслуживание на дашборде USA" | show_capabilities, list_maintenance_events, start_maintenance, end_maintenance, search_dashboards | start_maintenance |
| "Запусти обслуживание на дашборде USA" (prefetch=True) | show_capabilities, list_maintenance_events, start_maintenance, end_maintenance | start_maintenance |
| "Покажи здоровье и окружения" | show_capabilities, get_health_summary, list_environments | list_environments |
| "Запусти миграцию" | show_capabilities, execute_migration | execute_migration |
| "сделай deploy дашборда" | show_capabilities, deploy_dashboard, search_dashboards | search_dashboards (elif!) |
| "Покажи доступные дашборды" | show_capabilities, superset_audit_permissions ⚠️ | search_dashboards |
---
## 10. Audit Checklist for LLM Reviewers
- [ ] **P0 fix verification**: Does `user_message_text` capture the original message BEFORE any augmentation? Verify lines 176-181 of `app.py`.
- [ ] **Keyword list completeness**: Are all 17+ tool categories covered? Any missing intents?
- [ ] **Russian stemming coverage**: Do stems `обслуж`, `окруж`, `задач`, `валидац`, `ветк`, `коммит`, `депло`, `миграц`, `бэкап` cover all common word forms?
- [ ] **Substring false positives**: Review V1-V6 (Section 6). Which should be prioritized for fixing?
- [ ] **Multi-intent correctness**: Does `get_tools_for_query` independent `if` accumulation produce correct results for mixed intents?
- [ ] **elif ordering in `infer_tool_from_text`**: Is the priority order (env > maintenance > dashboard > health > task > llm > branch > commit > deploy > migration > backup > validation > documentation > capabilities) correct?
- [ ] **`_FAST_CONFIRM_TOOLS` membership**: Are all read-only tools correctly classified? Any write tools incorrectly fast-tracked?
- [ ] **Dashboard title injection (D1-D11)**: Even though fixed by original text isolation, should the keyword lists be hardened against future injection vectors?
- [ ] **File upload contamination**: Is the `user_message_text` capture point (before line 215 file upload) correct?
- [ ] **Test coverage gaps**: Any intent categories missing from the 163 tests?
---
*Document prepared for LLM audit. All code references are to the superset-tools repository as of 2026-06-30.*