Skip to content

Commit 7257cf1

Browse files
angelovstantonangelovstanton
committed
fix(036): wire BehaviorAnalyzer to config + template loader; widen breakdown to strings
BehaviorAnalyzer now receives SpectraConfig and PromptTemplateLoader at construction and passes them to BuildAnalysisPrompt, so the .spectra/prompts/ behavior-analysis.md template and the user's analysis.categories config are no longer dead code. The 3 GenerateHandler call sites pass both. The per-category breakdown is widened from a closed BehaviorCategory enum to a free-form string identifier so custom categories survive end-to-end through IdentifiedBehavior, BehaviorAnalysisResult.Breakdown, AnalysisPresenter, CountSelector, and the JSON output. The enum (Spectra.Core.Models.BehaviorCategory) is deleted along with the derived getter that previously collapsed unknown AI-returned categories to HappyPath. FilterByFocus is rewritten as a single substring-match pass that handles both the legacy keywords (happy/negative/edge/security/performance) and custom IDs (e.g., --focus "keyboard" matches "keyboard_interaction"); the no-match fallback returning all behaviors is preserved. Empty/null/whitespace AI categories are bucketed under "uncategorized" at the analyzer's grouping step rather than crashing or being silently rebadged. CopilotGenerationAgent has the same latent dead-loader bug for the test-generation template; documented in research.md as a follow-up, explicitly out of scope for this feature. 1459 tests passing (1453 prior + 6 new).
1 parent e0659bf commit 7257cf1

File tree

19 files changed

+1371
-191
lines changed

19 files changed

+1371
-191
lines changed

‎specs/036-fix-analyzer-categories/checklists/requirements.md‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
# Specification Quality Checklist: Fix BehaviorAnalyzer Category Injection
2+
3+
**Purpose**: Validate specification completeness and quality before proceeding to planning
4+
**Created**: 2026-04-10
5+
**Feature**: [spec.md](../spec.md)
6+
7+
## Content Quality
8+
9+
- [x] No implementation details (languages, frameworks, APIs)
10+
- [x] Focused on user value and business needs
11+
- [x] Written for non-technical stakeholders
12+
- [x] All mandatory sections completed
13+
14+
## Requirement Completeness
15+
16+
- [x] No [NEEDS CLARIFICATION] markers remain
17+
- [x] Requirements are testable and unambiguous
18+
- [x] Success criteria are measurable
19+
- [x] Success criteria are technology-agnostic (no implementation details)
20+
- [x] All acceptance scenarios are defined
21+
- [x] Edge cases are identified
22+
- [x] Scope is clearly bounded
23+
- [x] Dependencies and assumptions identified
24+
25+
## Feature Readiness
26+
27+
- [x] All functional requirements have clear acceptance criteria
28+
- [x] User scenarios cover primary flows
29+
- [x] Feature meets measurable outcomes defined in Success Criteria
30+
- [x] No implementation details leak into specification
31+
32+
## Notes
33+
34+
- The user-supplied `/speckit.specify` input was implementation-rich (code diffs, file paths, exact constructor signatures). The spec re-states the intent in user/business terms; the implementation details are intentionally deferred to plan.md, research.md, and tasks.md.
35+
- During planning the actual code state was verified: `IdentifiedBehavior.CategoryRaw` is already a `string`, not an enum — but a derived `Category` getter collapses unknown values to `HappyPath`, which is the same user-visible bug the input describes. The spec stays at the user-facing level ("preserved end-to-end as the raw string") and lets the plan handle the mechanical detail.
36+
- Items marked incomplete require spec updates before `/speckit.clarify` or `/speckit.plan`.

‎specs/036-fix-analyzer-categories/contracts/analyzer-api.md‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
# Contract: BehaviorAnalyzer Public/Internal API
2+
3+
**Feature**: 036-fix-analyzer-categories
4+
**Date**: 2026-04-10
5+
6+
This is the contract for the consumers of `BehaviorAnalyzer` and the types it produces. It is internal-only (the analyzer is not part of any external API surface), but the contract still matters because three call sites in `GenerateHandler` and four test files depend on it.
7+
8+
## `BehaviorAnalyzer` constructor
9+
10+
```csharp
11+
public BehaviorAnalyzer(
12+
SpectraProviderConfig? provider,
13+
Action<string>? onStatus = null,
14+
SpectraConfig? config = null,
15+
PromptTemplateLoader? templateLoader = null)
16+
```
17+
18+
### Parameter contract
19+
20+
| Parameter | Required | Effect when null |
21+
|-----------|----------|------------------|
22+
| `provider` | No | AI calls fail at runtime; analysis returns null. (Same as today.) |
23+
| `onStatus` | No | No progress callbacks emitted. (Same as today.) |
24+
| `config` | No | Categories injected into the prompt fall back to the documented Spec 030 defaults via `PromptTemplateLoader.GetCategories(null)`. |
25+
| `templateLoader` | No | Legacy hardcoded prompt is used (the existing `BuildAnalysisPrompt` legacy branch). Tests rely on this. |
26+
27+
### Calling contract
28+
29+
- Production callers in `GenerateHandler` MUST pass non-null `config` and `templateLoader`. (Verified by inspection during tasks; no automated enforcement.)
30+
- Test callers MAY pass `null` for any combination.
31+
32+
## `IdentifiedBehavior` JSON contract
33+
34+
The AI returns objects shaped as:
35+
36+
```json
37+
{ "category": "<string>", "title": "<string>", "source": "<string>" }
38+
```
39+
40+
### Deserialization contract
41+
42+
- `category` is deserialized as a `string` (was already a string at the field level today, surfaced through `CategoryRaw`; the rename to `Category` is a public API change).
43+
- An empty string, missing field, or null value MUST NOT cause deserialization to fail; the field accepts it as-is. The "uncategorized" substitution happens at grouping time, not at parse time.
44+
- Any string the AI returns is accepted; there is no closed enum to validate against.
45+
46+
## `BehaviorAnalysisResult.Breakdown` contract
47+
48+
```csharp
49+
public IReadOnlyDictionary<string, int> Breakdown { get; init; }
50+
```
51+
52+
### Producer guarantees
53+
54+
- Keys are non-empty strings.
55+
- Empty/null/whitespace AI categories are bucketed as `"uncategorized"`.
56+
- Sum of values equals `TotalBehaviors`.
57+
- Iteration order is the order in which categories first appear in the AI response (insertion order via `GroupBy` + `ToDictionary`).
58+
59+
### Consumer obligations
60+
61+
- Consumers MUST handle arbitrary string keys, not a fixed set.
62+
- Consumers MUST NOT cast keys to a closed enum.
63+
- Consumers MUST provide a fallback rendering for unknown keys (the standard fallback is `id.Replace('_', ' ').Replace('-', ' ')`).
64+
65+
## `FilterByFocus` contract
66+
67+
```csharp
68+
internal static List<IdentifiedBehavior> FilterByFocus(
69+
List<IdentifiedBehavior> behaviors, string focusArea)
70+
```
71+
72+
### Behavior
73+
74+
1. Tokenize `focusArea` on whitespace, `,`, `;`. Lowercase.
75+
2. For each behavior, normalize `Category` by lowercasing and replacing `_` and `-` with spaces.
76+
3. A behavior matches if **any** focus token is a substring of its normalized category.
77+
4. If at least one behavior matches, return the matching subset.
78+
5. If no behavior matches, return the input list unchanged (focus is then applied downstream by the generation prompt instead).
79+
80+
### Edge cases
81+
82+
| Input | Output |
83+
|-------|--------|
84+
| `focusArea = ""` | Returns input unchanged (after `IsNullOrWhiteSpace` check at the analyzer's caller — `AnalyzeAsync` only invokes `FilterByFocus` when focus is non-empty, so this is a defensive guarantee). |
85+
| `focusArea = "happy"` and category list contains `"happy_path"` | Matches. |
86+
| `focusArea = "keyboard"` and category list contains `"keyboard_interaction"` | Matches. |
87+
| `focusArea = "asdf"` and no category contains "asdf" | Returns input unchanged. |
88+
| Multiple focus tokens, some match, some don't | Returns the union of behaviors that any single token matched. |
89+
90+
## `GenerateHandler` call site contract
91+
92+
All three `BehaviorAnalyzer` instantiation sites in `GenerateHandler.cs` MUST pass `config` and a `PromptTemplateLoader`:
93+
94+
```csharp
95+
var loader = new PromptTemplateLoader(currentDir); // currentDir already exists in scope
96+
var analyzer = new BehaviorAnalyzer(provider, onStatusCallback, config, loader);
97+
```
98+
99+
The `currentDir` variable already exists at lines 183, 754, 1323, 1396 of the handler. Each of the three call sites is inside (or downstream of) one of those `currentDir` declarations, so capturing it is straightforward.
100+
101+
## Out of contract
102+
103+
- `CopilotGenerationAgent` and its `BuildFullPrompt` are not changed by this feature even though they have an analogous bug. Documented as a follow-up in research.md D8.
104+
- The Spec 030 default category list is not changed by this feature.
105+
- The shape of `analysis.categories` in `spectra.config.json` is not changed.
106+
- The `PromptTemplateLoader` API is not changed.
107+
- The dashboard's category breakdown rendering is not changed (out of scope per spec).

‎specs/036-fix-analyzer-categories/data-model.md‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
# Phase 1 — Data Model: Fix BehaviorAnalyzer Category Injection
2+
3+
**Feature**: 036-fix-analyzer-categories
4+
**Date**: 2026-04-10
5+
6+
This feature reshapes existing types rather than introducing new ones. The "data model" is therefore a before/after table for each affected type.
7+
8+
## Type 1 — `IdentifiedBehavior` (Spectra.CLI.Agent.Analysis)
9+
10+
| Aspect | Before | After |
11+
|--------|--------|-------|
12+
| Category field | `string CategoryRaw` (with `[JsonPropertyName("category")]`) | `string Category` (same JSON name) |
13+
| Derived enum getter | `BehaviorCategory Category => ParseCategory(CategoryRaw)` (collapses unknowns to `HappyPath`) | **REMOVED** |
14+
| Default for empty/null | Falls into HappyPath via the parser's `_ =>` arm | Stored as-is; the analyzer's grouping step substitutes `"uncategorized"` for empty/null |
15+
16+
### Validation rules
17+
- `Category` MUST be preserved exactly as the AI sent it (no normalization, no rebadging) — this is the field-level guarantee for FR-006.
18+
- The model carries no validation on the category string; the analyzer is responsible for the "uncategorized" fallback at grouping time.
19+
20+
## Type 2 — `BehaviorAnalysisResult` (Spectra.CLI.Agent.Analysis)
21+
22+
| Aspect | Before | After |
23+
|--------|--------|-------|
24+
| `Breakdown` type | `IReadOnlyDictionary<BehaviorCategory, int>` | `IReadOnlyDictionary<string, int>` |
25+
| `GetRemainingByCategory` parameter | `IReadOnlyList<BehaviorCategory>?` | `IReadOnlyList<string>?` |
26+
| `GetRemainingByCategory` return | `IReadOnlyDictionary<BehaviorCategory, int>` | `IReadOnlyDictionary<string, int>` |
27+
| Implementation of `GetRemainingByCategory` | Looks up enum keys, zeroes them | Looks up string keys, zeroes them |
28+
29+
### Validation rules
30+
- `Breakdown` keys are arbitrary non-empty strings (the "uncategorized" bucket included).
31+
- `GetRemainingByCategory` MUST treat the parameter as a closed set of identifiers to subtract; behaviors with categories *not* in the parameter list are preserved.
32+
33+
## Type 3 — `BehaviorAnalyzer` (Spectra.CLI.Agent.Copilot)
34+
35+
| Aspect | Before | After |
36+
|--------|--------|-------|
37+
| Constructor signature | `BehaviorAnalyzer(SpectraProviderConfig? provider, Action<string>? onStatus = null)` | `BehaviorAnalyzer(SpectraProviderConfig? provider, Action<string>? onStatus = null, SpectraConfig? config = null, PromptTemplateLoader? templateLoader = null)` |
38+
| Private fields | `_provider`, `_onStatus` | + `_config`, `_templateLoader` |
39+
| `BuildAnalysisPrompt` call in `AnalyzeAsync` | `BuildAnalysisPrompt(documents, focusArea)` | `BuildAnalysisPrompt(documents, focusArea, _config, _templateLoader)` |
40+
| Breakdown grouping in `AnalyzeAsync` | `behaviors.GroupBy(b => b.Category)` (enum) | `behaviors.GroupBy(b => string.IsNullOrWhiteSpace(b.Category) ? "uncategorized" : b.Category)` |
41+
| `FilterByFocus` body | Enum keyword expansion + `behaviors.Where(b => matchingCategories.Contains(b.Category))` | String tokenization + substring match against normalized `b.Category` |
42+
| `FilterByFocus` parameter type | `List<IdentifiedBehavior> behaviors, string focusArea` | unchanged |
43+
44+
### Validation rules
45+
- The new constructor params remain optional with `null` defaults — required by FR-005's "Internal/test-only callers MAY pass null" clause.
46+
- When `templateLoader` is null, the legacy fallback path runs unchanged (this is the path existing tests exercise).
47+
48+
## Type 4 — `AnalysisPresenter` (Spectra.CLI.Output)
49+
50+
| Aspect | Before | After |
51+
|--------|--------|-------|
52+
| `CategoryLabels` | `Dictionary<BehaviorCategory, string>` with 5 fixed entries | `Dictionary<string, string>` keyed by category ID, with the same 5 entries plus the 6 Spec 030 defaults; additional/unknown IDs render via fallback formatter `id.Replace('_', ' ').Replace('-', ' ')` |
53+
| `RenderXxx(IReadOnlyList<BehaviorCategory>?)` parameter (line 65) | enum list | `IReadOnlyList<string>?` |
54+
55+
### Display rules
56+
- A category ID without a label entry MUST render as a human-readable string by replacing `_` and `-` with spaces (e.g., `keyboard_interaction` → "keyboard interaction"). No exception, no warning.
57+
- Order in display follows the order of keys in the breakdown dictionary as returned by the analyzer (no alphabetical re-sort) — preserves the AI's original ordering.
58+
59+
## Type 5 — `CountSelector` (Spectra.CLI.Interactive)
60+
61+
| Aspect | Before | After |
62+
|--------|--------|-------|
63+
| `SelectedCategories` property | `IReadOnlyList<BehaviorCategory>?` | `IReadOnlyList<string>?` |
64+
| Internal `Categories` property (line 152) | `IReadOnlyList<BehaviorCategory>?` | `IReadOnlyList<string>?` |
65+
| `CategoryLabels` dict | enum-keyed | string-keyed (same fallback formatter as `AnalysisPresenter`) |
66+
| `analysis.Breakdown.OrderBy(...)` (line 97) | iterates enum keys | iterates string keys; ordering remains insertion order |
67+
68+
## Type 6 — `BehaviorCategory` enum (Spectra.Core.Models)
69+
70+
| Aspect | Before | After |
71+
|--------|--------|-------|
72+
| Existence | 5-value enum (`HappyPath`, `Negative`, `EdgeCase`, `Security`, `Performance`) with `[JsonStringEnumConverter]` | **DELETED** |
73+
74+
### Migration rules
75+
- Every reference to `BehaviorCategory.HappyPath` becomes the literal string `"happy_path"`.
76+
- Every reference to `BehaviorCategory.Negative` becomes `"negative"`.
77+
- `BehaviorCategory.EdgeCase` → `"edge_case"`.
78+
- `BehaviorCategory.Security` → `"security"`.
79+
- `BehaviorCategory.Performance` → `"performance"`.
80+
- Spec 030 defaults that were not in the legacy enum (`boundary`, `error_handling`) gain first-class display labels in `AnalysisPresenter.CategoryLabels` for nicer rendering.
81+
82+
## State transition: behavior category lifecycle
83+
84+
```
85+
AI returns JSON [string identifier from prompt-allowed set or invented]
86+
│
87+
▼
88+
JSON deserialize [IdentifiedBehavior.Category : string, preserved verbatim]
89+
│
90+
▼
91+
GroupBy in AnalyzeAsync [empty/null/whitespace → "uncategorized"; everything else preserved]
92+
│
93+
▼
94+
BehaviorAnalysisResult.Breakdown [IReadOnlyDictionary<string,int>]
95+
│
96+
├──────► AnalysisPresenter renders → CategoryLabels lookup → fallback formatter
97+
├──────► CountSelector lists → CategoryLabels lookup → fallback formatter
98+
├──────► JSON output → key/value preserved as-is
99+
└──────► SuggestionBuilder consumes → unchanged semantics
100+
```
101+
102+
## Type-shape diff summary
103+
104+
| File | Lines changed (estimate) | Net production line delta |
105+
|------|--------------------------|---------------------------|
106+
| `IdentifiedBehavior.cs` | ~12 | -10 (drop derived getter) |
107+
| `BehaviorAnalysisResult.cs` | ~4 | 0 |
108+
| `BehaviorAnalyzer.cs` | ~30 | -5 (FilterByFocus simpler) |
109+
| `AnalysisPresenter.cs` | ~10 | +5 (fallback formatter helper) |
110+
| `CountSelector.cs` | ~10 | 0 |
111+
| `GenerateHandler.cs` | ~6 (3 call sites × 2 lines each) | +6 |
112+
| `BehaviorCategory.cs` | -16 | -16 (deleted) |
113+
| **Total** | ~88 | ≈ −20 |
114+
115+
---
116+
117+
**Status**: Phase 1 data model complete.

0 commit comments

Comments
 (0)
⚡