Skip to content

Commit 3a18897

Browse files
authored
fix(embed): use --target sibling binary unconditionally (#2723)
Resolve the #2676-vs-#1240 conflict that broke test-embed-windows on main: scope #2676's 'missing sentence-transformers -> uvx' fallback to the sysconfig-scripts path only. A --target-bundled sibling binary is deliberate and is used unconditionally (preserving #1240). Adds a regression test; fixes a test fixture that conflated the two binary-resolution paths. Greens main.
1 parent b3e32ce commit 3a18897

2 files changed

Lines changed: 51 additions & 5 deletions

File tree

hindsight-embed/hindsight_embed/daemon_embed_manager.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,13 @@ def _find_api_command(self, api_version: str, env: Mapping[str, str] | None = No
262262
package_root = Path(__file__).parent.parent
263263
for bin_dir in ("bin", "Scripts"):
264264
candidate = package_root / bin_dir / binary_name
265-
if candidate.exists() and self._can_use_installed_api_binary(env):
265+
if candidate.exists():
266+
# A hindsight-api binary bundled into a --target layout is a
267+
# deliberate slim-bundle install; use it unconditionally. Falling
268+
# back to uvx here reintroduces issue #1240 (the Windows embed
269+
# smoke test enforces this). The #2676 "missing local ML deps ->
270+
# uvx" fallback intentionally applies only to the sysconfig-scripts
271+
# path above (standard venv installs), not to a --target bundle.
266272
return [str(candidate)]
267273

268274
# Fall back to uvx for the installed version (resolved by the caller).

hindsight-embed/tests/test_embed_manager.py

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -160,14 +160,21 @@ def test_find_api_command_skips_slim_binary_without_local_ml(tmp_path, monkeypat
160160
installed. In that case, use the uvx full-package fallback instead of
161161
starting a daemon that immediately fails during local provider init.
162162
"""
163-
scripts_dir = tmp_path / "bin"
163+
scripts_dir = tmp_path / "venv_bin"
164164
scripts_dir.mkdir()
165165
(scripts_dir / "hindsight-api").touch()
166166

167+
# Realistic venv layout: the module lives under site-packages, so the
168+
# __file__-relative --target path (<site-packages>/bin) is distinct from the
169+
# sysconfig scripts dir and finds no binary. The slim sibling is only in the
170+
# sysconfig scripts dir — exactly where the #2676 uvx fallback applies (a
171+
# --target bundle's sibling would instead be used unconditionally, per #1240).
172+
module_path = tmp_path / "site-packages" / "hindsight_embed" / "daemon_embed_manager.py"
173+
module_path.parent.mkdir(parents=True)
174+
module_path.write_text("")
175+
167176
manager = DaemonEmbedManager()
168-
monkeypatch.setattr(
169-
"hindsight_embed.daemon_embed_manager.__file__", str(tmp_path / "hindsight_embed" / "daemon_embed_manager.py")
170-
)
177+
monkeypatch.setattr("hindsight_embed.daemon_embed_manager.__file__", str(module_path))
171178
monkeypatch.setattr("hindsight_embed.daemon_embed_manager.sysconfig.get_path", lambda key: str(scripts_dir))
172179
monkeypatch.setattr("hindsight_embed.daemon_embed_manager.platform.system", lambda: "Linux")
173180
monkeypatch.setattr("hindsight_embed.daemon_embed_manager.find_spec", lambda name: None)
@@ -226,6 +233,39 @@ def test_find_api_command_target_install_uses_file_relative_fallback(tmp_path, m
226233
assert manager._find_api_command("0.0.0") == [str(sibling_bin)]
227234

228235

236+
def test_find_api_command_target_install_uses_sibling_even_without_local_ml(tmp_path, monkeypatch):
237+
"""A --target-bundled sibling binary must be used even when local ML deps
238+
are missing.
239+
240+
Regression for the #2676 vs #1240 conflict: the "missing sentence-transformers
241+
-> uvx" fallback (#2676) applies only to the sysconfig-scripts path (standard
242+
venv installs). A deliberate --target bundle must still use its sibling binary,
243+
because falling back to uvx on --target installs reintroduces #1240 (enforced
244+
by the Windows embed smoke test).
245+
"""
246+
venv_scripts = tmp_path / "venv_bin"
247+
venv_scripts.mkdir()
248+
249+
target_dir = tmp_path / "target"
250+
pkg_dir = target_dir / "hindsight_embed"
251+
pkg_dir.mkdir(parents=True)
252+
fake_module = pkg_dir / "daemon_embed_manager.py"
253+
fake_module.write_text("")
254+
sibling_bin = target_dir / "bin" / "hindsight-api"
255+
sibling_bin.parent.mkdir()
256+
sibling_bin.touch()
257+
258+
manager = DaemonEmbedManager()
259+
monkeypatch.setattr("hindsight_embed.daemon_embed_manager.__file__", str(fake_module))
260+
monkeypatch.setattr("hindsight_embed.daemon_embed_manager.sysconfig.get_path", lambda key: str(venv_scripts))
261+
monkeypatch.setattr("hindsight_embed.daemon_embed_manager.platform.system", lambda: "Linux")
262+
# Default local providers + no sentence_transformers: the sysconfig path would
263+
# fall back to uvx, but the --target sibling must still win.
264+
monkeypatch.setattr("hindsight_embed.daemon_embed_manager.find_spec", lambda name: None)
265+
266+
assert manager._find_api_command("1.2.3", env={}) == [str(sibling_bin)]
267+
268+
229269
def test_find_api_command_falls_back_to_uvx_when_no_binary(tmp_path, monkeypatch):
230270
"""Without an installed binary or dev checkout, fall back to uvx."""
231271
scripts_dir = tmp_path / "bin"

0 commit comments

Comments
 (0)