Make max file size configurable via SEMBLE_MAX_FILE_BYTES and warn on skipped files - #252
Make max file size configurable via SEMBLE_MAX_FILE_BYTES and warn on skipped files#252TommyC81 wants to merge 1 commit into
Conversation
Confidence Score: 4/5The PR appears safe to merge, with a non-blocking logging edge case when the package logger was preconfigured by an embedding application. The indexing-limit behavior is coherent and tested, but CLI logging can omit its promised stderr handler merely because an unrelated handler already exists on the Files Needing Attention: src/semble/cli.py Reviews (1): Last reviewed commit: "rework: env var only, no CLI flag, per r..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if package_logger.handlers: | ||
| return |
There was a problem hiding this comment.
If an embedding application attaches a non-stderr handler or NullHandler directly to the semble logger before invoking the CLI, this early return prevents installation of the CLI stderr handler, so oversized-file and invalid-limit warnings are redirected or discarded instead of being shown on stderr.
There was a problem hiding this comment.
Addressed in 78d5bb0 — idempotence now keys on a CLI-owned handler type, so a foreign preconfigured handler no longer suppresses the stderr setup.
Greptile P2 on PR MinishLab#252: the any-handler early return meant a foreign handler attached to the semble logger by an embedding application would suppress the CLI stderr handler, redirecting or discarding oversized-file warnings. Idempotence now keys on the CLI-owned handler type (_CliLogHandler), so repeated setup still adds exactly one handler while a preconfigured foreign logger no longer blocks stderr visibility. Lore-id: f488e6e1 Constraint: CLI logging setup must be idempotent across repeated in-process invocations Tested: 328 tests pass; ruff, ruff format, mypy clean Tested: in-process check: with a foreign NullHandler attached, CLI handler installed exactly once across two setup calls Related: c9380176 Confidence: high Scope-risk: narrow Reversibility: clean
Files larger than 1 MB are skipped during indexing without any indication, silently leaving gaps in search results (MinishLab#250). Resolve the limit per call from the SEMBLE_MAX_FILE_BYTES environment variable (following SEMBLE_CACHE_LOCATION / SEMBLE_CLONE_TIMEOUT / SEMBLE_MODEL_NAME), falling back to the unchanged 1 MB default; malformed or nonpositive values warn and fall back instead of crashing indexing. Warn at index time naming files skipped for size, with the CLI surfacing warnings on stderr via an idempotent, CLI-owned handler.
78d5bb0 to
68f3c45
Compare
|
Squashed the branch into a single commit ( |
1 participant
Closes #250
Following the discussion in #250, this makes the 1 MB max file size configurable via a
SEMBLE_MAX_FILE_BYTESenvironment variable and makes skipped files visible — no CLI flag, no default change.Changes
SEMBLE_MAX_FILE_BYTESenv var (src/semble/index/files.py) — newget_max_file_bytes()resolves the limit per call from the environment, falling back to the unchanged 1 MB default. Follows the existingSEMBLE_CACHE_LOCATION/SEMBLE_CLONE_TIMEOUT/SEMBLE_MODEL_NAMEidiom, and works for the MCP server path too, where env vars are the only channel.src/semble/index/create.py) —create_index_from_pathcollectsTOO_LARGEfiles and logs oneWARNINGnaming them (up to 5, then...) with the active limit. The CLI installs an idempotent stderr handler on thesemblelogger so the warning is visible; the root logger is untouched.tests/index/test_index.py: env resolution (default + override), invalid values fall back, oversized file skipped with a warning naming the file, and oversized file indexed once the limit is raised.Checklist
Closes #250)make test— 328 passed (4 new)make lint/make typecheck— ruff check, ruff format, mypy clean