PR Review: bench: add BEAM 100K benchmark (end-to-end answer quality)

Executive Summary

Affected Areas: benchmarks/ (new files), .gitignore (cache path)

Business Impact: Adds the first end-to-end answer quality benchmark (vs. retrieval-only). Enables measuring MemPalace RAG quality against BEAM's official rubric judge.

Flow Changes: None — purely additive. No changes to core MemPalace code.

Ratings

PR Health

• Has clear description
• References ticket/issue (if applicable)
• Appropriate size (or justified if large)
• Has relevant tests (if applicable)

High Priority Issues

🔗 #1: Massive code duplication between bench and converter

Location: benchmarks/convert_beam.py:1-129 vs benchmarks/beam_100k_bench.py:63-175 | Confidence: ✅ HIGH

The entire convert_beam.py (129 lines) is a near-verbatim copy of _convert_parquet_to_json() + surrounding helpers from beam_100k_bench.py. The chat-turn parsing, time-anchor cleaning, question extraction, and output schema are duplicated line-for-line. Any future bug fix in parsing logic must be applied in two places.

Fix: Have convert_beam.py import and call the functions from beam_100k_bench.py, or extract the shared conversion logic into a benchmarks/_beam_utils.py module that both files import.

- # convert_beam.py: 100+ lines of duplicated parsing logic
+ from beam_100k_bench import _convert_parquet_to_json, _download_beam_parquet
+
+ def main():
+     input_file = sys.argv[1] if len(sys.argv) > 1 else "data/beam-100k.parquet"
+     output_file = sys.argv[2] if len(sys.argv) > 2 else "data/beam-100k.json"
+     _convert_parquet_to_json(input_file, output_file)

Medium Priority Issues

🐛 #2: Judge score double-quantization may mask LLM output issues

Location: benchmarks/beam_100k_bench.py:358-366 | Confidence: ⚠️ MED

The BEAM judge prompt instructs the LLM to return exactly 1.0, 0.5, or 0.0. But judge_rubric() re-thresholds the parsed score (>=0.75 → 1.0, >=0.25 → 0.5, else 0.0). If the LLM correctly returns 0.5, this survives. But if it returns an unexpected value like 0.7 (partial-ish), the double-quantization silently rounds it to 1.0 — inflating scores. The BEAM paper expects the judge to return only the three canonical values.

  try:
      parsed = json.loads(response)
      score = float(parsed.get("score", 0))
-     if score >= 0.75:
-         return 1.0
-     elif score >= 0.25:
-         return 0.5
-     else:
-         return 0.0
+     # BEAM rubric judge should return exactly 1.0, 0.5, or 0.0
+     if score in (1.0, 0.5, 0.0):
+         return score
+     # Fallback for unexpected values: snap to nearest tier
+     if score >= 0.75:
+         return 1.0
+     elif score >= 0.25:
+         return 0.5
+     return 0.0

🚨 #3: SSL verification globally disabled at import time

Location: benchmarks/beam_100k_bench.py:52 | Confidence: ⚠️ MED

ssl._create_default_https_context = ssl._create_unverified_context disables SSL certificate verification process-wide, not just for the BEAM download. Any subsequent HTTPS call (OpenAI API, Anthropic API) in the same process will skip cert verification. This is a known pattern from convomem_bench.py, but the LLM API calls in this benchmark make the attack surface wider.

Consider scoping the bypass to just the download function:

- ssl._create_default_https_context = ssl._create_unverified_context
  ...
  def _download_beam_parquet(cache_dir):
+     ctx = ssl.create_default_context()
+     ctx.check_hostname = False
+     ctx.verify_mode = ssl.CERT_NONE
      ...
-     urllib.request.urlretrieve(HF_BEAM_URL, parquet_path)
+     with urllib.request.urlopen(HF_BEAM_URL, context=ctx) as resp:
+         with open(parquet_path, "wb") as f:
+             f.write(resp.read())

⚡ #4: Module-level chromadb.EphemeralClient() executes on import

Location: benchmarks/beam_100k_bench.py:249 | Confidence: ⚠️ MED

_bench_client = chromadb.EphemeralClient() runs at import time, creating a ChromaDB client even if you only import a helper function. This matches longmemeval_bench.py's pattern, so it's consistent — but worth noting that it means import beam_100k_bench has side effects (allocates an in-memory DB).

Consider lazy initialization:

- _bench_client = chromadb.EphemeralClient()
+ _bench_client = None
+
+ def _get_client():
+     global _bench_client
+     if _bench_client is None:
+         _bench_client = chromadb.EphemeralClient()
+     return _bench_client

Low Priority Issues

🎨 #5: Hardcoded model version strings will go stale

Location: benchmarks/beam_100k_bench.py:36,211,395 | Confidence: ⚠️ MED

The default model is gpt-5.4-mini (Azure) and claude-haiku-4-5-20251001 (reranker). These will go stale as new model versions are released. Other benchmarks in the repo have the same pattern, but centralizing model defaults (or documenting them in BENCHMARKS.md) would help maintainability.

🎨 #6: No-rubric fallback scoring is coarse

Location: benchmarks/beam_100k_bench.py:447-454 | Confidence: ⚠️ MED

When a question has no rubric items, the code scores pass/fail based on len(answer) > 50. This arbitrary character threshold means a 51-character hallucination scores identically to a perfect answer. Consider at minimum logging a warning that unscored questions were encountered, or using the reference answer for a simple string comparison.

Flow Impact Analysis

No existing code paths are modified. The PR is purely additive:

benchmarks/beam_100k_bench.py (NEW)
  └── uses: chromadb.EphemeralClient (direct, not palace_db.py — consistent with other benchmarks)
  └── uses: mempalace.dialect.Dialect (optional AAAK mode)
  └── uses: openai.AzureOpenAI / openai.OpenAI (synthesis + judging)
  └── uses: Anthropic API via urllib (optional LLM reranking)

benchmarks/convert_beam.py (NEW)
  └── standalone CLI script, no MemPalace imports

.gitignore
  └── adds: benchmarks/.beam_cache/

Created by Octocode MCP https://octocode.ai 🔍🐙

Thanks for the review @bgauryy. Pushed a follow-up commit (8dfae0d) addressing the issues that hold up under inspection. Going through each finding:

Fixed

Code duplication (HIGH). Real issue. Extracted shared parsing logic into benchmarks/_beam_utils.py. convert_beam.py is now a 38-line wrapper that calls convert_parquet_to_json(). beam_100k_bench.py imports ensure_beam_dataset() from the same module. Net 54 lines removed, parsing logic exists in one place. Future bug fixes only need to land in _beam_utils.py.

SSL verification scoped (MED). Real issue. Removed the module-level ssl._create_default_https_context = ssl._create_unverified_context bypass that disabled certificate verification process-wide. SSL is now skipped only inside download_beam_parquet() via a per-request SSLContext, so LLM API calls (OpenAI, Anthropic) keep default certificate verification. This is a tighter scope than the existing convomem_bench.py pattern, intentionally.

No-rubric fallback (LOW). Real issue. The previous fallback scored answers as PASS based on len(answer) > 50, which would have inflated scores on hallucinated 51-character answers if a question shipped without a rubric. Verified 0/400 BEAM questions trigger this branch, but the new behavior prints [SKIP] and excludes the question from totals. Safer if a future dataset variant ever ships unscored questions.

Skipped (with rationale)

Judge double-quantization. This one looks like a bug but isn't. The threshold snapping is intentional and matches the canonical BEAM Rust reference implementation in Karta's beam_100k.rs. LLMs occasionally return non-canonical scores like 0.7 despite the prompt asking for exactly 1.0/0.5/0.0, and snapping to the nearest tier is the standard BEAM normalization. The proposed "fix" produces identical behavior in the well-behaved case and the same fallback in the misbehaved case, so it would just add code without changing scores.

Lazy ChromaDB initialization. Matches the existing pattern in longmemeval_bench.py and convomem_bench.py, both of which create the EphemeralClient at module load. Changing it for one file breaks consistency without solving a real problem (the benchmark scripts are entry points, not libraries that get imported as helpers).

Hardcoded model strings. Already overridable via AZURE_OPENAI_CHAT_MODEL env var, OPENAI_CHAT_MODEL env var, and --llm-model CLI flag. The hardcoded values are documented defaults, not constraints.

The branch is also synced with origin/main (75 commits, no conflicts).

Thanks for the follow-up @rohithzr
I verified each claim against the code on the branch. The three fixes are solid and the commit is clean. Quick note on one point:

ChromaDB lazy init

You mention this matches longmemeval_bench.py and convomem_bench.py, both creating EphemeralClient at module load. That's accurate for longmemeval_bench.py, but convomem_bench.py actually uses chromadb.PersistentClient(path=...) inside a function (~line 181) -- which is the lazy pattern I originally suggested.

Not a blocker -- these scripts are entry points, not importable libraries, so the practical risk is low. Just wanted to correct the record since the consistency argument only holds for one of the two benchmarks cited.

Everything else checks out. The Karta reference impl confirms the judge snapping, SSL scoping is clean, and the _beam_utils.py extraction is well done. LGTM.

You're right and I was wrong. I checked the file: convomem_bench.py has used chromadb.PersistentClient(path=palace_path) inside its per-evaluation function since the original benchmark commit (0f8fa8c). It was never module-level. I pattern-matched from my local experiments instead of checking the actual code, and the consistency claim only ever held for longmemeval_bench.py.

Pushed 475d3fb with the lazy init pattern. _bench_client is now None at module load and gets constructed on first call to _get_bench_client(). Verified import beam_100k_bench has no side effects:

_bench_client at import time: None
_bench_client after _get_bench_client(): Client
cached on second call: True

Thanks for the correction. This is a better pattern, the only reason I argued against it was a wrong factual premise.