## Code Review: Fix summary for large belief networks

### CLAUDE.md (new file)
VERDICT: PASS
CORRECTNESS: VALID
SPEC_COMPLIANCE: N/A
ISSUE_COMPLIANCE: UNRELATED
BELIEF_COMPLIANCE: N/A
TEST_COVERAGE: N/A
INTEGRATION: N/A
REASONING: Standard project documentation file. Not related to the issue fix itself, but harmless and useful. Content matches what's already loaded from the project instructions.
---

### ftl_project_expert/cli.py:summary
VERDICT: CONCERN
CORRECTNESS: QUESTIONABLE
SPEC_COMPLIANCE: N/A
ISSUE_COMPLIANCE: PARTIAL
BELIEF_COMPLIANCE: N/A
TEST_COVERAGE: UNTESTED
INTEGRATION: WIRED
REASONING:

**What it does well:**
- Caps beliefs at 500, directly addressing the root cause of context window overflow.
- Filters to `--status IN` beliefs only, reducing noise.
- Sorts `--by-impact` so truncation drops the least important beliefs.
- Reports prompt size in KB on failure, addressing the "report the actual failure clearly" suggestion.
- Status message distinguishes truncated vs. non-truncated runs with sort order context.

**Concerns:**

1. **Unverified CLI flags** (`--status IN`, `--by-impact`): The `reasons list` command is invoked with flags `--status` and `--by-impact` that may not exist. If `reasons list` doesn't support these flags, it will fail with a non-zero return code, and the code silently falls through to the `beliefs.md` path (or worse, to the "No beliefs found" error). There's no validation or fallback. The observation for `invoke_raises` returned "Function 'invoke' not found" suggesting `invoke` lives in `llm.py`, but the `reasons` CLI flags are the bigger risk — if they don't exist, the feature silently breaks for all `reasons.db` users.

2. **beliefs.md truncation is not sorted by impact**: The `reasons list` path sorts by impact before truncating, but the `beliefs.md` path just takes the first 500 sections in file order. The issue suggests "Sample the top N by recency, depth, or topic diversity" — file-order truncation of `beliefs.md` is arbitrary. The status message says "by file order" which is honest, but the quality of the summary will vary depending on how `beliefs.md` is ordered. This is a known limitation, not a bug.

3. **`sorted_by_impact` is always `True` for the reasons path**: Even if `--by-impact` is silently ignored (flag not recognized but doesn't error), `sorted_by_impact` would still be `True`, misleading the user. The variable is set unconditionally at line ~2055 rather than being derived from whether sorting actually happened.

4. **No test coverage**: The observation confirms `summary_tests.test_count == 0`. There are zero tests for the `summary` function, and no tests were added in this change. For a fix to a production failure affecting 10k+ node networks, this is a significant gap. At minimum, a unit test for `build_summary_prompt` with the new parameters and an integration-style test verifying truncation behavior would be valuable.

5. **Hardcoded `max_beliefs = 500`**: Not configurable via CLI flag or config file. Reasonable default, but users with larger context models or smaller belief sets may want to tune this. Minor concern — can be addressed later.

6. **The `beliefs.md` section regex** (`(?=^### \S+)`) splits on any `### ` heading. The filter `s.strip().startswith("###")` then re-validates. This is correct but the split will produce an initial empty-or-preamble element that gets filtered. No bug, just slightly wasteful.
---

### ftl_project_expert/prompts/summary.py:build_summary_prompt
VERDICT: PASS
CORRECTNESS: VALID
SPEC_COMPLIANCE: N/A
ISSUE_COMPLIANCE: ADDRESSES
BELIEF_COMPLIANCE: N/A
TEST_COVERAGE: UNTESTED
INTEGRATION: WIRED
REASONING: The prompt now tells the LLM how many beliefs it's seeing out of how many total, and whether they're sorted by impact. This is clean — it gives the model the context it needs to produce a qualified summary rather than treating a subset as the whole. The new parameters have sensible defaults (`total_count=0`, `sorted_by_impact=False`) so existing callers (there's only one) won't break if called without them, though the caller does pass them. The only caller is updated in the same diff. No tests exist (confirmed by observation).
---

### Overall Assessment

The change addresses the core issue: `summary` will no longer fail on 10k+ belief networks because beliefs are capped at 500. The error reporting improvement (prompt size in KB) addresses the diagnostic gap. The prompt update correctly informs the LLM about truncation.

**Key risks:**
- The `reasons list --status IN --by-impact` flags are assumed to exist but not verified in this review. If they don't, the reasons.db path silently fails.
- Zero test coverage for any of this logic.

### SELF_REVIEW
LIMITATIONS: Could not verify whether `reasons list` actually supports `--status` and `--by-impact` flags — this is an external CLI tool and its interface wasn't included in the diff or observations. Could not see `invoke()` in `llm.py` to verify error propagation behavior. No test files were available to check for indirect coverage.
---

### FEATURE_REQUESTS
- Include the interface/help output of external CLI tools invoked with new flags (here, `reasons list --help`) so reviewers can verify flag validity.
- When `test_count == 0`, automatically flag this in the observation results as a review risk factor.
---
