Skip to content

Commit e1bfb0a

Browse files
authored
refactor(exclusion): consolidate _session_text_for_exclusion + 6 repeated call patterns (closes #23) (#24)
Three layers of duplication around exclusion-rule filtering: 1. `_session_text_for_exclusion(session)` was defined twice — byte-identical bodies (only docstring differs) in scripts/export.py:476 and api/export_api.py:43. Two maintenance points for one function. 2. Six call sites repeated the same `if rules: build_searchable_text(...) → is_excluded_by_rules(...) → skip` pattern across api/sessions.py, api/projects.py, api/search.py, api/export_api.py (×2), and scripts/export.py. ~60 lines of near-identical wrapping. 3. Latent inconsistency: the helper rejected whitespace-only message text (`isinstance(text, str) and text.strip()`); the 4 inline call sites did not (`if msg.get("text")`). Bug surface small but real, and there was no reason for the difference. Consolidated into utils/exclusion_rules.py: - session_text_for_exclusion(session) — moved from the duplicate-defined private helpers; now the single source of truth. - is_session_excluded(rules, session, project_name) — wraps the full pattern. Returns False when rules is empty/falsy, so callers don't have to gate on `if rules:` before calling. Six call sites collapsed to one line each. After: one definition of the extraction logic, one definition of the wrapping pattern, the whitespace-handling inconsistency disappears (everyone uses the helper), and ~20 net lines removed across 6 files. Tests: tests/test_exclusion_helpers.py adds 16 unit cases for the new helpers — text-extraction edge cases (empty, whitespace-only, non-string, mixed) and rule-matching across all four haystack fields (project_name, session_title, model_names, content) plus AND/OR/quoted-phrase grammar plus defensive cases (no metadata key, project_name=None). 91/91 pytest pass (was 75; +16 new).
1 parent 3bc9a5d commit e1bfb0a

7 files changed

Lines changed: 228 additions & 90 deletions

File tree

api/export_api.py

Lines changed: 9 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
from utils.session_stats import compute_stats
1414
from utils.md_exporter import session_to_markdown
1515
from utils.json_exporter import session_to_json
16-
from utils.exclusion_rules import build_searchable_text, is_excluded_by_rules
16+
from utils.exclusion_rules import is_session_excluded
1717

1818
export_bp = Blueprint("export", __name__)
1919

@@ -40,16 +40,6 @@ def _write_state(sessions_map: dict, count: int):
4040
json.dump(state, f, indent=2)
4141

4242

43-
def _session_text_for_exclusion(session: dict) -> str:
44-
"""Extract a plain-text snippet from session messages for exclusion matching."""
45-
parts = []
46-
for msg in session.get("messages", []):
47-
text = msg.get("text") or ""
48-
if isinstance(text, str) and text.strip():
49-
parts.append(text)
50-
return "\n\n".join(parts)
51-
52-
5343
@export_bp.route("/api/export/state")
5444
def get_export_state():
5545
state = _read_state()
@@ -98,16 +88,12 @@ def bulk_export():
9888
if session["title"] == "Untitled Session":
9989
continue
10090

101-
if rules:
102-
meta = session["metadata"]
103-
searchable = build_searchable_text(
104-
project_name=project.get("display_name") or project["name"],
105-
session_title=session["title"],
106-
model_names=list(meta.get("models_used") or []),
107-
content_snippet=_session_text_for_exclusion(session),
108-
)
109-
if is_excluded_by_rules(rules, searchable):
110-
continue
91+
if is_session_excluded(
92+
rules,
93+
session,
94+
project.get("display_name") or project["name"],
95+
):
96+
continue
11197

11298
stats = compute_stats(session)
11399
md = session_to_markdown(session, stats)
@@ -166,17 +152,8 @@ def export_session(project_name, session_id):
166152
fmt = request.args.get("format", "md")
167153
session = parse_session(filepath)
168154
rules = current_app.config.get("EXCLUSION_RULES") or []
169-
if rules:
170-
meta = session["metadata"]
171-
text_parts = [msg.get("text") or "" for msg in session.get("messages", []) if msg.get("text")]
172-
searchable = build_searchable_text(
173-
project_name=project_name,
174-
session_title=session["title"],
175-
model_names=list(meta.get("models_used") or []),
176-
content_snippet="\n\n".join(text_parts),
177-
)
178-
if is_excluded_by_rules(rules, searchable):
179-
return jsonify({"error": "Session not found"}), 404
155+
if is_session_excluded(rules, session, project_name):
156+
return jsonify({"error": "Session not found"}), 404
180157
stats = compute_stats(session)
181158
title_slug = _slugify(session["title"]) or "session"
182159

api/projects.py

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
from flask import Blueprint, current_app, jsonify
66

77
from utils.session_path import get_claude_projects_dir, list_projects, list_sessions, safe_join
8-
from utils.exclusion_rules import build_searchable_text, is_excluded_by_rules
8+
from utils.exclusion_rules import is_session_excluded
99

1010
projects_bp = Blueprint("projects", __name__)
1111

@@ -60,16 +60,8 @@ def get_project_sessions(project_name):
6060
# Skip untitled sessions (no real conversation)
6161
if parsed["title"] == "Untitled Session":
6262
continue
63-
if rules:
64-
text_parts = [msg.get("text") or "" for msg in parsed.get("messages", []) if msg.get("text")]
65-
searchable = build_searchable_text(
66-
project_name=project_name,
67-
session_title=parsed["title"],
68-
model_names=list(meta.get("models_used") or []),
69-
content_snippet="\n\n".join(text_parts),
70-
)
71-
if is_excluded_by_rules(rules, searchable):
72-
continue
63+
if is_session_excluded(rules, parsed, project_name):
64+
continue
7365
result.append({
7466
**s,
7567
"title": parsed["title"],

api/search.py

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
from utils.session_path import get_claude_projects_dir, list_projects, list_sessions
88
from utils.jsonl_parser import parse_session
9-
from utils.exclusion_rules import build_searchable_text, is_excluded_by_rules
9+
from utils.exclusion_rules import is_session_excluded
1010

1111
search_bp = Blueprint("search", __name__)
1212

@@ -33,17 +33,8 @@ def search():
3333
except Exception:
3434
continue
3535

36-
if rules:
37-
meta = session["metadata"]
38-
text_parts = [msg.get("text") or "" for msg in session.get("messages", []) if msg.get("text")]
39-
searchable = build_searchable_text(
40-
project_name=project["name"],
41-
session_title=session["title"],
42-
model_names=list(meta.get("models_used") or []),
43-
content_snippet="\n\n".join(text_parts),
44-
)
45-
if is_excluded_by_rules(rules, searchable):
46-
continue
36+
if is_session_excluded(rules, session, project["name"]):
37+
continue
4738

4839
for msg in session["messages"]:
4940
text = msg.get("text", "") or msg.get("content", "")

api/sessions.py

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
from utils.session_path import get_claude_projects_dir, safe_join
99
from utils.jsonl_parser import parse_session
1010
from utils.session_stats import compute_stats
11-
from utils.exclusion_rules import build_searchable_text, is_excluded_by_rules
11+
from utils.exclusion_rules import is_session_excluded
1212

1313
sessions_bp = Blueprint("sessions", __name__)
1414

@@ -27,17 +27,8 @@ def get_session(project_name, session_id):
2727
try:
2828
session = parse_session(filepath)
2929
rules = current_app.config.get("EXCLUSION_RULES") or []
30-
if rules:
31-
meta = session["metadata"]
32-
text_parts = [msg.get("text") or "" for msg in session.get("messages", []) if msg.get("text")]
33-
searchable = build_searchable_text(
34-
project_name=project_name,
35-
session_title=session["title"],
36-
model_names=list(meta.get("models_used") or []),
37-
content_snippet="\n\n".join(text_parts),
38-
)
39-
if is_excluded_by_rules(rules, searchable):
40-
return jsonify({"error": "Session not found"}), 404
30+
if is_session_excluded(rules, session, project_name):
31+
return jsonify({"error": "Session not found"}), 404
4132
return jsonify(session)
4233
except Exception as e:
4334
tb = traceback.format_exc()

scripts/export.py

Lines changed: 8 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,7 @@
3131
from utils.exclusion_rules import (
3232
resolve_exclusion_rules_path,
3333
load_rules,
34-
build_searchable_text,
35-
is_excluded_by_rules,
34+
is_session_excluded,
3635
)
3736

3837

@@ -348,17 +347,13 @@ def cmd_export(args):
348347
skipped += 1
349348
continue
350349

351-
if rules:
352-
meta = session["metadata"]
353-
searchable = build_searchable_text(
354-
project_name=project.get("display_name") or project["name"],
355-
session_title=session["title"],
356-
model_names=list(meta.get("models_used") or []),
357-
content_snippet=_session_text_for_exclusion(session),
358-
)
359-
if is_excluded_by_rules(rules, searchable):
360-
skipped += 1
361-
continue
350+
if is_session_excluded(
351+
rules,
352+
session,
353+
project.get("display_name") or project["name"],
354+
):
355+
skipped += 1
356+
continue
362357

363358
stats = compute_stats(session)
364359
meta = session["metadata"]
@@ -473,16 +468,6 @@ def _export_single(session: dict, stats: dict, fmt: str, out_dir: str):
473468
# ==================== Helpers ====================
474469

475470

476-
def _session_text_for_exclusion(session: dict) -> str:
477-
"""Extract plain text from all session messages for exclusion rule matching."""
478-
parts = []
479-
for msg in session.get("messages", []):
480-
text = msg.get("text") or ""
481-
if isinstance(text, str) and text.strip():
482-
parts.append(text)
483-
return "\n\n".join(parts)
484-
485-
486471
# ==================== Argument Parser ====================
487472

488473

tests/test_exclusion_helpers.py

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
"""
2+
Unit tests for the consolidated exclusion-rule helpers introduced in issue #23:
3+
4+
- ``session_text_for_exclusion`` — moved from a duplicate-defined private helper
5+
in ``scripts/export.py`` and ``api/export_api.py`` into ``utils/exclusion_rules``.
6+
- ``is_session_excluded`` — wraps the previously-inlined "extract text →
7+
build_searchable_text → is_excluded_by_rules" pattern that was repeated
8+
across six call sites.
9+
10+
Both functions are pure and dependency-free, so they're tested directly without
11+
booting Flask or any of the API blueprints.
12+
13+
Run:
14+
pytest tests/test_exclusion_helpers.py -v
15+
"""
16+
17+
from __future__ import annotations
18+
19+
import sys
20+
from pathlib import Path
21+
22+
import pytest
23+
24+
REPO_ROOT = Path(__file__).resolve().parent.parent
25+
sys.path.insert(0, str(REPO_ROOT))
26+
27+
from utils.exclusion_rules import (
28+
is_session_excluded,
29+
load_rules,
30+
session_text_for_exclusion,
31+
)
32+
33+
34+
# ---------------------------------------------------------------------------
35+
# Helpers
36+
# ---------------------------------------------------------------------------
37+
38+
def _write_rules(tmp_path, *lines: str) -> str:
39+
"""Write rules file and return its path. Tokenized by load_rules."""
40+
p = tmp_path / "exclusion-rules.txt"
41+
p.write_text("\n".join(lines), encoding="utf-8")
42+
return str(p)
43+
44+
45+
def _session(*, title: str = "session", models: list[str] | None = None,
46+
messages: list[dict] | None = None) -> dict:
47+
return {
48+
"title": title,
49+
"metadata": {"models_used": models or []},
50+
"messages": messages or [],
51+
}
52+
53+
54+
# ---------------------------------------------------------------------------
55+
# session_text_for_exclusion
56+
# ---------------------------------------------------------------------------
57+
58+
class TestSessionTextForExclusion:
59+
60+
def test_empty_session(self):
61+
assert session_text_for_exclusion({}) == ""
62+
63+
def test_session_with_no_messages(self):
64+
assert session_text_for_exclusion({"messages": []}) == ""
65+
66+
def test_joins_message_text_with_blank_lines(self):
67+
s = _session(messages=[{"text": "alpha"}, {"text": "beta"}])
68+
assert session_text_for_exclusion(s) == "alpha\n\nbeta"
69+
70+
def test_skips_messages_without_text(self):
71+
s = _session(messages=[{"text": "alpha"}, {"role": "tool"}, {"text": "gamma"}])
72+
assert session_text_for_exclusion(s) == "alpha\n\ngamma"
73+
74+
def test_skips_whitespace_only_text(self):
75+
# Regression: this is the inconsistency the consolidation fixed —
76+
# the helper rejects whitespace-only strings, the previous inline
77+
# variants didn't. The helper version is now canonical.
78+
s = _session(messages=[
79+
{"text": "alpha"},
80+
{"text": " "}, # whitespace-only — should be skipped
81+
{"text": "\n\t\n"}, # whitespace-only — should be skipped
82+
{"text": "beta"},
83+
])
84+
assert session_text_for_exclusion(s) == "alpha\n\nbeta"
85+
86+
def test_skips_non_string_text(self):
87+
s = _session(messages=[{"text": "alpha"}, {"text": 42}, {"text": None}, {"text": "beta"}])
88+
assert session_text_for_exclusion(s) == "alpha\n\nbeta"
89+
90+
91+
# ---------------------------------------------------------------------------
92+
# is_session_excluded
93+
# ---------------------------------------------------------------------------
94+
95+
class TestIsSessionExcluded:
96+
97+
def test_returns_false_when_rules_empty(self, tmp_path):
98+
s = _session(title="anything", messages=[{"text": "anything"}])
99+
assert is_session_excluded([], s, "any project") is False
100+
assert is_session_excluded(None, s, "any project") is False # type: ignore[arg-type]
101+
102+
def test_matches_on_project_name(self, tmp_path):
103+
rules = load_rules(_write_rules(tmp_path, "secret-project"))
104+
s = _session()
105+
assert is_session_excluded(rules, s, "my secret-project work") is True
106+
assert is_session_excluded(rules, s, "unrelated work") is False
107+
108+
def test_matches_on_session_title(self, tmp_path):
109+
rules = load_rules(_write_rules(tmp_path, "confidential"))
110+
assert is_session_excluded(rules, _session(title="Confidential debrief"), "proj") is True
111+
assert is_session_excluded(rules, _session(title="Public roadmap"), "proj") is False
112+
113+
def test_matches_on_model_name(self, tmp_path):
114+
rules = load_rules(_write_rules(tmp_path, "claude-opus-4-7"))
115+
s = _session(models=["claude-opus-4-7", "claude-haiku-4-5"])
116+
assert is_session_excluded(rules, s, "proj") is True
117+
118+
def test_matches_on_message_content(self, tmp_path):
119+
rules = load_rules(_write_rules(tmp_path, "password"))
120+
s = _session(messages=[{"text": "do not commit the password"}])
121+
assert is_session_excluded(rules, s, "proj") is True
122+
123+
def test_AND_rule_requires_both_terms(self, tmp_path):
124+
# AND has higher precedence than OR (per the rule grammar).
125+
rules = load_rules(_write_rules(tmp_path, "alpha AND beta"))
126+
s_both = _session(messages=[{"text": "alpha and beta together"}])
127+
s_one = _session(messages=[{"text": "only alpha here"}])
128+
assert is_session_excluded(rules, s_both, "proj") is True
129+
assert is_session_excluded(rules, s_one, "proj") is False
130+
131+
def test_OR_rule_matches_either(self, tmp_path):
132+
rules = load_rules(_write_rules(tmp_path, "alpha OR beta"))
133+
s_alpha = _session(messages=[{"text": "alpha here"}])
134+
s_beta = _session(messages=[{"text": "beta here"}])
135+
s_neither = _session(messages=[{"text": "gamma here"}])
136+
assert is_session_excluded(rules, s_alpha, "proj") is True
137+
assert is_session_excluded(rules, s_beta, "proj") is True
138+
assert is_session_excluded(rules, s_neither, "proj") is False
139+
140+
def test_quoted_phrase_match(self, tmp_path):
141+
rules = load_rules(_write_rules(tmp_path, '"project alpha"'))
142+
s_match = _session(title="Project alpha kickoff")
143+
s_partial = _session(title="alpha project") # token order matters
144+
assert is_session_excluded(rules, s_match, "proj") is True
145+
assert is_session_excluded(rules, s_partial, "proj") is False
146+
147+
def test_handles_session_without_metadata(self, tmp_path):
148+
# Defensive: session dicts coming from older code paths might be
149+
# missing a metadata key. Should not raise.
150+
rules = load_rules(_write_rules(tmp_path, "anything"))
151+
bare = {"title": "x", "messages": []} # no metadata key at all
152+
assert is_session_excluded(rules, bare, "proj") is False
153+
154+
def test_project_name_None_does_not_break(self, tmp_path):
155+
rules = load_rules(_write_rules(tmp_path, "confidential"))
156+
s = _session(title="Confidential")
157+
# project_name=None should still let title-based rules match.
158+
assert is_session_excluded(rules, s, None) is True

0 commit comments

Comments
 (0)