diff --git a/src/instinct/store.py b/src/instinct/store.py index ba94e90..28f26a0 100644 --- a/src/instinct/store.py +++ b/src/instinct/store.py @@ -632,14 +632,16 @@ def trending(self, days: int = 7, limit: int = 10) -> list[dict]: from datetime import timedelta cutoff = (datetime.now(timezone.utc) - timedelta(days=days)).isoformat() - # Real delta: count observations in the period from confidence_log - log_rows = self._conn.execute( - """SELECT pattern, COUNT(*) as observations - FROM confidence_log WHERE ts >= ? - GROUP BY pattern ORDER BY observations DESC - LIMIT ?""", - (cutoff, limit), - ).fetchall() + # Real delta: count observations in the period from confidence_log. + # pattern ASC is the deterministic group-key tiebreaker for aggregate + # rows; rowid is not meaningful after GROUP BY collapses source rows. + log_rows = self._conn.execute( + """SELECT pattern, COUNT(*) as observations + FROM confidence_log WHERE ts >= ? + GROUP BY pattern ORDER BY observations DESC, pattern ASC + LIMIT ?""", + (cutoff, limit), + ).fetchall() if log_rows: results = [] @@ -734,16 +736,18 @@ def effectiveness(self, days: int = 30) -> dict: from datetime import timedelta cutoff = (datetime.now(timezone.utc) - timedelta(days=days)).isoformat() - rows = self._conn.execute( - """SELECT pattern, - COUNT(*) as suggested, - SUM(confirmed) as confirmed - FROM suggest_log - WHERE suggested_at >= ? - GROUP BY pattern - ORDER BY confirmed DESC, suggested DESC""", - (cutoff,), - ).fetchall() + # pattern ASC is the deterministic group-key tiebreaker for aggregate + # rows; rowid is not meaningful after GROUP BY collapses source rows. + rows = self._conn.execute( + """SELECT pattern, + COUNT(*) as suggested, + SUM(confirmed) as confirmed + FROM suggest_log + WHERE suggested_at >= ? + GROUP BY pattern + ORDER BY confirmed DESC, suggested DESC, pattern ASC""", + (cutoff,), + ).fetchall() patterns = [] total_suggested = 0 @@ -831,12 +835,14 @@ def stats(self) -> dict: d = dict(rows) result = {k: (v or 0) for k, v in d.items()} - # Per-category breakdown - cat_rows = self._conn.execute(""" - SELECT category, COUNT(*) as count, - AVG(confidence) as avg_confidence - FROM instincts GROUP BY category ORDER BY count DESC - """).fetchall() + # Per-category breakdown. + # category ASC is the deterministic group-key tiebreaker for aggregate + # rows; rowid is not meaningful after GROUP BY collapses source rows. + cat_rows = self._conn.execute(""" + SELECT category, COUNT(*) as count, + AVG(confidence) as avg_confidence + FROM instincts GROUP BY category ORDER BY count DESC, category ASC + """).fetchall() result["by_category"] = { r["category"]: {"count": r["count"], "avg_confidence": round(r["avg_confidence"], 1)} for r in cat_rows diff --git a/tests/test_store.py b/tests/test_store.py index ff0f127..af91201 100644 --- a/tests/test_store.py +++ b/tests/test_store.py @@ -1191,7 +1191,7 @@ def test_export_all_deterministic_under_confidence_collision(): shutil.rmtree(d, ignore_errors=True) -def test_confirm_suggestion_picks_newest_id_under_suggested_at_collision(): +def test_confirm_suggestion_picks_newest_id_under_suggested_at_collision(): """_confirm_suggestion() picks the most recent unconfirmed entry via ORDER BY suggested_at DESC LIMIT 1. Without an id-DESC tiebreaker, rapid suggest() calls within one millisecond create indistinguishable @@ -1227,7 +1227,102 @@ def test_confirm_suggestion_picks_newest_id_under_suggested_at_collision(): ).fetchall() ]) assert confirmed_ids == [2, 3] - finally: - store.close() - shutil.rmtree(d, ignore_errors=True) + finally: + store.close() + shutil.rmtree(d, ignore_errors=True) + + +# ── Aggregate-query tiebreakers (PR #36 §3 follow-up) ── +# These are the GROUP BY siblings to the rowid/id tiebreaker fixes from +# #30/#33/#34/#36. Aggregate rows have no meaningful rowid after GROUP BY +# collapses source rows, so the deterministic tiebreaker is the group key ASC. + + +def test_trending_real_delta_ties_break_by_pattern_asc(): + """trending() real-delta path groups confidence_log rows by pattern. + Equal observation counts must sort by pattern ASC, not source-row order.""" + store, d = _make_store() + try: + for pattern in ("pat:gamma", "pat:alpha", "pat:beta"): + store.observe(pattern) + store.observe(pattern) + + first = [r["pattern"] for r in store.trending(days=1, limit=10)] + second = [r["pattern"] for r in store.trending(days=1, limit=10)] + + assert first == second, ( + f"trending() aggregate ordering not deterministic: " + f"{first} vs {second}" + ) + assert first[:3] == ["pat:alpha", "pat:beta", "pat:gamma"], ( + f"trending() did not break observation-count ties by pattern ASC: " + f"{first}" + ) + finally: + store.close() + shutil.rmtree(d, ignore_errors=True) + + +def test_effectiveness_ties_break_by_pattern_asc(): + """effectiveness() groups suggest_log rows by pattern. Equal + (confirmed, suggested) tuples must sort by pattern ASC.""" + store, d = _make_store() + try: + ts = "2026-04-25T12:00:00+00:00" + for pattern in ("pat:gamma", "pat:alpha", "pat:beta"): + for _ in range(2): + store._conn.execute( + "INSERT INTO suggest_log " + "(pattern, suggested_at, confirmed, confirmed_at) " + "VALUES (?, ?, 1, ?)", + (pattern, ts, ts), + ) + store._conn.commit() + + first = [r["pattern"] for r in store.effectiveness(days=365)["patterns"]] + second = [r["pattern"] for r in store.effectiveness(days=365)["patterns"]] + + assert first == second, ( + f"effectiveness() aggregate ordering not deterministic: " + f"{first} vs {second}" + ) + assert first == ["pat:alpha", "pat:beta", "pat:gamma"], ( + f"effectiveness() did not break aggregate ties by pattern ASC: " + f"{first}" + ) + finally: + store.close() + shutil.rmtree(d, ignore_errors=True) + + +def test_stats_by_category_ties_break_by_category_asc(): + """stats().by_category groups instincts by category. Equal category + counts must sort by category ASC for stable display output.""" + store, d = _make_store() + try: + ts = "2026-04-25T12:00:00+00:00" + for category in ("preference", "combo", "fix_pattern"): + for i in range(2): + store._conn.execute( + "INSERT INTO instincts (pattern, category, confidence, " + "first_seen, last_seen, source, project, promoted, metadata) " + "VALUES (?, ?, 1, ?, ?, '', '', 0, '{}')", + (f"pat:{category}-{i}", category, ts, ts), + ) + store._conn.commit() + + first = list(store.stats()["by_category"]) + second = list(store.stats()["by_category"]) + + assert first == second, ( + f"stats().by_category ordering not deterministic: " + f"{first} vs {second}" + ) + assert first == ["combo", "fix_pattern", "preference"], ( + f"stats().by_category did not break count ties by category ASC: " + f"{first}" + ) + finally: + store.close() + shutil.rmtree(d, ignore_errors=True)