Skip to content

Code quality audit — 23 issues across backend and frontend #10

@KTS-o7

Description

@KTS-o7

Code Quality Audit

Full audit of the codebase post-merge of PRs #8 and #9. Issues are grouped by severity.


High Severity

BUG-1 — IncidentResolutionPanel.tsx:21 — Incident Resolution feature is completely broken

runIncidentResolution() is defined in api.ts:51 as requiring two arguments (context and rootCauseExpln). The call site passes zero arguments — this is a TypeScript compile error and always throws at runtime. The _context and _root_cause_expln returned by /log/analyse are never stored in the Zustand store, so they cannot be forwarded to /incident/resolve.

Fix: Add _context and _root_cause_expln to the store, then pass them through:

const res = await runIncidentResolution(store.analysisMeta._context, store.analysisMeta._root_cause_expln);

BUG-2 — api.ts:3-7LogAnalysisResponse interface missing _context and _root_cause_expln

The backend returns both fields in the /log/analyse response (routes.py:117-118) but the TypeScript interface omits them. They are silently discarded on the client. This is the root cause of BUG-1.

Fix:

export interface LogAnalysisResponse {
  severity: string;
  root_cause: string;
  summary: string[];
  _context: Record<string, unknown>;
  _root_cause_expln: string;
}

SEC-1 — docker-compose.yaml:22-23 — Default MongoDB password changeme committed in source

MONGO_INITDB_ROOT_PASSWORD=${MONGO_PASSWORD:-changeme} means any deployment where MONGO_PASSWORD is not explicitly set launches MongoDB with a well-known password. Additionally MONGO_URI defaults to mongodb://localhost:27017/ with no auth, so MongoDB failures are also invisible (see ERR-2).

Fix: Remove the :-changeme fallback. Require users to set MONGO_PASSWORD explicitly.


SEC-2 — routes.py:129-130, 178, 220 — Internal exception messages returned to clients

raise HTTPException(status_code=500, detail=f"Error analysing log: {str(e)}") returns raw exception messages — including internal paths, service errors, and stack context — verbatim in HTTP responses. This is an information-disclosure risk.

Fix: Log the full exception server-side and return a generic message:

logger.exception("Unexpected error in analyse_log")
raise HTTPException(status_code=500, detail="Internal server error")

ERR-1 — database_handlers.py:72-73get_collection() swallows all exceptions and returns None

except Exception as e: return None — but add_documents() and query_collection() call get_collection() without checking for None, so any ChromaDB failure produces an untracked AttributeError ('NoneType' object has no attribute 'add') instead of a clear error.

Fix: Let exceptions propagate from get_collection() or add explicit None guards in callers.


ERR-2 — database_handlers.py:139-141MongoClient connection never validated at startup

MongoClient uses lazy connection — the constructor never raises even if the server is unreachable. Failures only surface on the first actual DB operation and are silently swallowed. MongoDB being down is invisible.

Fix: Call self.client.server_info() in __init__ with a short timeout to eagerly validate the connection.


Medium Severity

BUG-3 — graph_generator.py:169 — Uses module-level ollama.generate instead of configured client

response = ollama.generate(model=model, prompt=prompt, ...)

This uses the default Ollama host (localhost:11434) rather than self.ollama_client which is configured with OLLAMA_HOST and a timeout. In Docker deployments this call silently fails and falls through to _fallback_root_cause, meaning the LLM analysis path never actually runs. The leftover debug print statements at lines 116, 167, 175, 177, 183 confirm this path is broken.

Fix: Use self.ollama_client.generate(...) and remove the debug print calls.


MAINT-1 — log_parser.py:1-4 — Mutates SSL_CERT_FILE environment variable at module import time

if 'SSL_CERT_FILE' in os.environ:
    del os.environ['SSL_CERT_FILE']

Side-effecting process-wide state at import time is surprising and can break legitimate SSL certificate pinning elsewhere.

Fix: Remove this and document the workaround in the README or handle at the deployment layer.


PERF-1 — database_handlers.py:21-38 — Embeddings generated with serial HTTP requests

OllamaEmbeddingFunction.__call__ loops over texts and makes one blocking requests.post per document. This is O(n) serial round-trips. For a batch of 20 chunks this means 20 sequential network calls.

Fix: Use ThreadPoolExecutor to parallelise, or check for Ollama's /api/embed batch endpoint.


MAINT-2 — database_handlers.py:79hash() used for document IDs is non-deterministic

ids = [str(hash(doc)) for doc in documents]

Python's hash() is randomised by PYTHONHASHSEED. Re-uploading the same docs creates duplicates in ChromaDB. Hash collisions silently overwrite entries.

Fix:

import hashlib
ids = [hashlib.sha256(doc.encode()).hexdigest()[:32] for doc in documents]

MAINT-3 — rag.py:52-53import json inside method body

Move to top-level import.


MAINT-4 — graph_generator.py:145import ollama inside method body

Move to top-level import.


TYPE-1 — routes.py:182/incident/resolve accepts untyped dict

async def resolve_incident(body: dict):

No Pydantic validation — a malformed body raises AttributeError and falls through to a generic 500 instead of a descriptive 422.

Fix: Define a ResolveRequest(BaseModel) with _context: dict and _root_cause_expln: str.


TYPE-2 — store.ts:74Date.now() used as unique ID

Not reliable if two analyses complete in the same millisecond.

Fix: Use crypto.randomUUID().


TEST-1 — test_routes.py:109, 156 — Overly permissive assertions

assert any(k in body for k in ("solution", "response", "result", "steps"))

Accepts any one of four different keys, meaning the test passes even with a garbage response. Line 109 has the same pattern.

Fix: Assert exact keys and their types.


TEST-2 — test_routes.py:100 — MongoDB persistence path never asserted

No assertion that mongo.save_dag or mongo.save_context were called after a successful analysis.

Fix:

routes_module.mongo.save_dag.assert_called_once()
routes_module.mongo.save_context.assert_called_once()

Low Severity

MAINT-5 — routes.py:14-17, log_parser.py:12-15logging.basicConfig called in module bodies

Root logger configuration should happen once in main.py. Calling it in module bodies has no effect if another module already called it first.


MAINT-6 — embedding.py:30, database_handlers.py:36print() used for error reporting instead of logging

These messages are not captured by log aggregation and are not suppressible in tests.


MAINT-7 — requirements.txt:17mirascope unused dependency

mirascope is listed but not imported anywhere. Remove it to reduce install size.


MAINT-8 — requirements.txt:24langchain==0.1.6 is outdated and oversized

Only RecursiveCharacterTextSplitter and langchain.schema.Document are used. Replace with the much smaller langchain-text-splitters package.


MAINT-9 — run.sh:46-49docker ps subshell evaluated twice; docker exec -it breaks in CI

docker exec $(docker ps -qf "name=ollama") ollama list
docker exec -it $(docker ps -qf "name=ollama") ollama pull ...

Cache the container ID in a variable. Use docker exec -i (not -it) in non-tty scripts.


TYPE-3 — parsing_data_models.py:9-11timestamp is an unvalidated str used as sort key

Used for graph ordering in graph_generator.py:26,48. A non-ISO-8601 string from the LLM silently produces wrong graph structure.

Fix: Add a @field_validator for ISO 8601 format or type as datetime.


MAINT-10 — store.ts:116 — Zustand persisted store has no schema version

If the store schema changes, old persisted data in localStorage will cause silent hydration bugs.

Fix: Add a version number and migrate function to the persist options.


Summary

ID File Severity Category
BUG-1 IncidentResolutionPanel.tsx:21 High Bug
BUG-2 api.ts:3-7 High Type Safety
SEC-1 docker-compose.yaml:22-23 High Security
SEC-2 routes.py:129,178,220 High Security
ERR-1 database_handlers.py:72-73 High Error Handling
ERR-2 database_handlers.py:139-141 High Error Handling
BUG-3 graph_generator.py:169 Medium Bug
MAINT-1 log_parser.py:1-4 Medium Maintainability
PERF-1 database_handlers.py:21-38 Medium Performance
MAINT-2 database_handlers.py:79 Medium Bug
MAINT-3 rag.py:52-53 Medium Maintainability
MAINT-4 graph_generator.py:145 Medium Maintainability
TYPE-1 routes.py:182 Medium Type Safety
TYPE-2 store.ts:74 Medium Type Safety
TEST-1 test_routes.py:109,156 Medium Test Quality
TEST-2 test_routes.py:100 Medium Test Quality
MAINT-5 routes.py:14, log_parser.py:12 Low Maintainability
MAINT-6 embedding.py:30, database_handlers.py:36 Low Maintainability
MAINT-7 requirements.txt:17 Low Maintainability
MAINT-8 requirements.txt:24 Low Maintainability
MAINT-9 run.sh:46-49 Low Maintainability
TYPE-3 parsing_data_models.py:9-11 Low Type Safety
MAINT-10 store.ts:116 Low Maintainability

Start here: BUG-1 + BUG-2 make the Incident Resolution feature completely non-functional and should be the first thing fixed. BUG-3 means the LLM analysis path silently falls back on every request in Docker. SEC-1 and SEC-2 are the most important security issues.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions