filterRelevant: reject Python-style snake_case options

README.md

@@ -144,6 +144,7 @@ const next = await smartapply(diff, files);
 Each decision is `{ path, decision, reason, raw, tokens, promptTokens, completionTokens, cached, skippedReason, error }`.
 
 - `decision` is `"yes"` or `"no"`. After `<think>` / `<thinking>` blocks are stripped, the decision is a leading standalone `yes` or `no` (quotes, markdown bold, backticks, and an `Answer:` or `Decision:` prefix are wrappers, including when a reason follows, as in `**no** unrelated`). Any other wording, including a negated "no" later in the sentence, is malformed: the file is included, `failedOpen` is true, and the judgment is not cached. An empty answer, a non-string message content, and a thrown call do the same. `error` is set when the call throws or the answer is empty. `result.failedOpen` counts those files, and `formatFilterSummary` includes that count.
+- Options are camelCase. Every patchling-py keyword in snake_case (`on_binary`, `max_bytes`, `skip_patterns`, `max_attempts`, `retry_backoff`, `sleep`, ...) throws a `TypeError` before any model call, naming the JS option: `skip_patterns` is `ignore`, `max_attempts: n` is `retries: n - 1`, `retry_backoff: s` (seconds) is `retryBaseMs: s * 1000`, and `sleep` is not supported in JS (pass `retryBaseMs: 0` in tests).
 - `skippedReason` is `"binary"`, `"oversize"`, `"truncated"`, `"ignored"`, or `null`.
 - `tokens`, `promptTokens`, and `completionTokens` are `0` when no model call was made. `reason` is a string. `failedOpen` is true only when the file was included without a definitive yes/no.
 - `result.billedTokens` sums token counts for decisions that were not cache hits. Identical contents at different paths are separate calls.

landing/lib/filter.js

@@ -1063,6 +1063,42 @@ async function cachePut(store, key, record) {
   else await store.set(key, record);
 }
 
+/**
+ * Snake_case option names, mapped to what to pass in JavaScript. Covers
+ * every keyword of patchling-py `filter_relevant(...)` (files, task, model,
+ * concurrency and cache share their names), plus snake_case spellings of
+ * JS-only options. Without this guard they are ignored silently.
+ */
+export const SNAKE_CASE_OPTIONS = Object.freeze({
+  api_key: 'use apiKey',
+  base_url: 'use baseUrl',
+  max_bytes: 'use maxBytes',
+  max_tokens: 'use maxTokens',
+  on_oversize: 'use onOversize',
+  on_binary: 'use onBinary',
+  max_output_tokens: 'use maxOutputTokens',
+  skip_patterns: 'use ignore (an array replaces DEFAULT_IGNORE_PATTERNS; [] or false classifies every path)',
+  max_attempts: 'use retries, which counts extra attempts: retries = max_attempts - 1',
+  retry_backoff: 'use retryBaseMs, in milliseconds: retryBaseMs = retry_backoff * 1000',
+  sleep: 'not supported in JS; backoff uses timers. In tests pass retryBaseMs: 0 to skip the waits',
+  call_llm: 'use callLlm',
+  on_decision: 'use onDecision',
+  reasoning_effort: 'use reasoningEffort',
+  retry_base_ms: 'use retryBaseMs',
+});
+
+/**
+ * Reject snake_case option names (the patchling-py spelling). Without this,
+ * `{ on_binary: 'include' }` is dropped and the default `exclude` applies.
+ * @param {object} opts
+ */
+function rejectSnakeCaseOptions(opts) {
+  const wrong = Object.keys(opts).filter((key) => Object.prototype.hasOwnProperty.call(SNAKE_CASE_OPTIONS, key));
+  if (!wrong.length) return;
+  const fixes = wrong.map((key) => `${key}: ${SNAKE_CASE_OPTIONS[key]}`).join('; ');
+  throw new TypeError(`filterRelevant options are camelCase in JavaScript. ${fixes}`);
+}
+
 /**
  * Ask a cheap model which files are relevant to `task`.
  *
@@ -1107,8 +1143,10 @@ async function cachePut(store, key, record) {
  * @param {string} [opts.baseUrl] `GPTDIFF_LLM_BASE_URL` when omitted
  * @param {Function} [opts.callLlm] injectable client (tests)
  * @returns {Promise<FilterRelevantResult>}
+ * @throws {TypeError} when an option uses the Python snake_case name (`on_binary`, `max_bytes`, ...)
  */
 export async function filterRelevant(files, task, opts = {}) {
+  if (opts != null && typeof opts === 'object') rejectSnakeCaseOptions(opts);
   if (files == null || typeof files !== 'object' || Array.isArray(files)) {
     throw new TypeError('files must be an object of path -> content');
   }

src/filter.js

@@ -1063,6 +1063,42 @@ async function cachePut(store, key, record) {
   else await store.set(key, record);
 }
 
+/**
+ * Snake_case option names, mapped to what to pass in JavaScript. Covers
+ * every keyword of patchling-py `filter_relevant(...)` (files, task, model,
+ * concurrency and cache share their names), plus snake_case spellings of
+ * JS-only options. Without this guard they are ignored silently.
+ */
+export const SNAKE_CASE_OPTIONS = Object.freeze({
+  api_key: 'use apiKey',
+  base_url: 'use baseUrl',
+  max_bytes: 'use maxBytes',
+  max_tokens: 'use maxTokens',
+  on_oversize: 'use onOversize',
+  on_binary: 'use onBinary',
+  max_output_tokens: 'use maxOutputTokens',
+  skip_patterns: 'use ignore (an array replaces DEFAULT_IGNORE_PATTERNS; [] or false classifies every path)',
+  max_attempts: 'use retries, which counts extra attempts: retries = max_attempts - 1',
+  retry_backoff: 'use retryBaseMs, in milliseconds: retryBaseMs = retry_backoff * 1000',
+  sleep: 'not supported in JS; backoff uses timers. In tests pass retryBaseMs: 0 to skip the waits',
+  call_llm: 'use callLlm',
+  on_decision: 'use onDecision',
+  reasoning_effort: 'use reasoningEffort',
+  retry_base_ms: 'use retryBaseMs',
+});
+
+/**
+ * Reject snake_case option names (the patchling-py spelling). Without this,
+ * `{ on_binary: 'include' }` is dropped and the default `exclude` applies.
+ * @param {object} opts
+ */
+function rejectSnakeCaseOptions(opts) {
+  const wrong = Object.keys(opts).filter((key) => Object.prototype.hasOwnProperty.call(SNAKE_CASE_OPTIONS, key));
+  if (!wrong.length) return;
+  const fixes = wrong.map((key) => `${key}: ${SNAKE_CASE_OPTIONS[key]}`).join('; ');
+  throw new TypeError(`filterRelevant options are camelCase in JavaScript. ${fixes}`);
+}
+
 /**
  * Ask a cheap model which files are relevant to `task`.
  *
@@ -1107,8 +1143,10 @@ async function cachePut(store, key, record) {
  * @param {string} [opts.baseUrl] `GPTDIFF_LLM_BASE_URL` when omitted
  * @param {Function} [opts.callLlm] injectable client (tests)
  * @returns {Promise<FilterRelevantResult>}
+ * @throws {TypeError} when an option uses the Python snake_case name (`on_binary`, `max_bytes`, ...)
  */
 export async function filterRelevant(files, task, opts = {}) {
+  if (opts != null && typeof opts === 'object') rejectSnakeCaseOptions(opts);
   if (files == null || typeof files !== 'object' || Array.isArray(files)) {
     throw new TypeError('files must be an object of path -> content');
   }

test/filter_options.test.js

@@ -0,0 +1,85 @@
+import { test } from 'node:test';
+import assert from 'node:assert/strict';
+import * as src from '../src/filter.js';
+import * as landing from '../landing/lib/filter.js';
+
+const yes = async () => ({ choices: [{ message: { content: 'yes' } }], usage: {} });
+
+/** Every keyword of patchling-py filter_relevant() (patchling/relevance.py @ bf9c8f0). */
+const PY_KEYWORDS = [
+  'files', 'task', 'model', 'concurrency', 'cache',
+  'api_key', 'base_url', 'max_bytes', 'max_tokens', 'on_oversize', 'on_binary',
+  'max_output_tokens', 'skip_patterns', 'max_attempts', 'retry_backoff', 'sleep', 'call_llm',
+];
+const SHARED_NAMES = new Set(['files', 'task', 'model', 'concurrency', 'cache']);
+
+function counting() {
+  const state = { calls: 0 };
+  state.callLlm = async () => { state.calls += 1; throw Object.assign(new Error('HTTP 503'), { status: 503 }); };
+  return state;
+}
+
+for (const [label, mod] of [['src', src], ['landing', landing]]) {
+  test(`${label}: skip_patterns: [] throws instead of still excluding a generated file`, async () => {
+    const llm = counting();
+    await assert.rejects(
+      mod.filterRelevant({ 'dist/app.js': 'x' }, 'task', { callLlm: llm.callLlm, apiKey: 'k', cache: false, skip_patterns: [] }),
+      (err) => err instanceof TypeError && /skip_patterns: use ignore/.test(err.message),
+    );
+    assert.equal(llm.calls, 0);
+  });
+
+  test(`${label}: max_attempts: 1 throws before any attempt`, async () => {
+    const llm = counting();
+    await assert.rejects(
+      mod.filterRelevant({ 'a.js': 'x' }, 'task', { callLlm: llm.callLlm, apiKey: 'k', cache: false, retryBaseMs: 0, max_attempts: 1 }),
+      /max_attempts: use retries, which counts extra attempts: retries = max_attempts - 1/,
+    );
+    assert.equal(llm.calls, 0, 'no mocked attempt is made');
+  });
+
+  test(`${label}: retry_backoff: 0 throws with the retryBaseMs conversion`, async () => {
+    const llm = counting();
+    await assert.rejects(
+      mod.filterRelevant({ 'a.js': 'x' }, 'task', { callLlm: llm.callLlm, apiKey: 'k', retry_backoff: 0 }),
+      /retry_backoff: use retryBaseMs, in milliseconds: retryBaseMs = retry_backoff \* 1000/,
+    );
+    assert.equal(llm.calls, 0);
+  });
+
+  test(`${label}: every snake_case patchling-py keyword is rejected`, async () => {
+    for (const key of PY_KEYWORDS.filter((k) => !SHARED_NAMES.has(k))) {
+      assert.ok(Object.hasOwn(mod.SNAKE_CASE_OPTIONS, key), `${key} is mapped`);
+      const llm = counting();
+      await assert.rejects(
+        mod.filterRelevant({ 'a.js': 'x' }, 'task', { callLlm: llm.callLlm, apiKey: 'k', cache: false, [key]: undefined }),
+        (err) => err instanceof TypeError && err.message.includes(`${key}: `),
+        key,
+      );
+      assert.equal(llm.calls, 0, key);
+    }
+  });
+}
+
+test('sleep is reported as not supported in JS', async () => {
+  await assert.rejects(
+    src.filterRelevant({ 'a.js': 'x' }, 'task', { callLlm: yes, apiKey: 'k', sleep: () => {} }),
+    /sleep: not supported in JS/,
+  );
+});
+
+test('several snake_case options are all named in one error', async () => {
+  await assert.rejects(
+    src.filterRelevant({ 'a.js': 'x' }, 'task', { callLlm: yes, apiKey: 'k', on_binary: 'include', max_bytes: 10 }),
+    /on_binary: use onBinary; max_bytes: use maxBytes/,
+  );
+});
+
+test('camelCase options and unrelated extra keys still work', async () => {
+  const result = await src.filterRelevant(
+    { 'a.bin': new Uint8Array([0, 1]), 'a.js': 'x' },
+    'task',
+    { callLlm: yes, apiKey: 'k', onBinary: 'include', cache: false, label: 'mine' },
+  );
+  assert.deepEqual(result.decisions.map((d) => d.decision).sort(), ['yes', 'yes']);
+});