Directory context artifacts embed compiled bytecode: the binary guard in readArtifactContent() can never fire, and the walk honours no ignore file

[gregoryfoster]

Two defects in readArtifactContent() combine so that a directory context artifact silently embeds compiled bytecode and other build output. Read out of the published package, socraticode@1.12.0, dist/services/context-artifacts.js (lines 125–145).

Both are one-line-ish fixes; I'm happy to send a PR for either or both.

1. The directory walk has a hardcoded ignore list and imports no ignore service

if (stat.isDirectory()) {
    // Find all files in the directory (recursively, skip hidden/dot-files)
    const files = await glob("**/*", {
        cwd: resolved,
        nodir: true,
        dot: false,
        ignore: ["**/node_modules/**", "**/.git/**"],
    });

node_modules and .git, and nothing else. Grepping the module for ignore returns exactly these two occurrences and no .gitignore / .socraticodeignore reference anywhere — so there is no code path by which either ignore file can apply to a context artifact. This is not a load-order or staleness effect; I confirmed it behaviourally first (adding __pycache__/ and *.pyc to .socraticodeignore and re-indexing left the artifact's chunk count identical) and then in the source.

That is defensible as a deliberate choice, but it is not what the README's index-scope framing leads a user to expect, and it is the general case that lets the next consumer embed dist/, .pytest_cache/ or coverage data.

2. The binary guard is already written, and it can never fire

The loop immediately below is what turns (1) into embedded mojibake rather than merely extra text files:

for (const file of files) {
    const filePath = path.join(resolved, file);
    try {
        const content = await fsp.readFile(filePath, "utf-8");
        parts.push(`# ── ${file} ──\n${content}`);
    }
    catch {
        // skip unreadable files (binary, permissions, etc.)
        logger.debug(`Artifact: skipping unreadable file ${file}`);
    }
}

The comment states the intent exactly — skip binary. It never happens, because fsp.readFile(path, "utf-8") does not throw on binary input; it returns a string with U+FFFD replacement characters. Verified against a .pyc fixture on Node:

readFile utf-8 on binary: RETURNED (no throw), len 8, hasFFFD true

So every .pyc takes the success branch and is embedded, mojibake and all. Nothing is unreadable, so nothing is skipped, so nothing is logged — the logger.debug line is dead code.

Measured consequence

A project declaring {"name": "alembic-migrations", "path": "./alembic/versions"} — a natural, documented-shape artifact. Every test run and every alembic invocation drops __pycache__/ into that directory:

• 70 .pyc files embedded, 352K of bytecode against 320K of source
• 32 of the artifact's 86 chunks were compiled bytecode
• a codebase_context_search for "what is the current alembic migration head revision" returned decompiled bytecode as its top hit

	chunks
baseline	86
after adding __pycache__/ + *.pyc to .socraticodeignore, re-indexed	86
after rm -rf alembic/versions/__pycache__, re-indexed	54

Top-hit score for the same query went 0.5417 → 0.6111 once the bytecode was gone.

Why this is worth fixing rather than documenting

It fails silently and upward. Nothing errors, the chunk count rises, and codebase_status reports more artifact content indexed than before. Every available signal says the artifact got healthier while search quality got worse — so there is no observation a user can make that would prompt them to look. The only reason I found it was chasing a bad search result back to its chunk.

Suggested fixes

1. Make the binary guard actually detect binary. This alone fixes the bytecode case and preserves the existing intent. Read as a Buffer and either sniff for a NUL byte in the first few KB, or decode with a fatal decoder:

   const buf = await fsp.readFile(filePath);
   if (buf.includes(0)) { logger.debug(`Artifact: skipping binary file ${file}`); continue; }
   const content = buf.toString("utf-8");

   (new TextDecoder("utf-8", { fatal: true }).decode(buf) inside the existing try would also work and would make the catch live, at the cost of rejecting a few legitimately-latin1 text files.)

2. Wire in the ignore chain the code indexer already uses, so a directory artifact honours the built-in defaults + .gitignore + .socraticodeignore. This is the general fix and the one that stops the next unanticipated build directory.

A cheap partial alternative to (2), if wiring the ignore service into artifacts is unwanted for scope reasons: extend the hardcoded list with the common build-output names (**/__pycache__/**, **/dist/**, **/build/**, **/*.pyc). Less principled, but it covers the case above.

Worth noting for (1) alone: the artifact walk already passes dot: false, so dot-directories like .pytest_cache/ are excluded — __pycache__ is the notable build directory that is not hidden and therefore is walked.

[giancarloerra]

Both confirmed on current main, end to end: real compiled .pyc files embedded into the index as mojibake through the success branch (the six-class decode check found ZERO inputs for which readFile(path, "utf-8") throws: UTF-8, latin1, UTF-16LE with BOM, .pyc, PNG, empty all return strings), ignore-file edits a no-op with an identical content hash, and at your fixture's scale a rebuilt equivalent produced 37.5% bytecode chunks against your 37.2%, with a pure-mojibake chunk ranked second for a natural-language query. Your string-constant observation is the mechanism: marshal data keeps docstrings readable, which is what BM25 matches.

Yes to the PR, thanks, both fixes together. Implementation notes from the verification, several of which change the shape you sketched:

Binary guard: the NUL sniff, not the fatal decoder. Measured difference: a latin1 text file keeps ~89% of its characters searchable under the sniff, while the fatal decoder silently drops the entire file, a 100% loss with nothing logged, which is the exact silent-failure shape this issue exists to remove. The sniff also matches the indexer's own Stage-0 binary guard on extensionless files, so it introduces no loss class the code index does not already have. Read each file once as a Buffer, sniff, toString("utf-8"); log per-file skips with a summary count so the exclusion is observable.

Ignore chain: post-glob filter, rooted at the artifact directory. Two constraints your sketch runs into: glob 11's ignore option does not accept the ignore package (it wants path-scurry objects), and the ignore package throws RangeError on absolute or ../-prefixed paths while artifact directories can be absolute or live under the global fallback outside the project. So build createIgnoreFilter/shouldIgnore over the glob results, mirroring how the indexer consumes the chain, rooted at the resolved artifact directory. One question to settle in review rather than in the diff: whether to root at the project instead when the artifact directory sits inside the project, so project-level ignore files apply too. Note the defaults already contain __pycache__ and *.pyc, so wiring the chain makes your exact case work with no new patterns.

Boundaries to keep:

• The single-file branch stays untouched: a declared single-file artifact pointing at a binary works today and must not gain a silent skip.
• A directory of only binaries keeps the existing loud "no readable files" throw.
• dot: false stays and gets documented; it is why .pytest_cache was already excluded while __pycache__ was walked.
• Implement entirely inside readArtifactContent so the indexed content and the staleness hash cannot diverge. Re-indexing is then automatic on the next content-hash mismatch; the CHANGELOG should say chunk counts drop on the next index touch.

Tests worth having: the ignore-chain exclusion; a top-level NUL binary skipped with no ignore pattern involved (guard live on its own); a latin1 file KEPT (the regression test that fails if a fatal decoder is ever swapped in); UTF-16 with BOM skipped (parity with Stage-0); binaries-only directory still throws; a single-file binary artifact unchanged; staleness hash equal to indexed-content hash on a mixed directory.

README in the same PR: the "read recursively" line gains the exclusion semantics and the dotfile rule; the Ignore Rules section currently implies the three layers cover artifacts, which becomes true with this change.

One correction to the writeup for the PR description: Qdrant fusion scores are rank-derived, so the 0.5417 to 0.6111 movement is not a relevance delta; the defensible statement is that bytecode hits left the top results and source filled them, which your before/after chunk counts already show.

[gregoryfoster]

@giancarloerra, looks like the initial comment on #114 was accidentally pasted here.

[giancarloerra]

@giancarloerra, looks like the initial comment on #114 was accidentally pasted here.

True! Fixed, trying and hoping to get all pending issues and PRs closed soon to make things clearer :-)