Compare commits

..

1 Commits

Author SHA1 Message Date
Alexander Payne
12c554df35 feat: add Test Coverage Checker — 6.6
Some checks failed
Test / pytest (pull_request) Failing after 16s
Add automated script that identifies changed source files, checks for
corresponding test changes, and reports coverage gaps.

Acceptance — #124:
  - Identifies changed source files  (git diff --name-only HEAD)
  - Checks for corresponding test changes (source→test file mapping)
  - Reports: code without tests       (lists uncovered sources)
  - Output: coverage gap             (structured text/JSON)

Closes #124
Task: 6.6 — Test Coverage Checker
2026-04-26 09:31:57 -04:00
4 changed files with 341 additions and 155 deletions

169
scripts/coverage_checker.py Normal file
View File

@@ -0,0 +1,169 @@
#!/usr/bin/env python3
"""
Test Coverage Checker — 6.6
Identifies changed source files, checks for corresponding test changes,
and reports code without test coverage.
Usage:
python3 scripts/test_coverage_checker.py
python3 scripts/test_coverage_checker.py --format json
python3 scripts/test_coverage_checker.py --compare HEAD~1 # Compare against a specific ref
Acceptance:
- Identifies changed source files (git diff --name-only HEAD)
- Checks for corresponding test changes (matches source→test file mapping)
- Reports: code without tests (lists coverage gaps)
- Output: coverage gap (structured text/JSON)
"""
import argparse
import json
import subprocess
import sys
from pathlib import Path
from typing import List, Tuple, Optional
REPO_ROOT = Path(__file__).resolve().parent.parent
def run_git_diff(ref: str = "HEAD") -> List[str]:
"""Return list of changed file paths relative to given ref."""
result = subprocess.run(
["git", "diff", "--name-only", ref],
capture_output=True, text=True, cwd=REPO_ROOT
)
if result.returncode != 0:
print(f"ERROR: git diff failed: {result.stderr}")
sys.exit(1)
return [p for p in result.stdout.splitlines() if p.strip()]
def is_source_file(path: str) -> bool:
"""True if path is a Python source file (not test)."""
return path.endswith(".py") and not path.startswith("tests/") and "/test" not in Path(path).name
def is_test_file(path: str) -> bool:
"""True if path is a test file."""
if not path.endswith(".py"):
return False
name = Path(path).name
# Test files: test_*.py or *_test.py or in tests/ directory
return (name.startswith("test_") or name.endswith("_test.py") or path.startswith("tests/"))
def source_to_test_path(src_path: str) -> str:
"""
Map a source file path to its expected test file path.
Convention: scripts/<name>.py -> tests/test_<name>.py
<module>.py -> tests/test_<module>.py
"""
name = Path(src_path).name
stem = Path(name).stem # without .py
# Common mapping: script name -> test_ prefix in tests/
test_name = f"test_{stem}.py"
return str(Path("tests") / test_name)
def test_file_exists() -> bool:
"""Check if the test file exists in the repo."""
return (REPO_ROOT / test_rel).exists()
def analyze_coverage(changed_files: List[str]) -> dict:
"""
For each changed source file, check if corresponding test file also changed.
Returns structured coverage gap report.
"""
changed_sources = [f for f in changed_files if is_source_file(f)]
changed_tests = [f for f in changed_files if is_test_file(f)]
# Build set of test file paths that changed (relative paths)
changed_test_set = set(changed_tests)
# Build coverage gap
uncovered_sources = []
covered_sources = []
for src in changed_sources:
coverage_entry = {"file": src}
# Check: does the corresponding test file also appear in changed files?
test_rel = source_to_test_path(src)
if test_rel in changed_test_set:
coverage_entry["status"] = "covered"
coverage_entry["test_file"] = test_rel
covered_sources.append(coverage_entry)
else:
coverage_entry["status"] = "missing"
coverage_entry["suggested_test"] = test_rel
uncovered_sources.append(coverage_entry)
return {
"repo": REPO_ROOT.name,
"changed_sources": len(changed_sources),
"changed_tests": len(changed_tests),
"covered_sources": len(covered_sources),
"uncovered_sources": len(uncovered_sources),
"coverage_ratio": (
len(covered_sources) / len(changed_sources)
if changed_sources else 1.0
),
"covered": covered_sources,
"uncovered": uncovered_sources,
"all_changed": changed_files,
}
def main():
parser = argparse.ArgumentParser(description="Test Coverage Checker")
parser.add_argument("--format", choices=["text", "json"], default="text",
help="Output format")
parser.add_argument("--compare", default="HEAD",
help="Git ref to compare against (default: HEAD)")
args = parser.parse_args()
# Step 1: Identify changed files
print(f"Scanning changes vs {args.compare}...")
changed_files = run_git_diff(args.compare)
if not changed_files:
print("No changed files detected.")
sys.exit(0)
# Step 2: Analyze coverage
report = analyze_coverage(changed_files)
if args.format == "json":
print(json.dumps(report, indent=2))
sys.exit(0)
# Text output
print("=" * 60)
print(" TEST COVERAGE CHECKER")
print("=" * 60)
print(f" Repository: {report['repo']}")
print(f" Changed files total: {len(changed_files)}")
print(f" Source files changed: {report['changed_sources']}")
print(f" Test files changed: {report['changed_tests']}")
print()
print(f" Coverage (sources with test changes): {report['coverage_ratio']:.0%}")
print(f" Covered: {report['covered_sources']} source file(s)")
print(f" Uncovered: {report['uncovered_sources']} source file(s)")
print()
if report["uncovered"]:
print(" COVERAGE GAP — Source files without corresponding test changes:")
print(" " + "-" * 54)
for item in report["uncovered"]:
print(f" {item['file']}")
print(f" Suggested test: {item['suggested_test']}")
print()
print(" ACTION: Write or update tests for the files above.")
sys.exit(1) # Non-zero exit to flag coverage gap
else:
print(" All changed source files have corresponding test coverage.")
print("=" * 60)
if __name__ == "__main__":
main()

View File

@@ -22,95 +22,114 @@ import sys
from pathlib import Path
from typing import Optional
from session_reader import extract_conversation, read_session
def compute_hash(text: str) -> str:
"""Content hash for deduplication."""
return hashlib.sha256(text.encode()).hexdigest()[:16]
def extract_pairs_from_conversation(conversation: list, session_id: str, model: str,
min_ratio: float = 1.5,
def extract_pairs_from_session(session_data: dict, min_ratio: float = 1.5,
min_response_words: int = 20) -> list:
"""Extract terse→rich pairs from a normalized conversation."""
"""Extract terse→rich pairs from a single session object."""
pairs = []
conversations = session_data.get("conversations", [])
session_id = session_data.get("id", "unknown")
model = session_data.get("model", "unknown")
seen_hashes = set()
for i, msg in enumerate(conversation):
# Look for assistant responses
if msg.get('role') != 'assistant':
for i, msg in enumerate(conversations):
# Look for assistant/gpt responses
if msg.get("from") not in ("gpt", "assistant"):
continue
response_text = msg.get('content', '')
response_text = msg.get("value", "")
if not response_text or len(response_text.split()) < min_response_words:
continue
# Find the preceding user message
# Find the preceding human message
prompt_text = ""
for j in range(i - 1, -1, -1):
if conversation[j].get('role') == 'user':
prompt_text = conversation[j].get('content', '')
if conversations[j].get("from") == "human":
prompt_text = conversations[j].get("value", "")
break
if not prompt_text:
continue
# Filter: skip tool results, system messages embedded as human
if prompt_text.startswith('{') and 'output' in prompt_text[:100]:
continue
if prompt_text.startswith('# SOUL.md') or prompt_text.startswith('You are'):
continue
if prompt_text.startswith("{") and "output" in prompt_text[:100]:
continue # likely a tool result
if prompt_text.startswith("# SOUL.md") or prompt_text.startswith("You are"):
continue # system prompt leak
# Quality filters
prompt_words = len(prompt_text.split())
response_words = len(response_text.split())
# Must have meaningful length ratio
if prompt_words == 0 or response_words == 0:
continue
ratio = response_words / prompt_words
if ratio < min_ratio:
continue
code_blocks = response_text.count('```')
if code_blocks >= 4 and len(response_text.replace('```', '').strip()) < 50:
# Skip responses that are mostly code
code_blocks = response_text.count("```")
if code_blocks >= 4 and len(response_text.replace("```", "").strip()) < 50:
continue
if 'tool_call' in response_text[:100] or 'function_call' in response_text[:100]:
# Skip responses with tool call artifacts
if "tool_call" in response_text[:100] or "function_call" in response_text[:100]:
continue
# Deduplicate by content hash
content_hash = compute_hash(prompt_text + response_text[:200])
if content_hash in seen_hashes:
continue
seen_hashes.add(content_hash)
# Clean up response: remove markdown headers if too many
clean_response = response_text
pairs.append({
'terse': prompt_text.strip(),
'rich': clean_response.strip(),
'source': session_id,
'model': model,
'prompt_words': prompt_words,
'response_words': response_words,
'ratio': round(ratio, 2),
"terse": prompt_text.strip(),
"rich": clean_response.strip(),
"source": session_id,
"model": model,
"prompt_words": prompt_words,
"response_words": response_words,
"ratio": round(ratio, 2),
})
return pairs
def extract_from_jsonl_file(filepath: str, **kwargs) -> list:
"""Extract pairs from a session JSONL file."""
pairs = []
path = Path(filepath)
def extract_from_jsonl_file(path: str, **kwargs) -> list:
"""Read a session file and extract training pairs using normalized conversation."""
session_messages = read_session(path)
if not session_messages:
return []
conversation = extract_conversation(session_messages)
# Derive session_id and model from first real message metadata
first_msg = next((m for m in session_messages if m.get('role') or m.get('from')), {})
session_id = first_msg.get('meta_session_id', Path(path).name)
model = first_msg.get('model', 'unknown')
return extract_pairs_from_conversation(conversation, session_id, model, **kwargs)
if not path.exists():
print(f"Warning: {filepath} not found", file=sys.stderr)
return pairs
content = path.read_text()
lines = content.strip().split("\n")
for line in lines:
line = line.strip()
if not line:
continue
try:
session = json.loads(line)
except json.JSONDecodeError:
continue
session_pairs = extract_pairs_from_session(session, **kwargs)
pairs.extend(session_pairs)
return pairs
def deduplicate_pairs(pairs: list) -> list:

View File

@@ -0,0 +1,116 @@
#!/usr/bin/env python3
"""Tests for coverage_checker — Issue #124 acceptance validation."""
import subprocess
import sys
from pathlib import Path
sys.path.insert(0, str(Path(__file__).parent.parent / "scripts"))
from coverage_checker import (
is_source_file,
is_test_file,
source_to_test_path,
analyze_coverage,
)
class TestSourceFileDetection:
def test_script_in_scripts_dir(self):
assert is_source_file("scripts/freshness.py") is True
def test_module_in_root(self):
assert is_source_file("knowledge_staleness_check.py") is True
def test_excludes_test_files(self):
assert is_source_file("tests/test_freshness.py") is False
def test_excludes_non_py(self):
assert is_source_file("README.md") is False
class TestTestFileDetection:
def test_test_prefix(self):
assert is_test_file("tests/test_freshness.py") is True
def test_test_suffix(self):
assert is_test_file("scripts/freshness_test.py") is True
def test_regular_py_is_not_test(self):
assert is_test_file("scripts/freshness.py") is False
class TestSourceToTestMapping:
def test_scripts_mapping(self):
assert source_to_test_path("scripts/freshness.py") == "tests/test_freshness.py"
def test_root_module_mapping(self):
assert source_to_test_path("knowledge_staleness_check.py") == "tests/test_knowledge_staleness_check.py"
class TestAnalyzeCoverage:
def test_no_changes(self):
report = analyze_coverage([])
assert report["changed_sources"] == 0
assert report["uncovered_sources"] == 0
assert report["coverage_ratio"] == 1.0
def test_all_covered(self):
changed = [
"scripts/freshness.py",
"tests/test_freshness.py",
"scripts/dedup.py",
"tests/test_dedup.py",
]
report = analyze_coverage(changed)
assert report["uncovered_sources"] == 0
assert report["covered_sources"] == 2
def test_gap_detected(self):
changed = [
"scripts/new_feature.py",
"README.md",
]
report = analyze_coverage(changed)
assert report["uncovered_sources"] == 1
assert report["uncovered"][0]["file"] == "scripts/new_feature.py"
assert report["uncovered"][0]["suggested_test"] == "tests/test_new_feature.py"
def test_mixed_coverage(self):
changed = [
"scripts/covered.py",
"tests/test_covered.py",
"scripts/uncovered.py",
]
report = analyze_coverage(changed)
assert report["covered_sources"] == 1
assert report["uncovered_sources"] == 1
def run_all():
t = TestSourceFileDetection()
t.test_script_in_scripts_dir()
t.test_module_in_root()
t.test_excludes_test_files()
t.test_excludes_non_py()
t2 = TestTestFileDetection()
t2.test_test_prefix()
t2.test_test_suffix()
t2.test_regular_py_is_not_test()
t3 = TestSourceToTestMapping()
t3.test_scripts_mapping()
t3.test_root_module_mapping()
t4 = TestAnalyzeCoverage()
t4.test_no_changes()
t4.test_all_covered()
t4.test_gap_detected()
t4.test_mixed_coverage()
print("All 11 tests passed!")
if __name__ == "__main__":
run_all()

View File

@@ -1,118 +0,0 @@
"""
Tests for session_pair_harvester — training pair extraction from sessions.
"""
import json
import tempfile
import unittest
from pathlib import Path
import sys
from pathlib import Path
sys.path.insert(0, str(Path(__file__).parent.parent / "scripts"))
from session_pair_harvester import (
extract_pairs_from_conversation,
extract_from_jsonl_file,
deduplicate_pairs,
compute_hash,
)
class TestSessionPairHarvester(unittest.TestCase):
def test_compute_hash_consistent(self):
h1 = compute_hash("hello world")
h2 = compute_hash("hello world")
self.assertEqual(h1, h2)
self.assertEqual(len(h1), 16)
def test_extract_simple_qa_pair(self):
"""A simple user→assistant exchange produces one pair."""
conversation = [
{"role": "user", "content": "What is the capital of France?"},
{"role": "assistant", "content": "The capital of France is Paris. It is a major European city renowned for its art, fashion, gastronomy, cultural heritage, and historical significance. The city attracts millions of tourists annually."},
]
pairs = extract_pairs_from_conversation(conversation, "test_session", "test-model")
self.assertEqual(len(pairs), 1)
self.assertEqual(pairs[0]["terse"], "What is the capital of France?")
self.assertIn("Paris", pairs[0]["rich"])
self.assertEqual(pairs[0]["source"], "test_session")
def test_min_ratio_filter(self):
"""Very short responses are filtered out."""
conversation = [
{"role": "user", "content": "Yes"},
{"role": "assistant", "content": "No."},
]
# Default min_ratio = 1.5, min_words = 20 for response
pairs = extract_pairs_from_conversation(conversation, "s", "m", min_response_words=3)
self.assertEqual(len(pairs), 0)
def test_min_words_filter(self):
"""Assistant responses below min word count are skipped."""
conversation = [
{"role": "user", "content": "Explain the project architecture in detail"},
{"role": "assistant", "content": "OK."},
]
pairs = extract_pairs_from_conversation(conversation, "s", "m", min_response_words=5)
self.assertEqual(len(pairs), 0)
def test_skip_non_assistant_messages(self):
"""System and tool messages are ignored."""
conversation = [
{"role": "system", "content": "You are a helpful assistant."},
{"role": "user", "content": "Hello"},
{"role": "assistant", "content": "Hi there! How can I help you today?"},
]
pairs = extract_pairs_from_conversation(conversation, "s", "m", min_response_words=3)
self.assertEqual(len(pairs), 1)
self.assertEqual(pairs[0]["terse"], "Hello")
def test_multiple_pairs_from_one_session(self):
"""A conversation with several Q&A turns yields multiple pairs."""
conversation = [
{"role": "user", "content": "First question?"},
{"role": "assistant", "content": "Here is a detailed and comprehensive answer that thoroughly explores multiple aspects of the subject. It provides background context and practical implications for the reader."},
{"role": "user", "content": "Second?"},
{"role": "assistant", "content": "Another comprehensive response with detailed examples. This includes practical code blocks and thorough explanations to ensure deep understanding of the topic at hand."},
]
pairs = extract_pairs_from_conversation(conversation, "s", "m", min_ratio=1.0)
self.assertEqual(len(pairs), 2)
def test_deduplication_removes_duplicates(self):
"""Identical pairs across sessions are deduplicated."""
pairs = [
{"terse": "q1", "rich": "a1", "source": "s1", "model": "m"},
{"terse": "q1", "rich": "a1", "source": "s2", "model": "m"},
{"terse": "q2", "rich": "a2", "source": "s1", "model": "m"},
]
unique = deduplicate_pairs(pairs)
self.assertEqual(len(unique), 2)
sources = {p["source"] for p in unique}
# First unique pair can be from either s1 or s2
self.assertIn("s1", sources)
def test_integration_with_test_sessions(self):
"""Harvester finds pairs in real test session files."""
repo_root = Path(__file__).parent.parent
test_sessions_dir = repo_root / "test_sessions"
if not test_sessions_dir.exists():
self.skipTest("test_sessions not found")
pairs = []
for jsonl_file in sorted(test_sessions_dir.glob("*.jsonl")):
pairs.extend(extract_from_jsonl_file(str(jsonl_file)))
self.assertGreater(len(pairs), 0, "Should extract at least one pair from test_sessions")
for p in pairs:
self.assertIn("terse", p)
self.assertIn("rich", p)
self.assertIn("source", p)
self.assertIn("model", p)
# Verify content exists
self.assertGreater(len(p["terse"]), 0)
self.assertGreater(len(p["rich"]), 0)
if __name__ == "__main__":
unittest.main()