fix: review_findings.category is NULL for all non-security findings (71 of 78)

[mickume]

Summary

The category column in review_findings (added in schema v7) is only populated for security-classified findings. All other findings get category=NULL.

review_findings by category:
  NULL:     71 findings (91%)
  security:  4 findings ( 5%)
  (3 major/security + 1 critical/security)

Root Cause

_detect_security_category() in review_parser.py:84-94 is the only categorization logic. It checks ~20 hardcoded security keywords and returns either "security" or None. No other category types exist.

def _detect_security_category(description: str) -> Optional[str]:
    # Returns "security" if keywords match, None otherwise
    ...

The category value is correctly passed through to the INSERT statement (review_store.py:229-239), but since only security is detected, everything else is NULL.

Files:

• agent_fox/session/review_parser.py:57-94 — keyword list and detection function
• agent_fox/knowledge/review_store.py:229-239 — INSERT statement

Impact

The category field was designed to enable filtering and prioritization of findings by type. With 91% NULL values, it's effectively useless for non-security analysis. Queries like "show all correctness findings" or "count performance issues" are impossible.

Suggested Fix

Expand _detect_security_category() (or replace it) with a multi-category classifier. Reasonable categories based on the existing finding descriptions:

• correctness — spec compliance, wrong behavior, missing functionality
• compatibility — API mismatches, proto field disagreements
• testing — missing tests, test infrastructure issues
• configuration — wrong ports, missing config, deployment issues
• security — existing keyword-based detection

Could be keyword-based (same approach, more lists) or LLM-based for higher accuracy.

[mickume]

Starting fix session on branch fix/485-fix-review-findingscategory-is-null-for-all-non-security-findings-71-of-78... (run: 20260417_184130_c51887)

[mickume]

Analysis

Auto-generated by `af-fix`. Reviewing issue context and codebase.

Diagnosis

Classification: bug

Root Cause / Problem:
_detect_security_category() in agent_fox/session/review_parser.py (lines 84–94) is the sole categorization function and only recognises a single category: "security". It checks ~20 hardcoded security keywords and returns None for everything else. Since the category field in ReviewFinding is set exclusively from the output of this function, all non-security findings are stored with category=NULL — 91% of rows.

The INSERT path in review_store.py is correct; the data is faithfully persisted. The gap is entirely in the detection logic.

Planned Fix

Approach:
Replace _detect_security_category() with _classify_category(), a multi-category keyword classifier. Five keyword sets cover the categories suggested in the issue: security (existing), correctness, compatibility, testing, and configuration. Categories are checked in priority order so that a finding matching both security and correctness keywords is classified as security. The old _detect_security_category name is kept as an alias for backward compatibility.

Files to modify:

• agent_fox/session/review_parser.py — add four keyword sets, replace single-category function with multi-category classifier, update call site in parse_review_findings()
• tests/unit/session/test_review_parser.py — add TestClassifyCategory and TestCategoryPopulatedInFindings test classes (14 new tests)

Assumptions:

• Keyword-based detection (same approach as the existing security detection) is sufficient. LLM-based classification is not required for this fix.
• Security takes priority when a description matches multiple categories.
• The _detect_security_category alias is kept so any external code that imported it continues to work.

---

Analysis by af-fix. Implementation follows.

[mickume]

Fix Implemented

Auto-generated by `af-fix`.

Summary

Replaced the single-category _detect_security_category() function in review_parser.py with _classify_category(), which checks five keyword lists in priority order: security, correctness, compatibility, testing, and configuration. The call site in parse_review_findings() now calls _classify_category(), so non-security findings receive the appropriate non-NULL category value.

Changes

File	Change
agent_fox/session/review_parser.py	Added four keyword sets, replaced single-category function with _classify_category(), kept _detect_security_category as alias
tests/unit/session/test_review_parser.py	Added TestClassifyCategory and TestCategoryPopulatedInFindings (14 new tests)

Tests

• tests/unit/session/test_review_parser.py::TestClassifyCategory — unit tests for _classify_category() covering all categories, no-match case, priority ordering, case-insensitivity
• tests/unit/session/test_review_parser.py::TestCategoryPopulatedInFindings — end-to-end tests via parse_review_output()

Verification

• All existing tests pass: ✅
• New tests pass: ✅ (14 added, 5009 total)
• Linter / formatter: ✅
• No regressions: ✅

Branch

fix/485-fix-review-findingscategory-is-null-for-all-non-security-findings-71-of-78 — PR #487 ready to merge into develop.

---

Fix by `af-fix`. Ready for review.

[mickume]

Fix Review Report

Overall verdict: PASS

Summary: The fix correctly expands the single-category security detector into a multi-category classifier supporting 5 categories (security, correctness, compatibility, testing, configuration) with priority ordering. Backward compatibility is preserved via alias. The review_store INSERT requires no changes. Comprehensive tests cover all categories, edge cases, and priority. Full test suite (5009 tests) and linter pass cleanly.

Per-criterion verdicts

• ✅ AC-1: PASS
  • Evidence: The _classify_category() function (review_parser.py:171-185) replaces the security-only detector with a multi-category classifier supporting correctness, compatibility, testing, and configuration in addition to security. Each category has a dedicated keyword frozenset (_CORRECTNESS_KEYWORDS, _COMPATIBILITY_KEYWORDS, _TESTING_KEYWORDS, _CONFIGURATION_KEYWORDS). The call site at line 251 uses the new function.
• ✅ AC-2: PASS
  • Evidence: Security keyword detection is preserved unchanged — _SECURITY_KEYWORDS frozenset (lines 57-81) is identical to the original. The backward-compatibility alias _detect_security_category = _classify_category (line 190) ensures any external callers still work. Security takes priority via being first in _CATEGORY_RULES (line 163).
• ✅ AC-3: PASS
  • Evidence: review_store.py is unchanged — the INSERT statement at line 229/239 already passes f.category through, and the schema column accepts any string value. No store-layer changes needed.
• ✅ AC-4: PASS
  • Evidence: Test class TestClassifyCategory covers all 5 categories, None fallback, security priority over correctness, and case-insensitivity. TestCategoryPopulatedInFindings verifies end-to-end through parse_review_output for all categories. All 5009 tests pass.
• ✅ AC-5: PASS
  • Evidence: make check passes: ruff lint clean (0 errors), ruff format clean (220 files), pytest 5009 passed in 75.10s with 0 failures. No regressions.

(run: 20260417_184130_c51887)

[mickume]

Fix complete on branch fix/485-fix-review-findingscategory-is-null-for-all-non-security-findings-71-of-78. Changes have been merged into develop. Create a PR from that branch to land them on main. (run: 20260417_184130_c51887)