fix: exclude binary and large files from LLM analysis - #307
Conversation
Signed-off-by: Makia98 <makia98@foxmail.com>
93b68fe to
41b5110
Compare
rng1995
left a comment
There was a problem hiding this comment.
[Automated SkillSpector Review]
Requesting changes because the filter uses attacker-controlled filename suffixes as a binary predicate and does not implement the PR title's large-file exclusion. A text payload named .png is removed from semantic analysis, while extensionless binaries and arbitrarily large text files are still batched. Please base skipping on inspected content or trusted metadata, define and test the size rule, and preserve scan-completeness accounting for skipped files.
Signed-off-by: Makia98 <makia98@foxmail.com>
|
@rng1995 |
…ut-files Signed-off-by: Makia98 <makia98@foxmail.com> # Conflicts: # src/skillspector/nodes/build_context.py # src/skillspector/nodes/report.py # tests/nodes/test_analysis_completeness.py
|
@rng1995 The conflicts with the latest |
rng1995
left a comment
There was a problem hiding this comment.
[Automated SkillSpector Review]
Re-review: still requesting changes. Content-based media/binary detection, inventory preservation, LLM-only caching, and completeness-ledger events resolve the filename-bypass portion of the prior review. The large-file blocker remains: _read_file_cache() reads the full file and _inspect_bytes() classifies arbitrarily large valid UTF-8 text as included. At this head, a 5,000,000-byte ASCII payload is returned as text / included and is eligible for LLM batching. Please define and enforce the advertised text-size limit before the full read/batch, record the skip in completeness metadata, and add boundary regressions.
| try: | ||
| content = full.read_text(encoding="utf-8", errors="replace") | ||
| file_cache[path] = content | ||
| inspection = _inspect_bytes(full.read_bytes()) |
There was a problem hiding this comment.
Blocking: this still reads the entire file and applies no large-text limit. A 5,000,000-byte ASCII payload is classified as text / included, so the previously requested large-file exclusion remains unimplemented. Please enforce a defined byte limit before the full read/LLM cache insertion, record the exclusion in the inspection ledger, and test just-below/at/above the boundary.
Summary
Skip image, audio, and video files when creating LLM analysis batches.
Changes
LLMAnalyzerBase.get_batches().Verification
1393 passed12 skipped6 xfailed