Skip to content

[Bug] GET /jobs/{job_id}/findings uses brittle dual-path row access due to inconsistent row_factory #307

Description

@arpit2006

What happened

GET /jobs/{job_id}/findings contains a defensive two-branch block (lines 869–882) to read raw_finding_count and finding_count from a SQLite row. One branch handles dict-like aiosqlite.Row objects; the other handles plain tuples. This fragile workaround exists because db.row_factory = aiosqlite.Row is set inconsistently across connection sites in the codebase — some callers set it, some do not.

This leads to a maintenance hazard: future schema changes or new query sites can silently regress because the code silently tries a second access strategy instead of failing fast.

Steps to reproduce

The bug is structural rather than a visible crash. To observe the code path:

  1. Open backend/app/main.py, lines 864–904.
  2. Notice db.row_factory is not set before the SELECT job_id, raw_finding_count, finding_count query.
  3. The resulting job_row is a plain tuple — the code reaches the else branch (line 878) that accesses job_row[1] and job_row[2] by index.
  4. Compare with _run_repo_scan_task (line 1127) where db.row_factory = aiosqlite.Row IS explicitly set — making the two sites behave differently.

Expected behaviour

get_db() should return a connection with row_factory = aiosqlite.Row already set, giving every caller a consistent dict-like interface. The dual-branch workaround in get_findings can then be simplified to a single clean dict access.

Actual behaviour

Relevant code — backend/app/main.py, lines 869–882:

if job_row is not None:
    if hasattr(job_row, "keys") or isinstance(job_row, dict):
        try:
            raw_finding_count = job_row["raw_finding_count"]
        except (KeyError, IndexError):
            pass
        try:
            finding_count = job_row["finding_count"]
        except (KeyError, IndexError):
            pass
    else:
        if len(job_row) > 1:
            raw_finding_count = job_row[1]
        if len(job_row) > 2:
            finding_count = job_row[2]

get_db() in backend/app/db.py, line 118–119:

async def get_db():
    return await aiosqlite.connect(DB_PATH)
    # ← row_factory is NOT set here

Environment

Field Value
OS Any
Python version 3.10+
PatchPilot version / commit main

Logs

No error — the code silently falls into the tuple-index branch and works, masking the root cause.

Additional context

Fix — two changes:

  1. backend/app/db.py — set row_factory in get_db():
async def get_db():
    db = await aiosqlite.connect(DB_PATH)
    db.row_factory = aiosqlite.Row
    return db
  1. backend/app/main.py — simplify the dual-branch block in get_findings to:
raw_finding_count = job_row["raw_finding_count"]
finding_count = job_row["finding_count"]

Any other callers that previously set db.row_factory = aiosqlite.Row themselves can remove that line (it becomes redundant).

Acceptance criteria:

  • get_db() sets db.row_factory = aiosqlite.Row before returning the connection.
  • The dual-branch hasattr/isinstance block in get_findings is removed and replaced with a single dict access.
  • All existing tests pass without modification.
  • No regression in any endpoint that previously set row_factory manually.

Metadata

Metadata

Assignees

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions