diff --git a/README.md b/README.md index 9d00ad5..26d17df 100644 --- a/README.md +++ b/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 `` / `` 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. diff --git a/landing/lib/filter.js b/landing/lib/filter.js index c821004..a8f38b0 100644 --- a/landing/lib/filter.js +++ b/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} + * @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'); } diff --git a/src/filter.js b/src/filter.js index c821004..a8f38b0 100644 --- a/src/filter.js +++ b/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} + * @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'); } diff --git a/test/filter_options.test.js b/test/filter_options.test.js new file mode 100644 index 0000000..ebe20ad --- /dev/null +++ b/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']); +});