Skip to content

Retire Node LLM backend; add MolBench-powered reference backend (Node 22)#4

Merged
dbolser merged 3 commits into
mainfrom
feat/molbench-backend
Jun 29, 2026
Merged

Retire Node LLM backend; add MolBench-powered reference backend (Node 22)#4
dbolser merged 3 commits into
mainfrom
feat/molbench-backend

Conversation

@dbolser

@dbolser dbolser commented Jun 29, 2026

Copy link
Copy Markdown
Owner

Follow-up to the MIT relicense (already on main). Makes the demo's Node server a
keyword-only dev stub and introduces a real, model-backed backend powered by MolBench —
so the plugin's intelligence lives in MolBench, behind the stable MVSJ contract.

What changed

  • demo/server.mjs → keyword-only dev stub. Dropped provider routing, LLM calls,
    key-based /models, and .env parsing (and with it the parseEnv/Node-20.12 coupling —
    it now imports on any Node). It's purely the offline UI/UX tool now.
  • examples/molbench_backend.py — the real backend = MolBench as a service. stdlib HTTP
    that calls build_system_prompts()['mvs'] (MolBench's real ~4.2k-char schema-primed prompt)
    • build_model(spec) + extract_json_object, then wraps the model's bare {root} tree in
      a proper MVS envelope
      (metadata.version + timestamp) so it renders. Default model
      anthropic:claude-haiku-4-5.
  • npm run build:demo bundles the demo page without starting the keyword stub, so it can
    be served statically against a real backend (no port clash).
  • package.json: engines.node>=22; dropped the @anthropic-ai/sdk dev-dep (−7 pkgs).
  • Removed .env.example (keys live with the backend, e.g. MolBench's own .env).
  • Tests: rewrote test/server.test.ts for the stub + an HTTP round-trip.
  • README: section 2 reframed (keyword demo vs model-backed backend), new
    “Run a local production setup”, and an RCSB-style integration guide.

Why

The plugin is thin transport; the brains (prompt + grounding + future optimization) belong in
MolBench, open, behind {prompt,model}→{mvsj,text?,error?}. A production backend that wraps
MolBench gets every improvement live, with zero plugin change.

Verification (on Node 22)

  • npm run typecheck clean · 29/29 tests · lib + build:demo build.
  • Real end-to-end with a live Haiku call: started examples/molbench_backend.py, drove the
    built demo page in a headless browser → ✓ Rendered (the real model's MVSJ passes Mol*'s
    sanityChecks). This is what caught the missing-metadata.version bug, now fixed.

Note for reviewers

The reference backend currently lives in this repo under examples/. If it becomes the
production service, its natural long-term home is MolBench itself (molbench/serve.py).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added a keyword-based demo backend and a Python reference backend for running chat requests locally.
    • Added a new build mode for producing the demo bundle without starting a server.
    • Expanded setup guidance for connecting a real model and embedding the chat experience in an existing app.
  • Bug Fixes

    • Updated chat and model endpoints to return clearer, more predictable results.
    • Improved detection of structure-related prompts and fallback guidance when no scene is found.
  • Chores

    • Updated ignored files and Node.js version requirements.

Dan Bolser and others added 2 commits June 29, 2026 00:19
…Node 22

The demo's Node server is now a keyword-only dev stub (offline UI/UX work). All
real natural-language → MVS goes through a model-backed backend that speaks the
contract — and the reference one is powered by MolBench, so its schema-primed
prompt and provider adapters (and future grounding / prompt-optimisation) reach
the chat box with no plugin change.

- demo/server.mjs: drop provider routing, LLM calls, /models-from-keys, .env
  parsing (and the parseEnv/Node-20.12 coupling). Keep keyword MVS + the
  contract + a keyword-only /models. Now imports on any Node.
- examples/molbench_backend.py: reference backend = MolBench as a service
  (build_system_prompts['mvs'] + build_model + extract_json_object). stdlib HTTP.
- esbuild.mjs + package.json: `build:demo` bundles the demo page without starting
  the keyword stub, so it can be served statically against a real backend.
- package.json: engines node >=22 (Node 20 LTS is now EOL); drop the
  @anthropic-ai/sdk devDep (-7 packages) — the Node server no longer calls it.
- Remove .env.example (keys live with the model-backed backend, e.g. MolBench's).
- test/server.test.ts: rewrite for the keyword stub + an HTTP round-trip.
- README: section 2 reframed (keyword demo vs model-backed backend), new
  "Run a local production setup", and an RCSB-style integration guide.

Verified: typecheck, 29/29 tests, lib + demo builds; the MolBench backend's
imports + prompt assembly + JSON extraction (no paid model call).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A real-model run surfaced this: MolBench's prompt emits a bare {root} tree (its
grader needs no envelope), but Mol*'s MVSData.fromMVSJ/sanityChecks require a
full MVS state with metadata.version. Wrap the extracted root in
{metadata:{version,timestamp}, root} so the scene actually loads. Verified via a
real Haiku call rendering end-to-end in a browser (Playwright).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request simplifies the bundled demo backend (demo/server.mjs) to a keyword-only development stub, removing direct LLM provider integrations and the @anthropic-ai/sdk dependency. To support real natural-language-to-MVS capabilities, a new Python reference backend powered by MolBench has been introduced in examples/molbench_backend.py. Additionally, the project license has been updated to MIT, the Node engine requirement bumped to >=22, and the documentation updated to explain local production setups and integration with existing Mol* applications. The review comments identify three medium-severity issues in the new Python backend: potential unhandled exceptions during LLM response parsing, a potential AttributeError crash if the POST request body is not a dictionary, and improper socket cleanup on server shutdown.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread examples/molbench_backend.py Outdated
Comment on lines +72 to +83
try:
raw = build_model(spec).generate(SYSTEM, prompt)
except Exception as e: # noqa: BLE001 - surface any provider/setup error to the UI
return {"mvsj": None, "error": f"{type(e).__name__}: {e}"}
tree, err = extract_json_object(raw) # strips ``` fences / prose, parses JSON
root = None if (err or tree is None) else extract_root(tree)
if root is None:
return {"mvsj": None, "text": raw} # no scene — show the raw reply so failures are visible
# MolBench's prompt yields a bare {root} tree (its grader needs no envelope), but Mol*
# requires a full MVS state with metadata.version — wrap it so the scene actually renders.
mvsj = {"metadata": {"version": "1", "timestamp": datetime.now(timezone.utc).isoformat()}, "root": root}
return {"mvsj": json.dumps(mvsj)}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The parsing and wrapping of the LLM response are currently performed outside of the try-except block. If extract_json_object or extract_root raises an exception (for example, if the model returns a valid JSON that does not match the expected MVS schema), the exception will propagate and crash the request handler.

Wrapping the entire processing logic inside the try block ensures that any parsing or extraction errors are gracefully caught and returned as an error message to the UI.

Suggested change
try:
raw = build_model(spec).generate(SYSTEM, prompt)
except Exception as e: # noqa: BLE001 - surface any provider/setup error to the UI
return {"mvsj": None, "error": f"{type(e).__name__}: {e}"}
tree, err = extract_json_object(raw) # strips ``` fences / prose, parses JSON
root = None if (err or tree is None) else extract_root(tree)
if root is None:
return {"mvsj": None, "text": raw} # no scene — show the raw reply so failures are visible
# MolBench's prompt yields a bare {root} tree (its grader needs no envelope), but Mol*
# requires a full MVS state with metadata.version — wrap it so the scene actually renders.
mvsj = {"metadata": {"version": "1", "timestamp": datetime.now(timezone.utc).isoformat()}, "root": root}
return {"mvsj": json.dumps(mvsj)}
try:
raw = build_model(spec).generate(SYSTEM, prompt)
tree, err = extract_json_object(raw) # strips ``` fences / prose, parses JSON
root = None if (err or tree is None) else extract_root(tree)
if root is None:
return {"mvsj": None, "text": raw} # no scene — show the raw reply so failures are visible
# MolBench's prompt yields a bare {root} tree (its grader needs no envelope), but Mol*
# requires a full MVS state with metadata.version — wrap it so the scene actually renders.
mvsj = {"metadata": {"version": "1", "timestamp": datetime.now(timezone.utc).isoformat()}, "root": root}
return {"mvsj": json.dumps(mvsj)}
except Exception as e: # noqa: BLE001 - surface any provider/setup error to the UI
return {"mvsj": None, "error": f"{type(e).__name__}: {e}"}

Comment on lines +113 to +117
try:
data = json.loads(raw or "{}")
except json.JSONDecodeError:
data = {}
self._send(chat(data.get("prompt", ""), data.get("model")))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

If the incoming POST request contains a JSON body that is not a dictionary (such as a JSON array [] or a primitive value like "string"), json.loads will succeed, but calling data.get() will raise an AttributeError and crash the request handler.

Adding a type check to ensure data is a dictionary before calling .get() prevents this crash.

Suggested change
try:
data = json.loads(raw or "{}")
except json.JSONDecodeError:
data = {}
self._send(chat(data.get("prompt", ""), data.get("model")))
try:
data = json.loads(raw or "{}")
if not isinstance(data, dict):
data = {}
except json.JSONDecodeError:
data = {}
self._send(chat(data.get("prompt", ""), data.get("model")))

Comment thread examples/molbench_backend.py Outdated
Comment on lines +128 to +131
try:
server.serve_forever()
except KeyboardInterrupt:
server.shutdown()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

When the server is stopped via KeyboardInterrupt, server.shutdown() is called. However, shutdown() is designed to be called when serve_forever() is running in a separate thread. More importantly, the socket is never closed, which can leave the port bound in the TIME_WAIT state and cause "Address already in use" errors upon immediate restart.

Using a finally block to call server.server_close() ensures that the socket is always properly released.

Suggested change
try:
server.serve_forever()
except KeyboardInterrupt:
server.shutdown()
try:
server.serve_forever()
except KeyboardInterrupt:
pass
finally:
server.server_close()

Apply Gemini Code Assist suggestions on examples/molbench_backend.py:
- chat(): move parse/wrap inside the try/except so extraction errors are
  returned as { error } rather than crashing the handler.
- do_POST(): guard against a non-dict JSON body (e.g. [] or a primitive)
  before calling .get().
- main(): release the socket via finally: server.server_close() so the port
  isn't left in TIME_WAIT on restart.

Verified: real Haiku call still renders (wrapped MVSJ, metadata.version=1);
missing-key path now returns a clean error.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The JS demo server is stripped of all multi-provider LLM logic and reduced to a keyword-only stub exposing handleChat. A new Python reference backend (examples/molbench_backend.py) implements the same /chat and /models HTTP contract using MolBench. Build tooling gains a build-demo mode, Node engine requirement is bumped to >=22, @anthropic-ai/sdk is removed, .env.example is deleted, tests are updated, and the README is rewritten to reflect the new setup.

Changes

Backend replacement, tooling, and docs

Layer / File(s) Summary
JS demo server simplified to keyword-only stub
demo/server.mjs
Removed .env/provider/LLM machinery; introduced KEYWORD_MODEL, handleChat(prompt), and simplified /models + /chat routing to keyword-only responses.
Python MolBench reference backend
examples/molbench_backend.py
New standalone Python HTTP server implementing /chat (MolBench completion → MVSJ) and /models (provider-filtered list), with dotenv loading, CORS, and threaded server.
Build tooling and package updates
esbuild.mjs, package.json, .gitignore
build-demo esbuild mode added; build:demo npm script added; Node engine bumped to >=22; @anthropic-ai/sdk removed from devDependencies; Python cache patterns added to .gitignore.
Tests updated for keyword/handleChat contract
test/server.test.ts
Replaced toResult/availableModels tests with buildKeywordMvs, pickEntry, handleChat unit tests and an HTTP integration test covering /models and /chat hit/miss.
README updated
README.md
"Connect a real model" section replaces old provider setup; new "Add it to an existing Mol* app" section with MvsRenderer/createHttpBackend snippet added.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐇 The provider keys are gone, hooray!
A Python backend joins the fray,
MolBench hums, the keyword stays,
Build-demo runs in simpler ways.
No .env.example to forget —
The rabbit tidied up the set! 🌿

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: retiring the Node LLM backend and adding a MolBench reference backend on Node 22.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/molbench-backend

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@demo/server.mjs`:
- Around line 136-142: The error fallback in the response handling around
handleChat(prompt) is broken because res.writeHead is executed in both the try
and catch paths, which can trigger ERR_HTTP_HEADERS_SENT after a throw. Refactor
the request handler in demo/server.mjs so the payload is computed first
(including JSON.stringify(handleChat(prompt))) and headers are written exactly
once in the success path or once in the catch path; use the existing handleChat
and res.writeHead/res.end flow to locate the fix.

In `@examples/molbench_backend.py`:
- Around line 43-48: The `/models` provider list is out of sync with the
documented keys because `_PROVIDER_DEFAULT` in `examples/molbench_backend.py`
omits `OPENROUTER_API_KEY`. Update `_PROVIDER_DEFAULT` to include an OpenRouter
representative model, or remove OpenRouter from the docs if it should not be
supported, so the behavior in `models()` and the documented `/models` contract
match.
- Around line 97-101: The preflight handler in do_OPTIONS is missing
Access-Control-Allow-Methods, so browser CORS checks never authorize the POST
used by /chat. Update the OPTIONS response to include
Access-Control-Allow-Methods with POST (and any other supported methods)
alongside the existing CORS headers, keeping the change within do_OPTIONS so the
host-page to backend flow can proceed past preflight.
- Around line 127-128: The backend server is currently bound to every network
interface by default, which should be changed to a loopback-only default. Update
the server startup in the code that creates ThreadingHTTPServer so it uses
127.0.0.1 unless an explicit opt-in env var is set for remote access, and keep
the existing logging/output in the MolBench chat backend startup path consistent
with that host choice.

In `@test/server.test.ts`:
- Around line 69-71: The test cleanup in the server shutdown path can hang
because fetch keeps HTTP keep-alive sockets open in Undici’s pool. Update the
finally block in server.test.ts to close idle connections on the server object
before awaiting the existing server.close() promise, using the
server.closeAllConnections() call alongside the current shutdown logic.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f25c90cc-8f65-4f5a-8e3b-ecc120eedffa

📥 Commits

Reviewing files that changed from the base of the PR and between 54d4601 and 19542d6.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (8)
  • .env.example
  • .gitignore
  • README.md
  • demo/server.mjs
  • esbuild.mjs
  • examples/molbench_backend.py
  • package.json
  • test/server.test.ts
💤 Files with no reviewable changes (1)
  • .env.example

Comment thread demo/server.mjs
Comment on lines 136 to 142
try {
const spec = model || env('MODEL') || DEFAULT_MODEL;
const forceKeyword = spec === KEYWORD_MODEL;
const prov = forceKeyword ? null : resolveProvider(spec);
const key = prov && env(prov.keyVar);

let result;
if (forceKeyword || !prov || !key) {
// Keyword mode — chosen explicitly, or no usable key for the active model.
result = { mvsj: buildKeywordMvs(prompt) };
if (!result.mvsj) {
const hint = 'name a structure like “lysozyme”, “hemoglobin”, or a PDB id (e.g. 4ins).';
result.text = forceKeyword
? `Keyword mode: ${hint}`
: `Keyword mode (no ${prov ? prov.keyVar : 'API key'} set): ${hint} ` +
`Add a key to .env to chat with ${spec}.`;
}
} else if (prov.kind === 'anthropic') {
result = toResult(await callAnthropic(key, prov.id, prompt));
} else {
result = toResult(await callOpenAiCompat(prov.baseUrl, key, prov.id, prompt));
}

res.writeHead(200, { 'content-type': 'application/json' });
res.end(JSON.stringify(result));
res.end(JSON.stringify(handleChat(prompt)));
} catch (err) {
res.writeHead(200, { 'content-type': 'application/json' });
res.end(JSON.stringify({ mvsj: null, error: String(err) }));
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Error fallback is broken: res.writeHead is called twice once anything throws.

res.writeHead(200, …) runs in the try before JSON.stringify(handleChat(prompt)). If that work throws, the catch calls res.writeHead again on a response whose headers are already sent, raising ERR_HTTP_HEADERS_SENT inside the async handler instead of returning the intended error JSON.

This is reachable: a non-string prompt (e.g. {"prompt": 123}) flows into pickEntry, which calls prompt.toLowerCase() and throws a TypeError. Compute the payload first, then write headers once.

🔧 Proposed fix
-    try {
-      res.writeHead(200, { 'content-type': 'application/json' });
-      res.end(JSON.stringify(handleChat(prompt)));
-    } catch (err) {
-      res.writeHead(200, { 'content-type': 'application/json' });
-      res.end(JSON.stringify({ mvsj: null, error: String(err) }));
-    }
+    let payload;
+    try {
+      payload = JSON.stringify(handleChat(String(prompt ?? '')));
+    } catch (err) {
+      payload = JSON.stringify({ mvsj: null, error: String(err) });
+    }
+    res.writeHead(200, { 'content-type': 'application/json' });
+    res.end(payload);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
try {
const spec = model || env('MODEL') || DEFAULT_MODEL;
const forceKeyword = spec === KEYWORD_MODEL;
const prov = forceKeyword ? null : resolveProvider(spec);
const key = prov && env(prov.keyVar);
let result;
if (forceKeyword || !prov || !key) {
// Keyword mode — chosen explicitly, or no usable key for the active model.
result = { mvsj: buildKeywordMvs(prompt) };
if (!result.mvsj) {
const hint = 'name a structure like “lysozyme”, “hemoglobin”, or a PDB id (e.g. 4ins).';
result.text = forceKeyword
? `Keyword mode: ${hint}`
: `Keyword mode (no ${prov ? prov.keyVar : 'API key'} set): ${hint} ` +
`Add a key to .env to chat with ${spec}.`;
}
} else if (prov.kind === 'anthropic') {
result = toResult(await callAnthropic(key, prov.id, prompt));
} else {
result = toResult(await callOpenAiCompat(prov.baseUrl, key, prov.id, prompt));
}
res.writeHead(200, { 'content-type': 'application/json' });
res.end(JSON.stringify(result));
res.end(JSON.stringify(handleChat(prompt)));
} catch (err) {
res.writeHead(200, { 'content-type': 'application/json' });
res.end(JSON.stringify({ mvsj: null, error: String(err) }));
}
let payload;
try {
payload = JSON.stringify(handleChat(String(prompt ?? '')));
} catch (err) {
payload = JSON.stringify({ mvsj: null, error: String(err) });
}
res.writeHead(200, { 'content-type': 'application/json' });
res.end(payload);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@demo/server.mjs` around lines 136 - 142, The error fallback in the response
handling around handleChat(prompt) is broken because res.writeHead is executed
in both the try and catch paths, which can trigger ERR_HTTP_HEADERS_SENT after a
throw. Refactor the request handler in demo/server.mjs so the payload is
computed first (including JSON.stringify(handleChat(prompt))) and headers are
written exactly once in the success path or once in the catch path; use the
existing handleChat and res.writeHead/res.end flow to locate the fix.

Comment on lines +43 to +48
# One representative model per provider, offered in the picker when that key is set.
_PROVIDER_DEFAULT = {
"ANTHROPIC_API_KEY": "anthropic:claude-haiku-4-5",
"OPENAI_API_KEY": "openai:gpt-4o-mini",
"GEMINI_API_KEY": "gemini:gemini-2.5-flash",
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Keep /models aligned with the documented providers.

OPENROUTER_API_KEY is documented here and in README.md, but _PROVIDER_DEFAULT omits it, so a backend configured only for OpenRouter never advertises an OpenRouter model to the picker. Either add it here or trim the docs so the /models contract stays truthful.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@examples/molbench_backend.py` around lines 43 - 48, The `/models` provider
list is out of sync with the documented keys because `_PROVIDER_DEFAULT` in
`examples/molbench_backend.py` omits `OPENROUTER_API_KEY`. Update
`_PROVIDER_DEFAULT` to include an OpenRouter representative model, or remove
OpenRouter from the docs if it should not be supported, so the behavior in
`models()` and the documented `/models` contract match.

Comment on lines +97 to +101
def do_OPTIONS(self) -> None: # noqa: N802 - http.server naming
self.send_response(204)
self.send_header("access-control-allow-origin", "*")
self.send_header("access-control-allow-headers", "content-type")
self.end_headers()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Add Access-Control-Allow-Methods to the preflight response.

Cross-origin /chat requests are sent as JSON, so browsers preflight them. This OPTIONS response never authorizes POST, which can make the documented host-page → backend flow fail before the real request is sent.

Suggested fix
     def do_OPTIONS(self) -> None:  # noqa: N802 - http.server naming
         self.send_response(204)
         self.send_header("access-control-allow-origin", "*")
+        self.send_header("access-control-allow-methods", "GET, POST, OPTIONS")
         self.send_header("access-control-allow-headers", "content-type")
         self.end_headers()
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def do_OPTIONS(self) -> None: # noqa: N802 - http.server naming
self.send_response(204)
self.send_header("access-control-allow-origin", "*")
self.send_header("access-control-allow-headers", "content-type")
self.end_headers()
def do_OPTIONS(self) -> None: # noqa: N802 - http.server naming
self.send_response(204)
self.send_header("access-control-allow-origin", "*")
self.send_header("access-control-allow-methods", "GET, POST, OPTIONS")
self.send_header("access-control-allow-headers", "content-type")
self.end_headers()
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@examples/molbench_backend.py` around lines 97 - 101, The preflight handler in
do_OPTIONS is missing Access-Control-Allow-Methods, so browser CORS checks never
authorize the POST used by /chat. Update the OPTIONS response to include
Access-Control-Allow-Methods with POST (and any other supported methods)
alongside the existing CORS headers, keeping the change within do_OPTIONS so the
host-page to backend flow can proceed past preflight.

Comment on lines +127 to +128
server = ThreadingHTTPServer(("0.0.0.0", PORT), Handler)
print(f" MolBench chat backend → http://localhost:{PORT}/chat [default: {DEFAULT_MODEL}]")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Default the reference backend to loopback, not all interfaces.

This starts an unauthenticated, key-backed service on every interface. For the documented local setup, 127.0.0.1 is the safer default; make 0.0.0.0 opt-in via an env var if you still want remote access.

Suggested fix
+HOST = os.environ.get("HOST", "127.0.0.1")
+
 def main() -> None:
     _load_dotenv()
-    server = ThreadingHTTPServer(("0.0.0.0", PORT), Handler)
-    print(f"  MolBench chat backend → http://localhost:{PORT}/chat  [default: {DEFAULT_MODEL}]")
+    server = ThreadingHTTPServer((HOST, PORT), Handler)
+    print(f"  MolBench chat backend → http://{HOST}:{PORT}/chat  [default: {DEFAULT_MODEL}]")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
server = ThreadingHTTPServer(("0.0.0.0", PORT), Handler)
print(f" MolBench chat backend → http://localhost:{PORT}/chat [default: {DEFAULT_MODEL}]")
HOST = os.environ.get("HOST", "127.0.0.1")
def main() -> None:
_load_dotenv()
server = ThreadingHTTPServer((HOST, PORT), Handler)
print(f" MolBench chat backend → http://{HOST}:{PORT}/chat [default: {DEFAULT_MODEL}]")
🧰 Tools
🪛 Ruff (0.15.20)

[error] 127-127: Possible binding to all interfaces

(S104)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@examples/molbench_backend.py` around lines 127 - 128, The backend server is
currently bound to every network interface by default, which should be changed
to a loopback-only default. Update the server startup in the code that creates
ThreadingHTTPServer so it uses 127.0.0.1 unless an explicit opt-in env var is
set for remote access, and keep the existing logging/output in the MolBench chat
backend startup path consistent with that host choice.

Source: Linters/SAST tools

Comment thread test/server.test.ts
Comment on lines +69 to +71
} finally {
await new Promise<void>((resolve) => server.close(() => resolve()));
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

Does Node.js fetch (undici) keep-alive prevent http.Server close callback from firing, and does closeAllConnections fix it?

💡 Result:

Yes, Node.js http.Server keep-alive connections can delay or prevent the server 'close' callback from firing until those connections either time out or are otherwise terminated [1][2][3]. When a client (such as Undici, which powers Node.js 'fetch') maintains an idle keep-alive connection, the server keeps that socket open, effectively preventing the server from closing and triggering the 'close' callback until all connections are reaped [4][2][3]. 'server.closeAllConnections' is an effective mechanism to resolve this [5][6][7]. It forcefully destroys all open connections connected to the server, including active ones and idle keep-alive connections, thereby allowing the 'close' callback to proceed immediately [5][8][6]. Key points regarding this behavior: 1. Server 'close' behavior: When 'server.close' is called, the server stops accepting new connections but will wait for existing active connections to finish their current request/response cycle [4]. Idle keep-alive connections are also monitored, but if they remain open due to client behavior, the server may appear to hang until those sockets are closed [2][3]. 2. Role of closeAllConnections: This method was specifically added (in Node.js v18.2.0) to address situations where a server must shut down immediately, overriding the graceful wait for persistent connections [5][6]. It is recommended to call 'server.closeAllConnections' after 'server.close' to avoid potential race conditions where new connections might be accepted between the two calls [6]. 3. Upgraded connections: Note that 'server.closeAllConnections' may not destroy upgraded connections (like WebSockets) [8][7], as these are handed off from the HTTP parser to the raw socket, meaning they are outside the scope of standard HTTP cleanup APIs [7]. For a graceful shutdown that avoids forceful termination while still ensuring the server closes, ensure that the 'keepAliveTimeout' on the server is configured appropriately, though 'server.closeAllConnections' remains the standard solution for forcing the termination of lingering keep-alive sockets [6][7].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '--- test/server.test.ts ---\n'
nl -ba test/server.test.ts | sed -n '1,140p'

printf '\n--- package / node version hints ---\n'
if [ -f package.json ]; then
  nl -ba package.json | sed -n '1,220p'
fi

printf '\n--- search for closeAllConnections / server.close usage ---\n'
rg -n "closeAllConnections|server\.close\(" -S .

Repository: dbolser/molstar-chat-driver

Length of output: 256


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- test/server.test.ts ---'
nl -ba test/server.test.ts | sed -n '1,140p'

echo
echo '--- package.json ---'
if [ -f package.json ]; then
  nl -ba package.json | sed -n '1,220p'
fi

echo
echo '--- search for closeAllConnections / server.close usage ---'
rg -n "closeAllConnections|server\.close\(" -S .

Repository: dbolser/molstar-chat-driver

Length of output: 233


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- test/server.test.ts ---'
cat -n test/server.test.ts | sed -n '1,160p'

echo
echo '--- package.json ---'
if [ -f package.json ]; then
  cat -n package.json | sed -n '1,240p'
fi

echo
echo '--- search for closeAllConnections / server.close usage ---'
rg -n "closeAllConnections|server\.close\(" -S .

Repository: dbolser/molstar-chat-driver

Length of output: 5374


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- demo/server.mjs outline ---'
ast-grep outline demo/server.mjs --view expanded || true

echo
echo '--- demo/server.mjs (relevant ranges) ---'
cat -n demo/server.mjs | sed -n '1,260p'

Repository: dbolser/molstar-chat-driver

Length of output: 7135


Close idle sockets before waiting on server.close()
fetch() keeps the HTTP connection alive in Undici’s pool, so this test can sit in server.close() until the keep-alive timeout. Since this repo targets Node 22, call server.closeAllConnections() before awaiting close.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/server.test.ts` around lines 69 - 71, The test cleanup in the server
shutdown path can hang because fetch keeps HTTP keep-alive sockets open in
Undici’s pool. Update the finally block in server.test.ts to close idle
connections on the server object before awaiting the existing server.close()
promise, using the server.closeAllConnections() call alongside the current
shutdown logic.

@dbolser dbolser merged commit bf4ab81 into main Jun 29, 2026
2 checks passed
@dbolser dbolser deleted the feat/molbench-backend branch June 29, 2026 19:53
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