Skip to content

Make the baseline embedder auto-path reliable so --require-baseline never hangs#77

Merged
johnjosephhorton merged 1 commit into
mainfrom
baseline-embedder-never-hangs
Jul 10, 2026
Merged

Make the baseline embedder auto-path reliable so --require-baseline never hangs#77
johnjosephhorton merged 1 commit into
mainfrom
baseline-embedder-never-hangs

Conversation

@johnjosephhorton

Copy link
Copy Markdown
Contributor

The bug (from a live research-agent run)

zwill twin-validate --survey w152 --require-baseline stalled for 600s and was abandoned — the agent fell back to --skip-baseline, losing the conditional-baseline comparison entirely (the whole point of the validation).

Root cause: --embedder auto preferred the remote Expected Parrot embeddings endpoint whenever EXPECTED_PARROT_API_KEY was set — which it always is for twin runs — and that endpoint is unavailable, so it hung.

I profiled the baseline at full Pew scale (69 questions, 200 respondents, 8 held-out, 98 covariates, 544k training rows) with a fast embedder: 22s. So the compute was never the problem — the embedding backend selection was.

Fix

  • auto prefers reliable, local backends and never the remote endpoint: direct OPENAI_API_KEY → local sentence-transformers (if installed) → a new built-in lexical embedder that always runs. The remote endpoint is opt-in only via --embedder edsl.
  • New hashing_embedder — bag-of-words feature hashing, zero dependencies, no model download, no network, instant. Weaker than a semantic embedder (it leans on respondent covariates), but the baseline always runs, so --require-baseline can't hang or be silently skipped. Auto emits a stderr warning when it falls back to it.
  • --embedder gains a hashing choice; help text and the agent-workflow guide updated for the new order.

Testing

  • pytest — 261 passed (rewrote the resolver test: forced local/hashing resolve without keys; auto stays local even with an Expected Parrot key set; auto with no keys/no ST returns the lexical embedder that actually embeds, plus a warning — never an error/hang).
  • ruff clean.
  • Live: the hashing embedder produces a real baseline; full-Pew-scale profile is 22s.

Note: the same run also didn't produce a final report (the agent went quiet after fighting the baseline). That looks like agent behavior downstream of this stall rather than a separate CLI error; fixing the stall should remove the thing it got stuck on. Happy to dig further if it recurs.

🤖 Generated with Claude Code

…ever hangs

A research-agent run stalled `twin-validate --require-baseline` for 600s and gave
up (ran `--skip-baseline`), losing the whole point of the validation. Root cause:
`--embedder auto` preferred the REMOTE Expected Parrot embeddings endpoint
whenever EXPECTED_PARROT_API_KEY was set — which it always is for twin runs — and
that endpoint is unavailable/404ing, so it hung. The XGBoost compute itself is
fine (~22s at full Pew scale: 69 questions, 200 respondents, 8 held-out, 544k
training rows).

Fixes:
- auto now prefers reliable, local backends and NEVER the remote endpoint:
  direct OPENAI_API_KEY, else local sentence-transformers if installed, else a
  new zero-dependency built-in lexical (hashing) embedder that always runs. The
  remote Expected Parrot embeddings stay reachable via explicit `--embedder edsl`.
- add hashing_embedder: bag-of-words feature hashing, no model download, no
  network, instant. Weaker than a semantic embedder (leans on covariates) but the
  baseline always runs — so `--require-baseline` can't hang or be skipped. Emits a
  stderr warning when auto falls back to it.
- `--embedder` gains a `hashing` choice; help + guide updated to describe the new
  auto order.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@johnjosephhorton johnjosephhorton merged commit b192396 into main Jul 10, 2026
1 of 3 checks passed
@johnjosephhorton johnjosephhorton deleted the baseline-embedder-never-hangs branch July 10, 2026 11:22
@greptile-apps

greptile-apps Bot commented Jul 10, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes a live hang in --require-baseline validation by removing the remote Expected Parrot embeddings endpoint from the auto selection path and replacing the hard error with a zero-dependency fallback embedder.

  • New hashing_embedder: bag-of-words feature hashing using SHA-1, dim=256, zero dependencies and no network — added to twin_baseline.py and exported via twin_baseline_commands.py.
  • resolve_baseline_embedder rewrite: auto now tries OpenAI key → local sentence-transformers → hashing_embedder (with a stderr warning), skipping the remote EDSL endpoint entirely; --embedder edsl remains the explicit opt-in.
  • CLI and docs updated: hashing added to --embedder choices; help text and agent-workflow guide reflect the new priority order.

Confidence Score: 3/5

Safe to merge for the common case; one concrete crash path in the newly added hashing embedder needs attention before running on FIPS-compliant hosts.

The hashing embedder is the entire reliability guarantee of this PR — it's what prevents --require-baseline from hanging — yet its SHA-1 call will raise ValueError on FIPS-compliant Python builds, crashing the fallback that was supposed to always succeed. The sys reference in twin_baseline_commands.py is only available through an undeclared transitive wildcard import, so any future refactoring of cli.py could turn the warning print into a NameError crash on the same code path.

zwill/twin_baseline.py (the hashing_embedder SHA-1 call) and zwill/twin_baseline_commands.py (the implicit sys dependency).

Important Files Changed

Filename Overview
zwill/twin_baseline.py Adds hashing_embedder — a zero-dependency bag-of-words feature-hash embedder; SHA-1 call without usedforsecurity=False will crash on FIPS-compliant Python builds.
zwill/twin_baseline_commands.py Rewrites resolve_baseline_embedder to drop the remote EDSL endpoint from auto and fall back to hashing_embedder; uses sys.stderr via transitive wildcard import rather than an explicit import sys.
zwill/cli_parser.py Adds hashing and reorders choices for --embedder; updates help text to reflect the new auto priority; no logic issues.
tests/test_twin_baseline.py Rewrites resolver test to cover local/hashing resolve without keys, confirms auto ignores the Expected Parrot key, and verifies the hashing embedder produces real vectors plus a warning.
zwill/guides/agent-workflow.md Documentation update clarifying the new auto backend priority and that the remote endpoint must be opted into explicitly with --embedder edsl.

Comments Outside Diff (1)

  1. zwill/twin_baseline_commands.py, line 1-4 (link)

    P2 sys is not explicitly imported in this file — it is currently only available because from .cli import * happens to pull sys into the namespace (since cli.py has no __all__). If cli.py is ever refactored or its __all__ is added, the fallback warning would crash with NameError: name 'sys' is not defined instead of just skipping the print, silently breaking the auto-fallback path. An explicit import sys at the top of this file removes the dependency on that transitive import.

Reviews (1): Last reviewed commit: "Make the baseline embedder auto-path rel..." | Re-trigger Greptile

Comment thread zwill/twin_baseline.py
for text in texts:
vector = [0.0] * dim
for token in str(text).lower().split():
bucket = int(hashlib.sha1(token.encode()).hexdigest(), 16) % dim

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 SHA-1 without usedforsecurity=False breaks on FIPS systems

hashlib.sha1(token.encode()) raises ValueError on Python builds compiled against OpenSSL in FIPS mode (e.g., RHEL 8+, some AWS/GovCloud images). This directly contradicts the stated invariant — the hashing embedder is supposed to "always run" as the last-resort fallback — but on FIPS-constrained hosts it would crash with a ValueError instead. Adding usedforsecurity=False (Python ≥ 3.9) tells OpenSSL the hash is used for non-cryptographic purposes and is always accepted.

Suggested change
bucket = int(hashlib.sha1(token.encode()).hexdigest(), 16) % dim
bucket = int(hashlib.sha1(token.encode(), usedforsecurity=False).hexdigest(), 16) % dim

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant