Skip to content

Commit 88f5bba

Browse files
committed
fix: address CodeRabbit review findings
- check_ship_criteria() now iterates required criteria first; missing metrics are blocking failures instead of silently passing - load_seed_queries() missing-file fallback includes narrator key - Add .superpowers/ to .gitignore (runtime state files) - Update test expectation for narrator in seed query fallback
1 parent e2911ca commit 88f5bba

File tree

4 files changed

+22
-14
lines changed

4 files changed

+22
-14
lines changed

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,3 +189,6 @@ data/test_uploads/
189189

190190
# Training pipeline artifacts
191191
training_data/
192+
193+
# Superpowers runtime state
194+
.superpowers/

tests/training/test_seed.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ def test_loads_valid_yaml(self, tmp_path):
3535
def test_returns_empty_on_missing_file(self, tmp_path):
3636
with patch("training.seed.get_school_dir", return_value=tmp_path):
3737
result = load_seed_queries("test-school")
38-
assert result == {"explainer": [], "summarizer": []}
38+
assert result == {"narrator": [], "explainer": [], "summarizer": []}
3939

4040

4141
class TestGenerateSyntheticCoursePairings:

training/eval.py

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -241,19 +241,24 @@ def check_ship_criteria(metrics: dict[str, float], task: str) -> ShipDecision:
241241
blocking_failures: list[CriterionFailure] = []
242242
warnings: list[str] = []
243243

244+
# Check all required criteria — missing metrics are blocking failures
245+
for metric, threshold in criteria.items():
246+
value = metrics.get(metric)
247+
if value is None:
248+
blocking_failures.append(
249+
CriterionFailure(metric=metric, threshold=threshold, actual=0.0)
250+
)
251+
elif value < threshold:
252+
blocking_failures.append(
253+
CriterionFailure(metric=metric, threshold=threshold, actual=value)
254+
)
255+
256+
# Check informational metrics (present in metrics but not in criteria)
244257
for metric, value in metrics.items():
245-
threshold = criteria.get(metric)
246-
if threshold is not None:
247-
if value < threshold:
248-
blocking_failures.append(
249-
CriterionFailure(metric=metric, threshold=threshold, actual=value)
250-
)
251-
else:
252-
# Informational metric — warn if very low
253-
if value < 0.5:
254-
warnings.append(
255-
f"{metric} is low ({value:.3f}) — consider improving before deploying"
256-
)
258+
if metric not in criteria and value < 0.5:
259+
warnings.append(
260+
f"{metric} is low ({value:.3f}) — consider improving before deploying"
261+
)
257262

258263
if blocking_failures:
259264
decision = "no_ship"

training/seed.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ def load_seed_queries(school: str) -> dict[str, list[dict]]:
164164
"""Load seed queries from a school's seed_queries.yaml."""
165165
seed_path = get_school_dir(school) / "seed_queries.yaml"
166166
if not seed_path.exists():
167-
return {"explainer": [], "summarizer": []}
167+
return {"narrator": [], "explainer": [], "summarizer": []}
168168
with seed_path.open("r", encoding="utf-8") as fh:
169169
data = yaml.safe_load(fh) or {}
170170
return {

0 commit comments

Comments
 (0)