Compare commits
1 Commits
step35/91-
...
step35/126
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
e2b1a9f8ac |
185
scripts/review_comment_generator.py
Executable file
185
scripts/review_comment_generator.py
Executable file
@@ -0,0 +1,185 @@
|
||||
#!/usr/bin/env python3
|
||||
"""
|
||||
Review Comment Generator — Issue #126
|
||||
Reads JSONL findings, deduplicates, posts as Gitea PR comments.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import argparse
|
||||
import hashlib
|
||||
import json
|
||||
import os
|
||||
import sys
|
||||
import urllib.request
|
||||
import urllib.error
|
||||
from datetime import datetime, timezone
|
||||
from pathlib import Path
|
||||
from typing import Dict, List, Optional
|
||||
|
||||
SCRIPT_DIR = Path(__file__).resolve().parent
|
||||
REPO_ROOT = SCRIPT_DIR.parent
|
||||
|
||||
DEFAULT_API_BASE = os.environ.get(
|
||||
"GITEA_API_BASE",
|
||||
"https://forge.alexanderwhitestone.com"
|
||||
)
|
||||
TOKEN_PATHS = [
|
||||
os.path.expanduser("~/.config/gitea/token"),
|
||||
os.path.expanduser("~/.hermes/gitea.token"),
|
||||
os.environ.get("GITEA_TOKEN", ""),
|
||||
]
|
||||
|
||||
def load_token() -> Optional[str]:
|
||||
token = os.environ.get("GITEA_TOKEN", "")
|
||||
if token:
|
||||
return token
|
||||
for path in TOKEN_PATHS:
|
||||
if path and os.path.exists(path):
|
||||
with open(path) as f:
|
||||
t = f.read().strip()
|
||||
if t:
|
||||
return t
|
||||
return None
|
||||
|
||||
class GiteaClient:
|
||||
def __init__(self, base_url: str, token: str, org: str, repo: str):
|
||||
self.base_url = base_url.rstrip("/")
|
||||
self.token = token
|
||||
self.org = org
|
||||
self.repo = repo
|
||||
|
||||
def _post(self, path: str, data: Dict) -> Optional[Dict]:
|
||||
url = f"{self.base_url}/api/v1{path}"
|
||||
body = json.dumps(data).encode("utf-8")
|
||||
req = urllib.request.Request(url, data=body, method="POST")
|
||||
req.add_header("Authorization", f"token {self.token}")
|
||||
req.add_header("Content-Type", "application/json")
|
||||
try:
|
||||
with urllib.request.urlopen(req, timeout=30) as resp:
|
||||
return json.loads(resp.read().decode())
|
||||
except urllib.error.HTTPError as e:
|
||||
err = e.read().decode() if e.read() else str(e)
|
||||
print(f"[ERROR] HTTP {e.code}: {err}", file=sys.stderr)
|
||||
return None
|
||||
except Exception as e:
|
||||
print(f"[ERROR] {e}", file=sys.stderr)
|
||||
return None
|
||||
|
||||
def post_issue_comment(self, issue_num: int, body: str) -> Optional[Dict]:
|
||||
return self._post(
|
||||
f"/repos/{self.org}/{self.repo}/issues/{issue_num}/comments",
|
||||
{"body": body}
|
||||
)
|
||||
|
||||
def content_hash(finding: Dict) -> str:
|
||||
key = f"{finding['file']}:{finding['line']}:{finding['text']}"
|
||||
return hashlib.sha256(key.encode("utf-8")).hexdigest()
|
||||
|
||||
def format_comment(finding: Dict) -> str:
|
||||
emoji = {
|
||||
"error": "🛑",
|
||||
"warning": "⚠️",
|
||||
"info": "ℹ️",
|
||||
}.get(finding.get("severity", ""), "📝")
|
||||
f = finding["file"]
|
||||
ln = finding["line"]
|
||||
txt = finding["text"]
|
||||
return f"{emoji} **Review Comment**\n\nFile: `{f}`\nLine: {ln}\n\n> {txt}\n"
|
||||
|
||||
def load_findings(path: Optional[Path], from_stdin: bool) -> List[Dict]:
|
||||
import fileinput
|
||||
findings = []
|
||||
sources = ["-"] if from_stdin else [str(path)]
|
||||
for line in fileinput.input(files=sources):
|
||||
line = line.strip()
|
||||
if not line or line.startswith("#"):
|
||||
continue
|
||||
try:
|
||||
f = json.loads(line)
|
||||
for key in ("file", "line", "text"):
|
||||
if key not in f:
|
||||
raise ValueError(f"Missing key: {key}")
|
||||
findings.append(f)
|
||||
except json.JSONDecodeError as e:
|
||||
print(f"WARNING: Skipping invalid JSON: {e}", file=sys.stderr)
|
||||
return findings
|
||||
|
||||
def main() -> int:
|
||||
parser = argparse.ArgumentParser(
|
||||
description="Post review findings as comments to a Gitea PR/issue"
|
||||
)
|
||||
parser.add_argument("--pr", type=int, required=True, help="PR/issue number")
|
||||
parser.add_argument("--org", default="Timmy_Foundation", help="Gitea org")
|
||||
parser.add_argument("--repo", default="compounding-intelligence", help="Repo name")
|
||||
parser.add_argument("--api-base", default=DEFAULT_API_BASE, help="Gitea API base")
|
||||
parser.add_argument("--token", default=None, help="API token (or env/file)")
|
||||
parser.add_argument("--input", type=Path, default=None, help="JSONL input file")
|
||||
parser.add_argument("--stdin", action="store_true", help="Read from stdin")
|
||||
parser.add_argument("--dry-run", action="store_true", help="Show without posting")
|
||||
parser.add_argument("--json", action="store_true", help="Emit JSON report")
|
||||
|
||||
args = parser.parse_args()
|
||||
|
||||
if not args.stdin and args.input is None:
|
||||
print("ERROR: --input or --stdin required", file=sys.stderr)
|
||||
return 1
|
||||
if args.stdin and args.input:
|
||||
print("ERROR: --stdin and --input exclusive", file=sys.stderr)
|
||||
return 1
|
||||
|
||||
token = args.token or load_token()
|
||||
if not token:
|
||||
print("ERROR: Token not found. Set GITEA_TOKEN or ~/.config/gitea/token", file=sys.stderr)
|
||||
return 1
|
||||
|
||||
findings = load_findings(args.input, args.stdin)
|
||||
if not findings:
|
||||
print("ERROR: No findings loaded", file=sys.stderr)
|
||||
return 1
|
||||
|
||||
if not args.json: print(f"Loaded {len(findings)} finding(s)")
|
||||
|
||||
seen: Dict[str, Dict] = {}
|
||||
for f in findings:
|
||||
h = content_hash(f)
|
||||
if h not in seen:
|
||||
seen[h] = f
|
||||
|
||||
unique = list(seen.values())
|
||||
if not args.json: print(f"After dedup: {len(unique)} unique")
|
||||
|
||||
if args.json:
|
||||
report = {
|
||||
"total": len(findings),
|
||||
"unique": len(unique),
|
||||
"findings": unique,
|
||||
"generated_at": datetime.now(timezone.utc).isoformat(),
|
||||
}
|
||||
print(json.dumps(report, indent=2))
|
||||
return 0
|
||||
|
||||
if args.dry_run:
|
||||
print("\n=== DRY RUN — would post ===")
|
||||
for i, f in enumerate(unique, 1):
|
||||
print(f"\n--- Comment {i}/{len(unique)} ---")
|
||||
print(format_comment(f))
|
||||
return 0
|
||||
|
||||
client = GiteaClient(args.api_base, token, args.org, args.repo)
|
||||
posted = 0
|
||||
for f in unique:
|
||||
body = format_comment(f)
|
||||
result = client.post_issue_comment(args.pr, body)
|
||||
if result:
|
||||
print(f"✅ Posted: {f['file']}:{f['line']} (id={result.get('id')})")
|
||||
posted += 1
|
||||
else:
|
||||
print(f"❌ Failed: {f['file']}:{f['line']}")
|
||||
|
||||
print(f"\nPosted {posted}/{len(unique)} to PR #{args.pr}")
|
||||
return 0 if posted == len(unique) else 1
|
||||
|
||||
if __name__ == "__main__":
|
||||
sys.exit(main())
|
||||
|
||||
5
scripts/sample_findings.jsonl
Normal file
5
scripts/sample_findings.jsonl
Normal file
@@ -0,0 +1,5 @@
|
||||
{"file": "scripts/harvester.py", "line": 47, "text": "Consider adding type hints to improve readability", "severity": "info"}
|
||||
{"file": "scripts/dedup.py", "line": 89, "text": "Add null check before accessing fact['confidence'] to avoid KeyError", "severity": "warning"}
|
||||
{"file": "scripts/bootstrapper.py", "line": 102, "text": "This loop is O(n^2) — could be optimized with a dict lookup", "severity": "info"}
|
||||
{"file": "scripts/harvester.py", "line": 47, "text": "Consider adding type hints to improve readability", "severity": "info"}
|
||||
{"file": "scripts/harvester.py", "line": 120, "text": "File handle not closed in error path — use context manager", "severity": "error"}
|
||||
@@ -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:
|
||||
|
||||
234
tests/test_review_comment_generator.py
Normal file
234
tests/test_review_comment_generator.py
Normal file
@@ -0,0 +1,234 @@
|
||||
#!/usr/bin/env python3
|
||||
"""
|
||||
Smoke tests for Review Comment Generator — Issue #126
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import subprocess
|
||||
import sys
|
||||
import hashlib
|
||||
from io import StringIO
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parents[1]
|
||||
SCRIPTS_DIR = REPO_ROOT / "scripts"
|
||||
GENERATOR = SCRIPTS_DIR / "review_comment_generator.py"
|
||||
SAMPLE_FINDINGS = SCRIPTS_DIR / "sample_findings.jsonl"
|
||||
|
||||
|
||||
class TestGeneratorPresence:
|
||||
def test_script_exists(self):
|
||||
assert GENERATOR.exists(), f"Missing: {GENERATOR}"
|
||||
|
||||
def test_shebang_is_python(self):
|
||||
with open(GENERATOR) as f:
|
||||
first = f.readline().strip()
|
||||
assert first.startswith("#!"), "No shebang"
|
||||
assert "python" in first.lower()
|
||||
|
||||
|
||||
class TestDeduplication:
|
||||
def test_content_hash_deterministic(self):
|
||||
from hashlib import sha256
|
||||
def ch(f):
|
||||
key = f"{f['file']}:{f['line']}:{f['text']}"
|
||||
return sha256(key.encode()).hexdigest()
|
||||
finding = {"file": "a.py", "line": 1, "text": "test"}
|
||||
assert ch(finding) == ch(finding)
|
||||
|
||||
def test_duplicate_findings_are_removed(self):
|
||||
findings = [
|
||||
{"file": "a.py", "line": 1, "text": "foo", "severity": "info"},
|
||||
{"file": "a.py", "line": 1, "text": "foo", "severity": "warning"},
|
||||
{"file": "b.py", "line": 2, "text": "bar", "severity": "info"},
|
||||
]
|
||||
seen = {}
|
||||
for f in findings:
|
||||
key = f"{f['file']}:{f['line']}:{f['text']}"
|
||||
seen[key] = f
|
||||
assert len(seen) == 2
|
||||
|
||||
def test_different_findings_are_kept(self):
|
||||
findings = [
|
||||
{"file": "a.py", "line": 1, "text": "foo"},
|
||||
{"file": "a.py", "line": 2, "text": "foo"},
|
||||
{"file": "a.py", "line": 1, "text": "bar"},
|
||||
]
|
||||
seen = {}
|
||||
for f in findings:
|
||||
key = f"{f['file']}:{f['line']}:{f['text']}"
|
||||
seen[key] = f
|
||||
assert len(seen) == 3
|
||||
|
||||
|
||||
class TestCommentFormatting:
|
||||
def test_format_basic(self):
|
||||
sys.path.insert(0, str(SCRIPTS_DIR))
|
||||
from review_comment_generator import format_comment
|
||||
f = {"file": "scripts/foo.py", "line": 10, "text": "Fix this bug", "severity": "warning"}
|
||||
body = format_comment(f)
|
||||
assert "📝 **Review Comment**" not in body # warning uses ⚠️
|
||||
assert "⚠️ **Review Comment**" in body
|
||||
assert "`scripts/foo.py`" in body
|
||||
assert "Line: 10" in body
|
||||
assert "> Fix this bug" in body
|
||||
|
||||
def test_format_severity_emoji(self):
|
||||
sys.path.insert(0, str(SCRIPTS_DIR))
|
||||
from review_comment_generator import format_comment
|
||||
cases = [("error", "🛑"), ("warning", "⚠️"), ("info", "ℹ️"), ("unknown", "📝")]
|
||||
for severity, emoji in cases:
|
||||
f = {"file": "x.py", "line": 1, "text": "test", "severity": severity}
|
||||
assert emoji in format_comment(f)
|
||||
|
||||
|
||||
class TestFindingsLoader:
|
||||
def test_load_from_file(self):
|
||||
sys.path.insert(0, str(SCRIPTS_DIR))
|
||||
from review_comment_generator import load_findings
|
||||
findings = load_findings(SAMPLE_FINDINGS, from_stdin=False)
|
||||
assert len(findings) >= 4
|
||||
|
||||
def test_load_ignores_blank_and_comments(self):
|
||||
import tempfile, os
|
||||
with tempfile.NamedTemporaryFile(mode="w", suffix=".jsonl", delete=False) as tf:
|
||||
tf.write('{"file":"a.py","line":1,"text":"valid"}\n')
|
||||
tf.write('\n')
|
||||
tf.write('# this is a comment\n')
|
||||
tf.write('{"file":"b.py","line":2,"text":"also valid"}\n')
|
||||
tfname = tf.name
|
||||
try:
|
||||
sys.path.insert(0, str(SCRIPTS_DIR))
|
||||
from review_comment_generator import load_findings
|
||||
assert len(load_findings(Path(tfname), from_stdin=False)) == 2
|
||||
finally:
|
||||
os.unlink(tfname)
|
||||
|
||||
def test_invalid_json_line_skipped(self, capsys):
|
||||
import tempfile, os
|
||||
with tempfile.NamedTemporaryFile(mode="w", suffix=".jsonl", delete=False) as tf:
|
||||
tf.write('invalid json\n')
|
||||
tf.write('{"file":"ok.py","line":1,"text":"valid"}\n')
|
||||
tfname = tf.name
|
||||
try:
|
||||
sys.path.insert(0, str(SCRIPTS_DIR))
|
||||
from review_comment_generator import load_findings
|
||||
assert len(load_findings(Path(tfname), from_stdin=False)) == 1
|
||||
finally:
|
||||
os.unlink(tfname)
|
||||
|
||||
|
||||
class TestDryRunMode:
|
||||
def test_dry_run_counts_unique(self):
|
||||
result = subprocess.run(
|
||||
[sys.executable, str(GENERATOR), "--pr", "126",
|
||||
"--input", str(SAMPLE_FINDINGS), "--dry-run"],
|
||||
capture_output=True, text=True, cwd=REPO_ROOT, timeout=15
|
||||
)
|
||||
assert result.returncode == 0
|
||||
assert "DRY RUN" in result.stdout
|
||||
assert "Review Comment" in result.stdout
|
||||
|
||||
def test_dry_run_shows_all_unique(self):
|
||||
result = subprocess.run(
|
||||
[sys.executable, str(GENERATOR), "--pr", "126",
|
||||
"--input", str(SAMPLE_FINDINGS), "--dry-run"],
|
||||
capture_output=True, text=True, cwd=REPO_ROOT, timeout=15
|
||||
)
|
||||
assert result.stdout.count("--- Comment") == 4
|
||||
|
||||
|
||||
class TestJSONOutputMode:
|
||||
def test_json_flag_emits_valid_json(self):
|
||||
result = subprocess.run(
|
||||
[sys.executable, str(GENERATOR), "--pr", "126",
|
||||
"--input", str(SAMPLE_FINDINGS), "--json"],
|
||||
capture_output=True, text=True, cwd=REPO_ROOT, timeout=15
|
||||
)
|
||||
assert result.returncode == 0
|
||||
payload = json.loads(result.stdout)
|
||||
assert "total" in payload and "unique" in payload and "findings" in payload
|
||||
assert payload["total"] >= payload["unique"]
|
||||
|
||||
def test_json_findings_have_required_fields(self):
|
||||
result = subprocess.run(
|
||||
[sys.executable, str(GENERATOR), "--pr", "126",
|
||||
"--input", str(SAMPLE_FINDINGS), "--json"],
|
||||
capture_output=True, text=True, cwd=REPO_ROOT, timeout=15
|
||||
)
|
||||
payload = json.loads(result.stdout)
|
||||
for f in payload["findings"]:
|
||||
assert "file" in f and "line" in f and "text" in f
|
||||
|
||||
|
||||
class TestGiteaClient:
|
||||
def test_post_issue_comment_builds_correct_url(self):
|
||||
sys.path.insert(0, str(SCRIPTS_DIR))
|
||||
from review_comment_generator import GiteaClient
|
||||
client = GiteaClient("https://example.com", "token123", "MyOrg", "myrepo")
|
||||
assert client.org == "MyOrg" and client.repo == "myrepo"
|
||||
|
||||
def test_generate_comment_body_has_required_fields(self):
|
||||
sys.path.insert(0, str(SCRIPTS_DIR))
|
||||
from review_comment_generator import format_comment
|
||||
f = {"file": "x.py", "line": 5, "text": "Fix this", "severity": "error"}
|
||||
body = format_comment(f)
|
||||
assert "x.py" in body and "5" in body and "Fix this" in body
|
||||
|
||||
|
||||
class TestFullPipeline:
|
||||
def test_end_to_end_json_output(self):
|
||||
result = subprocess.run(
|
||||
[sys.executable, str(GENERATOR), "--pr", "126",
|
||||
"--input", str(SAMPLE_FINDINGS), "--json"],
|
||||
capture_output=True, text=True, cwd=REPO_ROOT, timeout=15
|
||||
)
|
||||
assert result.returncode == 0
|
||||
data = json.loads(result.stdout)
|
||||
assert data["total"] == 5
|
||||
assert data["unique"] == 4
|
||||
f = data["findings"][0]
|
||||
for key in ("file", "line", "text", "severity"):
|
||||
assert key in f
|
||||
|
||||
def test_token_loading_fallback(self):
|
||||
sys.path.insert(0, str(SCRIPTS_DIR))
|
||||
from review_comment_generator import load_token
|
||||
token = load_token()
|
||||
assert token is None or isinstance(token, str)
|
||||
|
||||
|
||||
class TestErrorHandling:
|
||||
def test_missing_input_shows_error(self):
|
||||
result = subprocess.run(
|
||||
[sys.executable, str(GENERATOR), "--pr", "126"],
|
||||
capture_output=True, text=True, cwd=REPO_ROOT, timeout=15
|
||||
)
|
||||
assert result.returncode != 0
|
||||
assert "--input" in result.stderr or "--stdin" in result.stderr
|
||||
|
||||
def test_invalid_json_line_skipped(self):
|
||||
import tempfile, os
|
||||
with tempfile.NamedTemporaryFile(mode="w", suffix=".jsonl", delete=False) as tf:
|
||||
tf.write('invalid json\n')
|
||||
tf.write('{"file":"ok.py","line":1,"text":"valid"}\n')
|
||||
tfname = tf.name
|
||||
try:
|
||||
result = subprocess.run(
|
||||
[sys.executable, str(GENERATOR), "--pr", "126",
|
||||
"--input", tfname, "--json"],
|
||||
capture_output=True, text=True, cwd=REPO_ROOT, timeout=15
|
||||
)
|
||||
data = json.loads(result.stdout)
|
||||
assert data["total"] == 1
|
||||
assert data["unique"] == 1
|
||||
finally:
|
||||
os.unlink(tfname)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
pytest.main([__file__, "-v"])
|
||||
@@ -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