Skip to content

Commit cf743f4

Browse files
Yi-111-aYi-111-a
authored andcommitted
fix(sleep): bound --scope invoked ancestor matching to the git root
_project_matches accepted any ancestor of the invoked project, so every session whose cwd was $HOME (or /) was harvested into every project below it. Those unrelated tasks were then mined, replayed and gated as if they belonged to the project, diluting the held-out set and costing replay tokens. An ancestor match is still legitimate when invoking from a subdirectory of a repository, so the clause is bounded rather than dropped: the ancestor must sit inside the invoked project's git root. With no git root the walk stops short of $HOME and /, which is where it previously ran away.
1 parent 79124b3 commit cf743f4

2 files changed

Lines changed: 133 additions & 1 deletion

File tree

‎skillopt_sleep/harvest.py‎

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,41 @@ def _dedup(xs: List[str]) -> List[str]:
309309
)
310310

311311

312+
def _git_root(path: str) -> str:
313+
"""Nearest ancestor of `path` holding a .git entry, else "".
314+
315+
A .git *file* (worktree/submodule) counts too, so `os.path.exists` is used
316+
rather than isdir.
317+
"""
318+
cur = os.path.abspath(path)
319+
while True:
320+
if os.path.exists(os.path.join(cur, ".git")):
321+
return cur
322+
parent = os.path.dirname(cur)
323+
if parent == cur:
324+
return ""
325+
cur = parent
326+
327+
328+
def _ancestor_in_scope(ancestor: str, invoked: str) -> bool:
329+
"""True when a session rooted at `ancestor` may belong to `invoked`.
330+
331+
A session started higher up than the invoked project is in scope only when
332+
it is still inside the invoked project's git root, so invoking from
333+
`repo/sub` keeps the sessions started at `repo`. $HOME and the filesystem
334+
root are never in scope: they are shared by every project, so admitting
335+
them is what previously pulled every $HOME-rooted session into every
336+
project below it. Without a git root the ancestor walk stops there.
337+
"""
338+
home = os.path.abspath(os.path.expanduser("~"))
339+
if ancestor in (home, os.path.abspath(os.sep)):
340+
return False
341+
root = _git_root(invoked)
342+
if root:
343+
return ancestor == root or ancestor.startswith(root + os.sep)
344+
return True
345+
346+
312347
def _project_matches(project: str, scope: Any, invoked: str) -> bool:
313348
if scope == "all":
314349
return True
@@ -319,7 +354,11 @@ def _project_matches(project: str, scope: Any, invoked: str) -> bool:
319354
return True
320355
a = os.path.abspath(project)
321356
b = os.path.abspath(invoked)
322-
return a == b or a.startswith(b + os.sep) or b.startswith(a + os.sep)
357+
if a == b or a.startswith(b + os.sep):
358+
return True
359+
if not b.startswith(a + os.sep):
360+
return False
361+
return _ancestor_in_scope(a, b)
323362

324363

325364
def harvest(
Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
"""Tests for the ``--scope invoked`` ancestor bound in harvest (issue #294).
2+
3+
Pure-stdlib (unittest), deterministic, no API key, no third-party deps.
4+
Run: python -m pytest tests/test_harvest_project_scope.py
5+
"""
6+
from __future__ import annotations
7+
8+
import os
9+
import tempfile
10+
import unittest
11+
from unittest import mock
12+
13+
from skillopt_sleep.harvest import _ancestor_in_scope, _git_root, _project_matches
14+
15+
16+
class TestProjectMatchesInvokedScope(unittest.TestCase):
17+
def test_exact_and_descendant_projects_still_match(self):
18+
self.assertTrue(_project_matches("/repo", "invoked", "/repo"))
19+
self.assertTrue(_project_matches("/repo/pkg", "invoked", "/repo"))
20+
21+
def test_sibling_prefix_does_not_match(self):
22+
# "/repo-tools" is not inside "/repo"
23+
self.assertFalse(_project_matches("/repo-tools", "invoked", "/repo"))
24+
self.assertFalse(_project_matches("/elsewhere", "invoked", "/repo"))
25+
26+
def test_scope_all_ignores_the_ancestor_bound(self):
27+
self.assertTrue(_project_matches("/", "all", "/repo"))
28+
29+
def test_explicit_scope_list_is_unaffected(self):
30+
self.assertTrue(_project_matches("/other", ["/other"], "/repo"))
31+
self.assertFalse(_project_matches("/other", ["/third"], "/repo"))
32+
33+
34+
class TestAncestorInScope(unittest.TestCase):
35+
def test_home_and_filesystem_root_are_never_in_scope(self):
36+
with tempfile.TemporaryDirectory() as tmp:
37+
# tmp is not a git repo, and its ancestors are not either
38+
repo = os.path.join(tmp, "work", "repo")
39+
os.makedirs(repo)
40+
with mock.patch.dict(os.environ, {"HOME": tmp}):
41+
home = os.path.abspath(os.path.expanduser("~"))
42+
self.assertFalse(_ancestor_in_scope(home, repo))
43+
self.assertFalse(_ancestor_in_scope(os.path.abspath(os.sep), repo))
44+
45+
def test_intermediate_ancestor_without_git_root_stays_in_scope(self):
46+
# Invoking from "work/repo" must keep sessions started at "work".
47+
with tempfile.TemporaryDirectory() as tmp:
48+
repo = os.path.join(tmp, "work", "repo")
49+
os.makedirs(repo)
50+
self.assertTrue(_ancestor_in_scope(os.path.join(tmp, "work"), repo))
51+
52+
def test_git_root_stops_the_ancestor_walk(self):
53+
with tempfile.TemporaryDirectory() as tmp:
54+
repo = os.path.join(tmp, "work", "repo")
55+
os.makedirs(os.path.join(repo, "sub"))
56+
os.makedirs(os.path.join(repo, ".git"))
57+
above = os.path.join(tmp, "work")
58+
59+
# "repo" is the git root: a session started there still matches
60+
# an invocation from "repo/sub".
61+
self.assertTrue(_ancestor_in_scope(repo, os.path.join(repo, "sub")))
62+
# "work" is above the git root, so it no longer matches.
63+
self.assertFalse(_ancestor_in_scope(above, os.path.join(repo, "sub")))
64+
65+
def test_git_root_detected_from_a_subdirectory(self):
66+
with tempfile.TemporaryDirectory() as tmp:
67+
repo = os.path.join(tmp, "repo")
68+
os.makedirs(os.path.join(repo, "a", "b"))
69+
os.makedirs(os.path.join(repo, ".git"))
70+
self.assertEqual(_git_root(os.path.join(repo, "a", "b")), repo)
71+
self.assertEqual(_git_root(tmp), "")
72+
73+
def test_git_root_accepts_a_git_file_as_in_a_worktree(self):
74+
with tempfile.TemporaryDirectory() as tmp:
75+
repo = os.path.join(tmp, "repo")
76+
os.makedirs(os.path.join(repo, "sub"))
77+
with open(os.path.join(repo, ".git"), "w", encoding="utf-8") as f:
78+
f.write("gitdir: /elsewhere/.git/worktrees/repo\n")
79+
self.assertEqual(_git_root(os.path.join(repo, "sub")), repo)
80+
81+
def test_home_is_refused_even_when_it_is_a_git_root(self):
82+
# HOME being a checkout must not reopen the unbounded match.
83+
with tempfile.TemporaryDirectory() as tmp:
84+
home = os.path.join(tmp, "home")
85+
repo = os.path.join(home, "work", "repo")
86+
os.makedirs(repo)
87+
os.makedirs(os.path.join(home, ".git"))
88+
with mock.patch.dict(os.environ, {"HOME": home}):
89+
self.assertFalse(_ancestor_in_scope(os.path.abspath(home), repo))
90+
91+
92+
if __name__ == "__main__": # pragma: no cover
93+
unittest.main()

0 commit comments

Comments
 (0)