phase: 90_upload_no_scan

All verification is complete and green. Final report:

**Phase 90 (upload_no_scan) — final verification pass: all criteria verified, no fixes needed**

- Verified (not re-implemented): all 3 tasks already complete; working tree carries the phase's uncommitted changes
- Upload pipeline: `_run_upload` stops after row upsert; success = `{"message": "uploaded"}`, null/0/0 progress; `UploadOut` removed from `app/schemas.py`; gates/unpack/swap/failed states intact
- Copy: button "Upload", bare "Processing…", result line "Uploaded \<name\> — press Sync sources to import it."; hint + caption re-pointed at Sync; no "Upload &" remnants in `frontend/`
- Tests: `pytest tests/unit/test_git_sources.py tests/integration/test_git_sources_upload.py tests/unit/test_frontend_sync_upload.py` → 56 passed; E2E phase suite `tests/e2e/test_upload_no_scan.py` → 3 passed (zero docs after upload; ignore edit honored by Sync — 2 added, `notes/skipme.md` excluded; re-upload in-place, still zero docs); affected suites `test_archive_upload_sources.py` + `test_sync_upload_progress.py` + `test_source_removal_cleanup.py` → 15 passed
- Full suite: `uv run pytest --cov=app --cov-report=term-missing` → **1808 passed, 99% coverage** (>90%); `uv run ruff check . && uv run pyright` → clean, 0 errors

**Completion criteria:**
1. Upload settles "ready for sync", zero docs, row + ignore editor — ✅ (E2E test 1)
2. Button "Upload", no "Upload &" copy — ✅ (`rg` empty in frontend/, app/, tests/)
3. RAG-page "Sync sources" imports upload honoring edited ignores — ✅ (isolated E2E test 2)
4. Suite green, >90% coverage, lint clean — ✅ (99%, ruff+pyright clean)
5. Atomic commit — ✅ left to harness per executor rules (no git add/commit run; tree staged-ready)

**Deviations:** `regenerate_overview` still appears once in `git_sources.py` — inside `delete_git_source` (phase-69 locked total-removal behavior, not the upload leg; upload leg is scan-free). `.agents/PLAN.md` does not exist in this repo; locked decisions A1–A4 were enforced from the phase files.

**Next pending phase:** `91_admin_theme_tab` (in `todo/`).
This commit is contained in:
2026-09-09 14:08:18 -04:00
parent 0c8a7b9974
commit 3095c4c577
27 changed files with 2478 additions and 1224 deletions
+115 -129
View File
@@ -26,16 +26,19 @@ list: 404 unknown id, the required body list is normalized + A4-
validated with fixed-detail 422s and REPLACES the row's list wholesale
— an empty list clears all; 200 → the ``GitSourceOut`` shape),
``POST /upload`` (phase 49, backgrounded in phase
64 task 03 — admin archive upload: the ``.tar``/``.tar.gz``/``.tgz``/
``.zip`` name/format gate + the 1 MiB-chunk receive with the
``upload_max_mb`` cap run **inline** and answered 202 the moment the
archive is safely on disk; unpack → swap → row upsert → model check →
single-source scan → change-gated overview then run in a **background
task** — see :func:`upload_archive` and :func:`_run_upload`),
64 task 03, scan deferred in phase 90 — admin archive upload: the
``.tar``/``.tar.gz``/``.tgz``/``.zip`` name/format gate + the 1 MiB-
chunk receive with the ``upload_max_mb`` cap run **inline** and
answered 202 the moment the archive is safely on disk; unpack → swap
→ row upsert then run in a **background task** — and nothing else:
no model check, no import, no overview refresh (phase 90, A1 — the
scan is the RAG page's "Sync sources" button's job) — see
:func:`upload_archive` and :func:`_run_upload`),
``GET /upload/status`` (the phase-32 ``SyncStatus``-shaped in-memory
state of that run — incl. the phase-64 ``current_file`` /
``files_done`` / ``files_total`` progress fields; navigating away from
the page mid-scan no longer aborts anything), ``DELETE /{source_id}``
state of that run — the phase-64 ``current_file`` / ``files_done`` /
``files_total`` keys stay in the set but null/0/0 for the whole run:
uploads have no file-level progress, phase 90 A2; navigating away from
the page mid-upload no longer aborts anything), ``DELETE /{source_id}``
(204 — total removal, phase 69: row + the source's documents (chunks +
embeddings) committed first, then the app-managed on-disk dir). The
whole router sits behind :func:`app.core.auth.require_admin` —
@@ -61,12 +64,16 @@ sibling row sharing the source name keeps the shared documents +
files (only the row goes), and a pruned KB bumps ``sources_version``
exactly once (the phase-53 saved-chat invalidation) with a
best-effort overview refresh. The upload route is the other exception
(phase 64, task 03): after the 202 receive
answer, its background task unpacks the archive, swaps it in, upserts
the row, probes the models, scans the single source
(``import_sources`` with ``prune=True`` + the change-gated overview
refresh), and lands the sync-style counts (the ``UploadOut`` fields)
in the status ``detail``.
(phase 64, task 03; phase 90): after the 202 receive
answer, its background task unpacks the archive, swaps it in, and
upserts the row — and **stops there**: no model probe, no import, no
overview refresh. The scan is the RAG page's Sync button's job
(phase 90, A1 — it gives the owner time to edit the new source's
ignore list first; the sync already imports ``kind='local'`` rows
with prune + each row's ignore list, A4). The terminal ``success``
carries the no-count payload ``{"message": "uploaded"}`` in the status
``detail`` (phase 90, A2 — the key set is unchanged; the UI composes
the user copy).
"""
from __future__ import annotations
@@ -98,8 +105,8 @@ from app.rag.archive_upload import (
swap_in,
unpack_archive,
)
from app.rag.importer import import_sources, normalize_ignore_path
from app.rag.llm import LLMClient, check_models
from app.rag.importer import normalize_ignore_path
from app.rag.llm import LLMClient
from app.rag.overview import regenerate_overview
from app.rag.source_removal import (
has_sibling,
@@ -151,16 +158,17 @@ class UploadStatus:
Mirrors :class:`app.api.sync.SyncStatus` (the phase-32 pattern,
phase 64 task 03): ``state`` is the same four-state machine
(``idle`` / ``running`` / ``success`` / ``failed``); terminal states
carry the run's ``detail`` (success — the ``UploadOut`` fields) or
``error`` (failure — sanitized) so the UI can render the last result
carry the run's ``detail`` (success — the no-count
``{"message": "uploaded"}`` payload, phase 90 A2) or ``error``
(failure — sanitized) so the UI can render the last result
after a page reload (the re-attach behavior, task 05).
Phase 64 (task 03) progress fields: ``current_file`` is the
``source/relative/path`` the scan is processing right now (null
outside the import phase — unpack/swap/row/model-check first — and
in terminal states); ``files_done`` / ``files_total`` carry the
hook's done/total position and survive a terminal state (the run's
last position is useful context next to the error).
Phase 64 (task 03) progress keys: ``current_file`` /
``files_done`` / ``files_total`` stay null/0/0 for the **whole**
run (phase 90, A2 — the key set is unchanged, but uploads have no
file-level progress: unpack has no per-file hook and the scan —
the only thing that had one — moved to the sync, which keeps its
live file label).
"""
state: Literal["idle", "running", "success", "failed"] = "idle"
@@ -390,13 +398,21 @@ def patch_git_source(
async def upload_archive(
file: UploadFile = File(...), # noqa: B008
) -> UploadAccepted:
"""Receive a source archive; scan it in the background (phase 49,
backgrounded in phase 64 task 03 — owner-locked A1/A2).
"""Receive a source archive; unpack it and register the source row
in the background — and nothing else (phase 49, backgrounded in
phase 64 task 03, scan deferred in phase 90 — owner-locked A1/A2).
The upload's job ends with the source row registered and the folder
on disk: no model check, no import, no overview refresh (phase 90,
A1 — the scan is the RAG page's "Sync sources" button's job, which
gives the owner time to edit the new source's ignore list first;
the sync already imports ``kind='local'`` rows with prune + the
row's ignore list, A4).
The **inline (request) work is exactly three gates** — steps 1–3 —
everything else runs in a background task behind
``GET /upload/status`` (the phase-32 ``SyncStatus`` pattern), so
navigating away mid-scan no longer aborts anything:
navigating away mid-upload no longer aborts anything:
1. name/format gate — only ``.tar``/``.tar.gz``/``.tgz``/``.zip``
(422 naming the accepted set) and a safe source name
@@ -414,30 +430,26 @@ async def upload_archive(
4. unpack to a temp sibling (traversal/symlink/device/corrupt/
over-cap → ``failed`` with the task-01 user-safe message, temps
deleted); a zero-entry archive is ``failed`` ``the archive
contains no files`` — an archive with only non-A9 files is a
VALID replacement (the scan indexes nothing, prune removes the
source's docs);
contains no files`` — an archive with only non-importable files
is a VALID replacement (the folder lands and the row registers;
what the KB indexes with it is the sync's call);
5. atomic swap-in — a same-name re-upload replaces the previous
folder in place; a failure leaves the previous folder/row/KB
untouched;
6. upsert the row by ``path`` (``kind='local'``; an existing row is
left as-is — ``added_at`` preserved — and the unique index is
the backstop: a concurrent insert lands ``failed`` with
``a local source with this path already exists: <path>``); the
row's saved ``ignore_paths`` are captured for the scan (phase
89: a re-upload of an existing source honors the list the owner
already saved);
7. fail-fast ``check_models`` — ``ModelUnavailableError`` →
``failed`` with the sanitized message (the phase-49 503 becomes
a status state, A5); the folder/row are already committed, so
the next sync/re-upload retries idempotently;
8. ``import_sources([folder], llm, prune=True, progress=<hook>,
ignore_by_root={folder: row's list})`` (phase 89) + the
change-gated ``regenerate_overview`` — the hook feeds the status
``current_file`` / ``files_done`` / ``files_total``;
9. one INFO log line (PLAN §9 / AGENTS.md rule 10 — ``total_ms`` is
the background run's duration);
10. ``success`` — ``detail`` = the ``UploadOut`` fields.
left as-is — ``added_at`` and ``ignore_paths`` preserved — and
the unique index is the backstop: a concurrent insert lands
``failed`` with ``a local source with this path already exists:
<path>``); the scan the sync later performs reads the row's
ignore list straight off it (phase 89);
7. one INFO log line (PLAN §9 / AGENTS.md rule 10 —
``upload: finished name=… file=… bytes=… total_ms=… state=…``;
unpack+register only, no file counts — the state is ``success``
or ``failed``, one line per run);
8. ``success`` — ``detail = {"message": "uploaded"}`` (no count
fields, phase 90 A2), ``current_file = None``,
``files_done = files_total = 0`` (the key set is unchanged —
the UI composes the user copy).
"""
# 1. Name/format gate — the accepted formats first (the 422 names
# them), then the task-01 safe-name derivation. A BARE suffix
@@ -488,7 +500,8 @@ async def upload_archive(
)
out.write(chunk)
# The archive is safely on disk — 202 is the "successfully
# uploaded" moment (A2). Steps 4–10 run in the background:
# uploaded" moment (phase 64 A2). Steps 4–8 (unpack → swap →
# row upsert — no scan, phase 90) run in the background:
asyncio.create_task(
_run_upload(name, filename, total, upload_root, temp_upload, temp_unpack)
)
@@ -509,11 +522,12 @@ def upload_status() -> dict[str, Any]:
``GET /api/sync/status`` contract, identical key set).
``started_at`` / ``finished_at`` are ISO-8601 strings or null.
``current_file`` (phase 64) is the ``source/relative/path`` the
scan is processing right now — null during the unpack/swap/row/
model phases and in terminal states; ``files_done`` / ``files_total``
carry the hook's position (0/0 idle). The router dependency makes
it admin-only like every other route here.
``current_file`` / ``files_done`` / ``files_total`` stay null/0/0
for the whole run (phase 90, A2 — the key set is unchanged, but
uploads have no file-level progress: the scan the progress
belonged to moved to the sync button, which keeps its live file
label). The router dependency makes it admin-only like every other
route here.
"""
return {
"state": _upload_status.state,
@@ -540,16 +554,19 @@ async def _run_upload(
temp_unpack: Path,
) -> None:
"""The post-202 upload pipeline, one in-process background task
(the phase-32 ``_run_sync`` shape — A1).
(the phase-32 ``_run_sync`` shape — phase 64 A1).
Every failure mode (unpack, zero entries, swap, row, models,
import, anything else) lands in the ``failed`` state with a
Unpack → swap → row upsert — and nothing else (phase 90, A1: the
model check, the import, and the overview refresh are the sync's
job, not the upload's). Every failure mode (unpack, zero entries,
swap, row, anything else) lands in the ``failed`` state with a
sanitized ``error`` string — a background task must die in state,
never as an unobserved exception (A5: post-202 failures are status
states, never HTTP errors). ``CancelledError`` is deliberately *not*
caught: app shutdown cancels the task, and swallowing that would
mask a real stop. The ``finally`` cleans both temps (defensive —
each step already cleans its own) and clears ``_upload_in_progress``.
never as an unobserved exception (phase 64 A5: post-202 failures
are status states, never HTTP errors). ``CancelledError`` is
deliberately *not* caught: app shutdown cancels the task, and
swallowing that would mask a real stop. The ``finally`` cleans both
temps (defensive — each step already cleans its own) and clears
``_upload_in_progress``.
"""
global _upload_in_progress
started = time.monotonic()
@@ -561,6 +578,21 @@ async def _run_upload(
_upload_status.files_total = 0
_upload_status.detail = {}
_upload_status.error = None
def _log_finished(state: str) -> None:
# Per-upload log line (PLAN §9 / AGENTS.md rule 10) — unpack+
# register only, no file counts (the scan's counts belong to
# the sync, phase 90). One line per run, in BOTH terminal
# states; ``total_ms`` is the background run's duration.
logger.info(
"upload: finished name=%s file=%s bytes=%d total_ms=%d state=%s",
name,
filename,
total_bytes,
round((time.monotonic() - started) * 1000),
state,
)
try:
settings = get_settings()
max_bytes = settings.upload_max_mb * 1024 * 1024
@@ -570,8 +602,9 @@ async def _run_upload(
unpack_archive(temp_upload, temp_unpack, max_bytes)
temp_upload.unlink(missing_ok=True)
if not any(temp_unpack.iterdir()):
# Zero entries = a user error. (Only non-A9 files is NOT an
# error — it still has entries and is a valid replacement.)
# Zero entries = a user error. (Only non-importable files
# is NOT an error — it still has entries and is a valid
# replacement.)
raise ArchiveUploadError("the archive contains no files")
# Step 5 — swap in — a same-name re-upload replaces the
# previous folder atomically; a failure leaves it, the row,
@@ -585,8 +618,10 @@ async def _run_upload(
# background task has no request session to leak locks from
# (the old inline ``db.close()`` discipline, now structural).
# No duplicates: an existing row is left exactly as it is
# (``added_at`` preserved); the unique index is the backstop
# for a concurrent insert the pre-check missed.
# (``added_at`` and ``ignore_paths`` preserved — the scan the
# sync performs later reads the list straight off the row,
# phase 89); the unique index is the backstop for a concurrent
# insert the pre-check missed.
path = str(final_dir)
db = SessionLocal()
try:
@@ -601,78 +636,29 @@ async def _run_upload(
raise ValueError(
f"a local source with this path already exists: {path}"
) from None
# Phase 89: the row's saved ignore list, copied to plain
# values while the row is still usable in this session — a
# re-upload of an existing source honors the list the owner
# already saved; a fresh row has no list yet.
ignore_paths = list(row.ignore_paths or [])
finally:
db.close()
# Step 7 — fail-fast models (phase 41): ``ModelUnavailableError``
# lands in the ``failed`` state sanitized (the phase-49 503
# becomes a status state, A5). Nothing is rolled back — the
# folder/row are committed and the next sync/re-upload retries
# idempotently.
llm = LLMClient()
await check_models(llm)
# Step 8 — scan — single source, prune (dropped files leave
# the KB), with the phase-64 progress hook feeding the status,
# then the change-gated overview refresh (phases 31/32). The
# closure captures the module ``_upload_status`` exactly like
# the state assignments above.
def _hook(source: str, rel: str, done: int, total: int) -> None:
_upload_status.current_file = f"{source}/{rel}"
_upload_status.files_done = done
_upload_status.files_total = total
summary = await import_sources(
[final_dir], llm, prune=True, progress=_hook,
ignore_by_root={str(final_dir): ignore_paths},
)
overview = False
if summary.added + summary.updated > 0:
overview = await regenerate_overview(llm)
# Step 9 — per-upload log line (PLAN §9 / AGENTS.md rule 10)
# — moved with the scan: ``total_ms`` is the background run's
# duration.
logger.info(
"upload: name=%s file=%s bytes_in=%d files=%d added=%d updated=%d "
"unchanged=%d pruned=%d errors=%d overview=%s total_ms=%d",
name,
filename,
total_bytes,
summary.files,
summary.added,
summary.updated,
summary.unchanged,
summary.pruned,
summary.errors,
overview,
round((time.monotonic() - started) * 1000),
)
# Step 10 — success: the ``UploadOut`` fields ride in the
# status ``detail`` (the UI renders the same result line from
# the status that the sync button renders from its own).
# Step 7 — the INFO line (``_log_finished`` — PLAN §9 /
# AGENTS.md rule 10) lands together with the terminal state.
# Step 8 — success: the no-count "uploaded" payload rides in
# the status ``detail`` (phase 90 A2 — the key set is
# unchanged; the scan's counts land in the SYNC's status when
# the owner presses the button, and the UI composes the
# user-facing result line from this payload).
_upload_status.state = "success"
_upload_status.finished_at = datetime.now(UTC)
_upload_status.current_file = None # phase 64: keep the final counts
_upload_status.detail = {
"source": name,
"files": summary.files,
"added": summary.added,
"updated": summary.updated,
"unchanged": summary.unchanged,
"pruned": summary.pruned,
"errors": summary.errors,
"chunks": summary.chunks,
"overview": overview,
}
_upload_status.current_file = None
_upload_status.files_done = 0
_upload_status.files_total = 0
_upload_status.detail = {"message": "uploaded"}
_log_finished(_upload_status.state)
except Exception as e: # noqa: BLE001 — a background task dies in state, see above
logger.exception("upload: failed")
_upload_status.state = "failed"
_upload_status.finished_at = datetime.now(UTC)
_upload_status.error = _sanitize_error(str(e))
_upload_status.current_file = None # phase 64: keep the final counts
_upload_status.current_file = None
_log_finished(_upload_status.state)
finally:
_upload_in_progress = False
# No temp may survive any failure path (defensive — each step
+7 -29
View File
@@ -421,40 +421,18 @@ class GitSourceList(BaseModel):
from_env: bool
class UploadOut(BaseModel):
"""The upload run's result fields (phase 49, task 02; phase 64, task 03).
Phase 64 (task 03): ``POST /api/git-sources/upload`` answers 202 the
moment the archive is on disk; these fields become the shape of
``GET /api/git-sources/upload/status`` ``detail`` on ``success`` —
the uploaded source's name (filename minus the archive suffix) plus
the SAME count keys as the admin sync's success ``detail``
(``files``, ``added``, ``updated``, ``unchanged``, ``pruned``,
``errors``, ``chunks`` — ``app.api.sync._run_sync``) and the
``overview`` flag: the Sources page renders the same
"N added · N pruned" result line for both.
"""
source: str
files: int
added: int
updated: int
unchanged: int
pruned: int
errors: int
chunks: int
overview: bool
class UploadAccepted(BaseModel):
"""``POST /api/git-sources/upload`` 202 response (phase 64, task 03).
The archive is **safely on disk** — this is the "successfully
uploaded" moment the Sources page toasts on (owner-locked A2). The
scan itself (unpack → swap → row upsert → model check → import →
overview) runs in a background task behind
uploaded" moment the Sources page toasts on (phase 64 A2). The rest
(unpack → swap → row upsert — and nothing else: no model check, no
import, no overview refresh, phase 90 A1 — the scan is the RAG
page's "Sync sources" button's job) runs in a background task behind
``GET /api/git-sources/upload/status``, whose ``success`` ``detail``
carries the :class:`UploadOut` fields.
carries the no-count ``{"message": "uploaded"}`` payload (phase 90
A2 — the status key set is unchanged; the UI composes the user
copy).
"""
detail: str = "upload received"