Skip to content

⚡️ refactor: critical fixes + performance optimization - #34

Merged
frack113 merged 11 commits into
mainfrom
refactor/critical-fixes
Sep 8, 2026
Merged

⚡️ refactor: critical fixes + performance optimization#34
frack113 merged 11 commits into
mainfrom
refactor/critical-fixes

Conversation

@frack113

@frack113 frack113 commented Sep 8, 2026

Copy link
Copy Markdown
Owner

Summary

9-commit refactor of the SigmaForge codebase addressing security bugs, blocking I/O, and runtime performance.

Changes

Phase 1 — Critical Bug Fixes

  • SSRF bypass fix in URL validation
  • Path traversal fix in file upload/download
  • ToolContext injection into tool executor
  • Broken import chain fixes
  • asyncio.to_thread for blocking version checks

Phase 2 — Dedup & Dependency Hygiene

  • force_hf_online() context manager to prevent accidental offline mode
  • SHA-256 dedup across download paths (raw httpx → LlamaClient)
  • Cancel endpoint status fix
  • Dependency consolidation

Phase 3 — CI Pipeline

  • GitHub Actions workflow (ruff, mypy, pytest, Biome)
  • mypy type-error fixes across the codebase

Phase 4 — Event Loop Blocking

  • logs.py, files.py, repo_router.py: file I/O moved to thread pool
  • prompts.py: _ensure_loaded guard to prevent redundant reads

Tier 1 — Event Loop & I/O Performance

  • HuggingFace model downloads → asyncio.to_thread (API stays responsive)
  • Single LlamaClient instance shared across SearchEngine + RAGPipeline
  • Upload: streaming compute_sha256_file() replaces double full-read
  • ToolDef.has_ctx pre-computed at decoration (no per-call inspect.signature)

Tier 2 — Caching, Persistence, Streaming

  • Retriever: module-level AsyncQdrantClient singleton + VectorStoreIndex cache per (collection, alpha)
  • set_config: persist=False kwarg; config endpoints batch writes, single db.persist() per request
  • download_file: streams response in 1 MiB chunks to disk (no full-buffer in RAM)
  • list_repos: fetch_remote=False default (no N+1 network calls on list)

Tier 3 — Quick Wins

  • sigma_ref_downloader: compute_sha256_file() replaces read_bytes() on multi-GB files
  • logs.py: single read_bytes() + in-memory decode attempts (was up to 5 full-file re-opens)
  • TaskDispatcher: poll interval 1.0s → 0.2s, max_workers default 1 → 4
  • Shared async httpx client pool: 4 throwaway AsyncClient per GitHub API call → 1 pooled client with keep-alive

Validation

ruff check main.py src/ tests/ scripts/   # clean
mypy src/ --no-error-summary              # clean
pytest tests/ -q --deselect tests/unit/sigma/  # 46 pre-existing failures, 0 new

The 46 pre-existing failures (sigma_ref_*, qdrant downloader, fusion_retriever, search router) exist on main and are unaffected by this branch.

Files Changed

~40 files across src/shared/, src/api/, src/application/, src/core/, src/infrastructure/, src/workers/, tests/, .github/

@frack113
fix: Phase 1 — critical bug fixes
b877100 
- Fix SSRF bypass in _validate_git_url: IP-address hostnames with
  non-standard ports were silently accepted (except ValueError swallowed
  the ip_address parse). Now hostnames without an IP component are
  allowed; private IPs are always blocked.
- Fix path traversal in delete_local_file: resolve() + relative_to()
  check prevents deleting files outside the documents directory.
- Fix ToolContext injection: tools with a ctx param now receive the
  ToolContext from ChatService instead of None. Executor only passes
  ctx when the function signature accepts it. Registry excludes ctx
  from generated JSON schema.
- Fix broken imports in scripts: src.back.utils -> src.shared.utils,
  src.back.database -> src.infrastructure.database.
- Move llama.cpp/Qdrant version checks to asyncio.to_thread() to avoid
  blocking the event loop.
- Add tests/unit/application/tools/test_ctx_injection.py (6 tests).
- Update SSRF tests to match corrected behavior.
@frack113
refactor: Phase 2 — dedup, cleanup, dependency hygiene
74c0e25 
- Centralize HF_HUB_OFFLINE monkey-patch into force_hf_online()
  context manager (src/shared/utils/hf_hub.py). Replaced 6
  copy-pasted save/set/restore blocks across 3 files.
- Deduplicate SHA-256 hashing: replaced 5 inline hashlib.sha256
  call sites with compute_sha256_file/bytes/str from
  src/shared/utils/crypto_utils.py.
- Remove 3rd LLM path: replace raw httpx POST in
  src/api/v1/sigma/explain.py with LlamaClient.chat() call.
- Fix fake cancel endpoint: no longer claims 'cancelled' status
  without a job registry; returns 'acknowledged' with honest note.
- Add missing direct dependencies to pyproject.toml:
  aiosqlite>=0.22, portalocker>=3.2 (were transitive-only).
- Add src/shared/utils/hf_hub.py (new module).
@frack113
ci: Phase 3 — add GitHub Actions CI + fix mypy errors
bfce279 
- Add .github/workflows/ci.yml with 3 jobs:
  lint (ruff check + format), typecheck (mypy), test (pytest).
- Fix 4 mypy errors so typecheck passes clean:
  - session.py: use lambda instead of dict.get as min() key
  - chat/service.py: add cast() for session store return values
- Add cast to typing import in service.py.
@frack113
perf: Phase 4 — eliminate blocking I/O from event loop
ada0b2c 
- logs.py: move SSE log reads (full + tail) to asyncio.to_thread()
  so the 500ms polling loop no longer stalls concurrent requests.
- logs.py: get_logs endpoint now awaits asyncio.to_thread(read_log_file).
- files.py: upload write+identify+hash moved to asyncio.to_thread()
  so large uploads don't freeze the event loop.
- repo_router.py: list_repos_handler GitPython loop extracted to
  sync helper invoked via asyncio.to_thread().
- prompts.py: _ensure_loaded() now guards with 'if not _prompts' to
  avoid 2-3 redundant locked DuckDB scans per chat message.
@frack113
fix: restore CI branch coverage + Biome check, keep success status in…
71a64e4 
… cancel

- CI: push triggers on all branches (was main-only)
- CI: add Biome check for src/presentation/static/
- CI: include main.py in ruff paths
- Cancel endpoint: outer status stays 'success' (non-breaking for clients)
@frack113
perf: Tier 1 optimizations (event loop, LLM reuse, I/O, reflection)
400485d 
- HF downloads (llm + embedding) now run in asyncio.to_thread
  instead of blocking the event loop for multi-GB transfers
- ChatService creates LlamaClient once and shares it between
  SearchEngine (router) and RAGPipeline — eliminates per-query
  client construction and second dead SearchEngine
- File upload: replaced read_bytes() + sha256_bytes with
  streaming compute_sha256_file — eliminates double full-file read
- ToolDef: pre-compute has_ctx at @tool registration; executor
  no longer calls inspect.signature per dispatch
@frack113
perf: Tier 2 optimizations (retriever cache, batch persist, streaming…
3213ea2 
… download, N+1 repos)

- Cache VectorStoreIndex per (collection, alpha) in retrievers.py;
  shared AsyncQdrantClient singleton; invalidation via
  reset_search_embed_model()
- set_config: add persist=False param; config endpoints batch writes
  and call persist() once per request instead of per key
- download_file: stream response in 1MiB chunks to disk instead of
  buffering full content in memory
- list_repos: fetch_remote=False default (no network per repo);
  explicit sync endpoint can opt in
- Update test_http.py mocks for streaming interface
@frack113
perf: Tier 3 quick wins (streaming hash, single-read logs, faster dis…
9a78333 
…patcher)

- sigma_ref_downloader: compute_sha256_file() replaces read_bytes()
  + compute_sha256_bytes() to avoid loading multi-GB files into RAM
- logs: single read_bytes() + in-memory decode attempts instead of
  up to 5 full-file re-opens for encoding detection
- TaskDispatcher: poll_interval 1.0s → 0.2s, max_workers default
  1 → 4 to reduce task pickup latency
@frack113
perf: shared async httpx client pool (GitHub API)
6d218b0 
- Add get_async_pooled_client() / close_all_async_pooled_clients()
  to shared/http.py (mirrors the sync pool pattern)
- Refactor github/api.py: 4 throwaway AsyncClient per call → 1
  shared pooled client with keep-alive connection reuse
- Wire close_all_async_pooled_clients() into lifespan shutdown
- Update test_api.py to mock the pool factory instead of
  httpx.AsyncClient directly
@frack113 frack113 changed the title refactor: critical fixes + performance optimization (4 phases + 3 tiers) ⚡️ refactor: critical fixes + performance optimization Sep 8, 2026
@frack113
frack113 merged commit f315b03 into main Sep 8, 2026
3 checks passed
@frack113
frack113 deleted the refactor/critical-fixes branch September 8, 2026 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@frack113