Compare commits
1 Commits
step35/91-
...
step35/124
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
12c554df35 |
169
scripts/coverage_checker.py
Normal file
169
scripts/coverage_checker.py
Normal 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()
|
||||
@@ -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:
|
||||
|
||||
116
tests/test_coverage_checker.py
Normal file
116
tests/test_coverage_checker.py
Normal 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()
|
||||
@@ -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()
|
||||
|
||||
Reference in New Issue
Block a user