Robustness and efficiency issues from code audit

[hummat]

Non-security findings from code audit

Verified bugs, packaging issues, and efficiency improvements.

---

1. [BUG] _find_paper_by_source_url crashes if PAPERS_DIR doesn't exist

paperpipe/cli/papers.py:75 — config.PAPERS_DIR.iterdir() is called without an existence check. Raises FileNotFoundError when invoked before the directory has been bootstrapped (e.g., first run, or when called outside the normal CLI entry point).

Fix: Guard with if not config.PAPERS_DIR.exists(): return None before iterating.

---

2. [BUG] Temp file leak on unexpected exceptions in download_source

paperpipe/paper.py:140-201 — tar_path.unlink() at line 201 is outside a finally block. The try/except at line 145 only catches tarfile.ReadError. Any other exception (e.g., MemoryError, OSError from tar.getmembers()) skips cleanup and leaks the temp file.

Fix: Move tar_path.unlink() into a finally block:

try:
    ...
except tarfile.ReadError:
    ...
finally:
    tar_path.unlink(missing_ok=True)

---

3. [PERF] Nested O(refs × members) scan in figure extraction

paperpipe/paper.py:305-318 — For each \includegraphics reference and each candidate extension, the code iterates all tar members (tar.getmembers()). This is O(refs × candidates × members).

Fix: Build a dict of tar members keyed by normalized name once, then do O(1) lookups per candidate:

member_index = {m.name.replace("\\", "/"): m for m in tar.getmembers() if m.isfile()}
# Also index by basename for partial matching

---

4. [MINOR] Duplicate URL check scans all paper directories on every add

paperpipe/cli/papers.py:75-93 — Every add call iterates the full PAPERS_DIR, opening and parsing each meta.json. O(n) per add. Not a bug per se — acceptable for personal paper collections — but worth noting if the DB grows large.

Lower priority. Could be addressed later with a URL index file if needed.

---

5. [BUG] _setup_debug_logging adds duplicate handlers on repeated calls

paperpipe/output.py:15-21 — Every call to _setup_debug_logging() unconditionally appends a new StreamHandler. In long-lived processes (MCP server) or tests, this duplicates log output and adds overhead.

Fix: Guard against re-adding:

def _setup_debug_logging() -> None:
    if _debug_logger.handlers:
        return
    ...

---

6. [BUG] Packaging: build includes wrong paths, ships stale backup file

• pyproject.toml:85-86, 97-98 — Wheel and sdist only-include reference skill (singular) and prompts, but the actual directory is skills (plural) and no prompts directory exists. Skills are missing from built distributions, so papi install skill fails post-install.
• paperpipe/install.py:23 — Runtime installer correctly looks for skills.
• paperpipe/cli.py.backup — Stale backup file is tracked in git and shipped in the wheel.

Fix: Change skill → skills, remove prompts in pyproject.toml build targets. Run git rm paperpipe/cli.py.backup and optionally add *.backup to .gitignore.

[hummat]

6. [CLEANUP] Stale cli.py.backup tracked in git and shipped in wheel

paperpipe/cli.py.backup — A backup file is tracked in git and included in built wheels (since the build target includes all of paperpipe/). This is dead code bloat in distributions.

Fix: git rm paperpipe/cli.py.backup and optionally add *.backup to .gitignore.

[hummat]

Verification & Cross-References

Source-verified all 6 findings against main @ 20b30cf.

Accuracy

#	Finding	Verified?	Notes
1	PAPERS_DIR crash	⚠️ Overstated	ensure_db() in cli/__init__.py:71 creates PAPERS_DIR before any command runs. Only triggers if the directory is deleted at runtime — not on "first run" as claimed. Consider downgrading or noting the mitigation.
2	Temp file leak	✅ Confirmed	tar_path.unlink() at line 201 outside finally. Any exception besides tarfile.ReadError leaks the file.
3	O(n²) figure extraction	✅ Confirmed	Lines 305-318 exact. tar.getmembers() re-scanned per candidate per reference.
4	O(n) URL duplicate check	✅ Confirmed	Acceptable at current scale, worth noting for growth.
5	Duplicate log handlers	✅ Latent	Code is correct but fragile — only called once today (CLI startup). Would surface in MCP server or tests if _setup_debug_logging() were called again.
6	Packaging: skill → skills	✅ Confirmed — critical	pyproject.toml:85,97 reference skill (singular), but filesystem has skills/ (plural). Built wheels miss the skills directory entirely. prompts entry is stale (no such dir); harmless but should be removed.

Cross-references

• Finding 2 (temp file leak in paper.py) → overlaps with Security audit: pickle RCE, path traversal, key leakage, and other findings #55.5 (unbounded decompression in same file); natural to fix together.
• Finding 5 (duplicate handlers) → more relevant given Security audit: pickle RCE, path traversal, key leakage, and other findings #55's MCP server findings; long-lived MCP process would accumulate handlers if debug logging is toggled.
• Finding 1 correction: The claim "first run" is inaccurate — ensure_db() handles bootstrapping. The function _find_paper_by_source_url is only reachable via add, which runs after ensure_db(). Still worth a defensive guard, but not a "crash on first run" bug.

[hummat]

7. [BUG] Figure basename collision silently overwrites + over-reports count

paperpipe/paper.py:330-335 — _extract_figures_from_latex flattens tar member paths to basename before writing:

dest_name = Path(member_name).name  # line 330
dest_path = figures_dir / dest_name  # line 331
dest_path.write_bytes(file_obj.read())
extracted_count += 1

If two tar members share a basename (e.g. figures/plot.png and appendix/plot.png), the second silently overwrites the first. extracted_count increments for both, so the reported count exceeds actual unique files on disk.

Fix: Detect collisions (e.g. prefix with parent directory name) or deduplicate before counting.