Stop indexing node_modules, and make a large scope a decision

A review on a real repository indexed 113,768 code units and began an embed
that would have run for about eight hours. Two independent causes, both mine:

1. Nothing excluded anything. Slopo does NOT read .gitignore, so a repo with
   node_modules in its ignore file still had all 21,104 files under it parsed -
   13,091 of them source. The generated config carried no source_dir_exclude at
   all, so the tool reviewed the dependencies instead of the code.

2. Nothing bounded the work. Slopo's own >1000 guard is useless here: it
   measures the PRE-EXISTING backlog before indexing, and a fresh database has
   no units at all. The guard that exists to protect an agent's latency is
   silent in the one case that most needs it.

Measured on that repository:

  no excludes                 113,768 units   ~7.9 h
  default excludes              8,607 units   ~36 min
  default + a server-only scope 1,519 units   ~6 min

So: default excludes for vendored and generated directories, ADDED to the
user's own list rather than chosen between (a settings patch replaces a whole
value, so a single sourceDirExclude pattern would otherwise re-enable the 13x
blow-up). And every action now indexes first, checks the backlog against
maxIndexUnits (default 5,000), and refuses with the count, the estimate and the
lever rather than starting the work.

The count comes from slopo's own database via node:sqlite - no new dependency,
and it degrades to the summary line on an older runtime or a locked file. That
matters because `Indexed N` reports only what was NEWLY indexed: on a warm
database it reads zero while the backlog is still full, which is exactly the
state a refusal leaves behind. Caught that hole by running the thing against the
real repository rather than trusting the first version.

Also fixes `status` reporting when the endpoint probe succeeded - the fix from
the previous commit is what made this run reachable at all.
This commit is contained in:
dsh-slopo agent committed 2026-09-22 13:23:49 +01:00
1 parent 46124b11aa
commit 1074b50e51
2 files changed
+247 -14

No files matched your search

+78 -14
View File
@@ -35,6 +35,13 @@ import Schema from '@deepseek-ai/schemastery'
import { missingRequired, normalizeConfig, renderSlopoConfig } from './lib/config.js' import { missingRequired, normalizeConfig, renderSlopoConfig } from './lib/config.js'
import { probeEmbeddingEndpoint } from './lib/probe.js' import { probeEmbeddingEndpoint } from './lib/probe.js'
import { toJsonValue } from './lib/json.js' import { toJsonValue } from './lib/json.js'
import {
budgetRemedy,
checkBudget,
effectiveExcludes,
parseIndexOutput,
readIndexCounts,
} from './lib/scope.js'
import { import {
resolveDshHome, resolveDshHome,
workspaceStateDir, workspaceStateDir,
@@ -76,6 +83,8 @@ const SETTING_KEYS = [
'embeddingParamsJson', 'embeddingParamsJson',
'sourceDir', 'sourceDir',
'sourceDirExclude', 'sourceDirExclude',
'useDefaultExcludes',
'maxIndexUnits',
'ignoreFile', 'ignoreFile',
'stateDir', 'stateDir',
'embeddingRequestDelay', 'embeddingRequestDelay',
@@ -107,8 +116,12 @@ export const Config = Schema.object({
embeddingParamsJson: Schema.string().default('{}'), embeddingParamsJson: Schema.string().default('{}'),
/** Code to index. Empty means the session's working directory. */ /** Code to index. Empty means the session's working directory. */
sourceDir: Schema.string().default(''), sourceDir: Schema.string().default(''),
/** gitignore-style paths to skip. */ /** Extra gitignore-style paths to skip, on top of the defaults. */
sourceDirExclude: Schema.array(String).default([]), sourceDirExclude: Schema.array(String).default([]),
/** Exclude vendored dependencies and build output. Off means slopo sees everything. */
useDefaultExcludes: Schema.boolean().default(true),
/** Refuse to embed more than this many units without being asked. 0 removes the limit. */
maxIndexUnits: Schema.number().default(5000),
/** Ignore file. Empty means inside the per-workspace state directory. */ /** Ignore file. Empty means inside the per-workspace state directory. */
ignoreFile: Schema.string().default(''), ignoreFile: Schema.string().default(''),
/** State directory. Empty means $DSH_HOME/dsh-slopo/workspaces/<workspace>. */ /** State directory. Empty means $DSH_HOME/dsh-slopo/workspaces/<workspace>. */
@@ -187,7 +200,7 @@ function embeddingParams(settings, problems) {
export function toSlopoConfig(settings, paths, problems = []) { export function toSlopoConfig(settings, paths, problems = []) {
const config = { const config = {
source_dir: paths.sourceDir, source_dir: paths.sourceDir,
source_dir_exclude: Array.isArray(settings.sourceDirExclude) ? settings.sourceDirExclude : [], source_dir_exclude: effectiveExcludes(settings),
db_file: paths.dbFile, db_file: paths.dbFile,
report_dir: paths.reportDir, report_dir: paths.reportDir,
ignore_file: paths.ignoreFile, ignore_file: paths.ignoreFile,
@@ -526,6 +539,12 @@ export function apply(ctx, config) {
embeddingDimensions: slopoConfig.embedding_dimensions ?? null, embeddingDimensions: slopoConfig.embedding_dimensions ?? null,
apiKeyConfigured: typeof settings.embeddingApiKey === 'string' && settings.embeddingApiKey !== '', apiKeyConfigured: typeof settings.embeddingApiKey === 'string' && settings.embeddingApiKey !== '',
embeddingEndpoint: endpoint, embeddingEndpoint: endpoint,
// What slopo will actually look at, and the limit on how much of
// it may be embedded without being asked. Both are the levers a
// runaway scope is fixed with, so status has to name them.
sourceDirExclude: effectiveExcludes(settings),
useDefaultExcludes: settings.useDefaultExcludes !== false,
maxIndexUnits: Number(settings.maxIndexUnits),
missingSettings: missing, missingSettings: missing,
problems, problems,
}, },
@@ -678,11 +697,48 @@ export function apply(ctx, config) {
? { SLOPO_EMBEDDING_API_KEY: settings.embeddingApiKey } ? { SLOPO_EMBEDDING_API_KEY: settings.embeddingApiKey }
: undefined : undefined
const steps = // Every action indexes FIRST, so the size gate sees the real scope
// before any embedding is paid for. Slopo's own >1000 guard cannot do
// this: it measures the pre-existing backlog, and a fresh database has
// no units at all — which is exactly when a runaway costs the most.
const excludes = effectiveExcludes(settings)
const indexArgs = slopoArgs(paths.configPath, 'index')
const restArgs =
args.action === 'prepare' args.action === 'prepare'
? [slopoArgs(paths.configPath, 'index'), slopoArgs(paths.configPath, 'embed')] ? [slopoArgs(paths.configPath, 'embed')]
: [slopoArgs(paths.configPath, args.action, { base: args.base, cluster: args.cluster })] : [slopoArgs(paths.configPath, args.action, { base: args.base, cluster: args.cluster })]
const command = steps.map((argv) => commandLine([binary, ...argv])).join(' && ')
/**
* Index, check the budget, then run the action.
* Returns a refusal instead of running when the scope is too large.
*/
const runScoped = async (signal) => {
const indexed = await run(commandLine([binary, ...indexArgs]), { env, signal })
const parsed = parseIndexOutput(indexed.stdout)
// Prefer the database. `Indexed N` counts only what was NEWLY indexed,
// so it reads zero on a warm database whose backlog is still full —
// exactly the state a refused run leaves behind.
const fromDb = readIndexCounts(paths.dbFile)
const counted =
fromDb === undefined
? parsed
: { units: fromDb.unembedded, files: fromDb.files, inScope: fromDb.units, embedded: fromDb.embedded }
if (indexed.exitCode !== 0) return { refused: undefined, counted, outcome: indexed }
const decision = checkBudget(counted, Number(settings.maxIndexUnits), excludes)
if (!decision.allowed) return { refused: decision.reason, counted, outcome: undefined }
const restCommand = restArgs.map((argv) => commandLine([binary, ...argv])).join(' && ')
return { refused: undefined, counted, outcome: await run(restCommand, { env, signal }) }
}
const scopeRefusal = (reason, counted) =>
foreground({
ok: false,
exitCode: null,
errorCode: 'SCOPE_TOO_LARGE',
message: reason,
remedy: budgetRemedy(Number(settings.maxIndexUnits), excludes),
details: { counted, excludes, maxIndexUnits: Number(settings.maxIndexUnits) },
})
const wantsBackground = args.foreground !== true && BACKGROUND_ACTIONS.has(args.action) const wantsBackground = args.foreground !== true && BACKGROUND_ACTIONS.has(args.action)
const jobs = ctx.get('jobs') const jobs = ctx.get('jobs')
@@ -695,12 +751,19 @@ export function apply(ctx, config) {
run: () => { run: () => {
const controller = new AbortController() const controller = new AbortController()
const done = (async () => { const done = (async () => {
const result = await run(command, { env, signal: controller.signal }) const { refused, counted, outcome } = await runScoped(controller.signal)
const logDetail = await readLogTail(paths.logFile) const logDetail = await readLogTail(paths.logFile)
if (result.exitCode === 0) { if (refused !== undefined) {
return { status: 'completed', detail: `exit code: 0`, output: result.stdout.trim() || 'No duplicates found' } return {
status: 'failed',
detail: 'SCOPE_TOO_LARGE',
output: `${refused}\n\n${budgetRemedy(Number(settings.maxIndexUnits), excludes)}`,
} }
const explained = explainFailure({ stderr: result.stderr, stdout: result.stdout, logDetail }) }
if (outcome.exitCode === 0) {
return { status: 'completed', detail: 'exit code: 0', output: outcome.stdout.trim() || 'No duplicates found' }
}
const explained = explainFailure({ stderr: outcome.stderr, stdout: outcome.stdout, logDetail })
return { return {
status: 'failed', status: 'failed',
detail: explained.code, detail: explained.code,
@@ -713,15 +776,16 @@ export function apply(ctx, config) {
return toJsonValue({ kind: 'background', action: args.action, jobId, logPath: paths.logFile, configPath: paths.configPath }) return toJsonValue({ kind: 'background', action: args.action, jobId, logPath: paths.logFile, configPath: paths.configPath })
} }
const result = await run(command, { env, signal: exec.signal }) const { refused, counted, outcome } = await runScoped(exec.signal)
const logDetail = await readLogTail(paths.logFile) const logDetail = await readLogTail(paths.logFile)
if (result.exitCode === 0) { if (refused !== undefined) return scopeRefusal(refused, counted)
return foreground({ ok: true, exitCode: 0, report: result.stdout.trim() || 'No duplicates found' }) if (outcome.exitCode === 0) {
return foreground({ ok: true, exitCode: 0, report: outcome.stdout.trim() || 'No duplicates found' })
} }
const explained = explainFailure({ stderr: result.stderr, stdout: result.stdout, logDetail }) const explained = explainFailure({ stderr: outcome.stderr, stdout: outcome.stdout, logDetail })
return foreground({ return foreground({
ok: false, ok: false,
exitCode: result.exitCode, exitCode: outcome.exitCode,
errorCode: explained.code, errorCode: explained.code,
message: explained.message, message: explained.message,
remedy: explained.remedy, remedy: explained.remedy,
+169
View File
@@ -0,0 +1,169 @@
/**
* What slopo is allowed to look at, and how much of it is too much.
*
* Two separate failures made a review on this machine index 113,768 code units
* and embed for what would have been about eight hours:
*
* 1. Nothing excluded anything. Slopo does NOT read `.gitignore`, so a
* repository with `node_modules` in its ignore file still had all 21,104
* files under `node_modules` parsed — 13,091 of them source files. Vendored
* dependencies are not "your code" by any reading, so excluding them is a
* correctness default rather than a preference.
*
* 2. Nothing bounded the work. Slopo's own >1000 guard cannot help on a first
* run: it measures the *existing* backlog before indexing, and a fresh
* database has no units at all. So the guard that exists precisely to protect
* an agent's latency is silent in the one case that most needs it.
*
* Measured on the repository this was written against:
*
* no excludes 113,768 units ~7.9 h
* default excludes 8,607 units ~36 min
* default + a server-only scope 1,519 units ~6 min
*/
/**
* Directories and files that are never the project's own source.
*
* Globs rather than bare names, because these were verified against slopo 0.8.0
* in this exact setup and a bare name's behaviour is not something to assume.
*/
import { createRequire } from 'node:module'
export const DEFAULT_SOURCE_EXCLUDES = Object.freeze([
'**/node_modules/**',
'**/.git/**',
'**/dist/**',
'**/build/**',
'**/out/**',
'**/coverage/**',
'**/.venv/**',
'**/venv/**',
'**/__pycache__/**',
'**/target/**',
'**/vendor/**',
'**/*.min.js',
])
/**
* Units per second on the reference backend (a CPU Ollama serving a 768-dim
* code model), measured rather than guessed. Used only to turn a unit count
* into an estimate in the refusal message — never as a decision input.
*/
export const REFERENCE_UNITS_PER_SECOND = 4
/**
* The excludes slopo should actually be given.
*
* Defaults and the user's own list are combined rather than chosen between,
* because a settings patch REPLACES a whole config value rather than merging
* into it: if `sourceDirExclude` were the only knob, setting it to `['ui/**']`
* would silently stop excluding `node_modules` and reintroduce the 13x blow-up.
*
* @param {{ sourceDirExclude?: unknown, useDefaultExcludes?: unknown }} settings
* @returns {string[]}
*/
export function effectiveExcludes(settings) {
const defaults = settings.useDefaultExcludes === false ? [] : [...DEFAULT_SOURCE_EXCLUDES]
const mine = Array.isArray(settings.sourceDirExclude) ? settings.sourceDirExclude : []
return [...new Set([...defaults, ...mine.filter((entry) => typeof entry === 'string' && entry !== '')])]
}
/**
* Read slopo's own summary line.
*
* `Indexed 3 code units from 2 files (0 unchanged, 0 removed).` — the count of
* code units slopo considered. On a fresh database that is the whole scope,
* which is exactly the number a budget should be checked against.
*
* @param {string} stdout
* @returns {{ units: number, files: number } | undefined}
*/
export function parseIndexOutput(stdout) {
// Tolerates a thousands separator, which slopo does not currently emit but
// which would silently truncate the count to its first group if it ever did.
const match = /Indexed\s+([\d,]+)\s+code units?\s+from\s+([\d,]+)\s+files?/i.exec(String(stdout))
if (match === null) return undefined
return { units: Number(match[1].replaceAll(',', '')), files: Number(match[2].replaceAll(',', '')) }
}
/** A human-scale estimate, so a refusal says minutes rather than a bare count. */
export function estimateDuration(units, unitsPerSecond = REFERENCE_UNITS_PER_SECOND) {
const seconds = units / unitsPerSecond
if (seconds < 90) return `about ${Math.max(1, Math.round(seconds))} seconds`
const minutes = seconds / 60
if (minutes < 90) return `about ${Math.round(minutes)} minutes`
return `about ${(minutes / 60).toFixed(1)} hours`
}
/**
* The exact backlog, read from slopo's own database.
*
* `Indexed N code units` reports only what was NEWLY indexed, so on a warm
* database it reads zero while the backlog is still full — and that is exactly
* the state a refused run leaves behind, which would let the next attempt slip
* straight through the gate. The database has the real numbers, and Node 22.5+
* reads it with no dependency at all.
*
* Returns undefined on an older runtime, a missing file, or a database another
* process is holding — a locked read must not fail the run, so the caller falls
* back to the summary line, which is accurate on a first index.
*
* @param {string} dbFile
* @returns {{ units: number, embedded: number, unembedded: number, files: number } | undefined}
*/
export function readIndexCounts(dbFile) {
try {
const require = createRequire(import.meta.url)
const { DatabaseSync } = require('node:sqlite')
const db = new DatabaseSync(dbFile, { readOnly: true })
try {
const count = (sql) => Number(db.prepare(sql).get().n)
const units = count('select count(*) as n from code_units')
const embedded = count('select count(*) as n from embeddings')
const files = count('select count(*) as n from files')
return { units, embedded, unembedded: Math.max(0, units - embedded), files }
} finally {
db.close()
}
} catch {
return undefined
}
}
/**
* Decide whether the backlog is small enough to embed without asking.
*
* A refusal is deliberately not a failure: it names the number, the estimate,
* and the one lever that changes it, so a large scope becomes a decision the
* caller makes once rather than an eight-hour surprise.
*
* @param {{ units: number, files?: number } | undefined} indexed units STILL TO EMBED
* @param {number} budget
* @param {string[]} excludes
* @returns {{ allowed: true } | { allowed: false, reason: string }}
*/
export function checkBudget(indexed, budget, excludes) {
if (indexed === undefined) return { allowed: true }
if (!Number.isFinite(budget) || budget <= 0) return { allowed: true }
if (indexed.units <= budget) return { allowed: true }
return {
allowed: false,
reason:
`${indexed.units.toLocaleString('en-US')} code units still need embedding` +
`${indexed.files === undefined ? '' : ` across ${indexed.files.toLocaleString('en-US')} files`}. ` +
`At the reference rate that is ${estimateDuration(indexed.units)}, against a budget of ` +
`${budget.toLocaleString('en-US')}.`,
}
}
/** The remedy text for a refusal, shared so it cannot drift from the cause. */
export function budgetRemedy(budget, excludes) {
const current = excludes.length === 0 ? 'nothing is currently excluded' : `${excludes.length} patterns are excluded`
return (
`Narrow the scope with the \`sourceDirExclude\` setting (${current}), or raise \`maxIndexUnits\` above the ` +
`count if you really mean to embed it — set it to 0 to remove the limit. On a repository with a ` +
'`node_modules` directory, excluding it is usually most of the difference.'
)
}