phase: 106_document_dates
Everything is verified green. Final report: **Phase 106 — Document dates (final verification pass; all 10 tasks already complete)** - Verified all phase artifacts: alembic `0020` (dev DB at `0020`), `doc_dates.py`, git `file_commit_dates`, importer `doc_dates_by_root`/`dates_updated`, both entry-point wirings, date APIs + tree `created_at`/`updated_at`, LLM surfaces (prompt block, `read` line 2, appended `ls` field), `apply_recency_boost` in `retrieve()`, UI columns/badge, admin editor, mock-LLM regex — all present and correct; no defects found, no fixes needed. - `uv run pytest --cov=app --cov-report=term-missing` → **2299 passed, TOTAL 99%** (>90% ✓) - `uv run pytest tests/e2e/test_document_dates.py -v --no-cov` → **6/6 passed** in isolation (DB up) - 12 regression E2E suites (retrieval_quality, whole_document_context, agent_document_tools, ls_tree_drilldown, read_truncation_cap, kb_tree, kb_tree_nav, document_viewer, edit_summaries, import_documents, sync_button, hidden_folders_toggle, smoke) → **all green in isolation** - `uv run ruff check .` → clean; `uv run pyright` → **0 errors, 0 warnings** **Completion criteria:** 1) non-null `created_at` + 0020 upgrade/downgrade on dev DB ✓ (real-Alembic integration tests) 2) sync refresh/older/manual-persists/content-reset/no sources_meta bump ✓ 3) zip/tar mtime + future→today ✓ 4) LLM date surfaces + cross-check ✓ 5) UI Created/Updated/badge positions ✓ 6) admin editor set+revert round-trip ✓ 7) old-correct-beats-new-similar (defaults & boost-off) + near-tie + `BOR_RECENCY_BOOST=0` byte-identical ✓ 8) full gate ✓ 9) commit/phase-move — left to harness per instructions. - **Notable:** recency default tuned 0.001 → **0.0007** (task 07 step 5 explicitly permits; measured margins recorded in `test_recency_boost.py` docstring). - **Next pending phase:** none — `todo/` holds only this phase.
This commit is contained in:
+185
-31
@@ -10,6 +10,14 @@ update or clear ``documents.summary`` and re-embed the ``is_summary``
|
||||
chunk (embed first, mutate second — a failed LLM call leaves the row and
|
||||
chunk untouched; the content chunks are never re-embedded, D4).
|
||||
|
||||
PATCH /api/documents/date — the admin document-date editor (phase 106,
|
||||
D7): set the owner's corrected ``documents.created_at`` (normalized
|
||||
through ``app.rag.doc_dates.normalize_doc_date`` — a manually set future
|
||||
date folds to today, D3) + flag it manual, or clear the manual flag
|
||||
(null date — the stored date stands until the next sync). A pure DB
|
||||
write: NO LLM/embedding call — a date is never embedded (the
|
||||
deliberate contrast with the phase-57 ``is_summary`` re-embed).
|
||||
|
||||
GET /api/docs/tree — the admin's full recursive KB tree in one fetch
|
||||
(phase 97, task 02): the same drill-down tree the agent's ``ls``
|
||||
walks, with the file metadata the RAG view's rows and stat cards need
|
||||
@@ -40,10 +48,13 @@ from app.core.auth import require_admin, require_user
|
||||
from app.db import get_db
|
||||
from app.models import Chunk, Document, FolderSummary
|
||||
from app.rag.agent import list_source_names
|
||||
from app.rag.doc_dates import normalize_doc_date
|
||||
from app.rag.folder_summaries import MIN_DOCS_PER_FOLDER, folder_of
|
||||
from app.rag.importer import match_extension
|
||||
from app.rag.llm import EmbeddingError, LLMClient
|
||||
from app.schemas import (
|
||||
DateResult,
|
||||
DateUpdate,
|
||||
DocContent,
|
||||
DocList,
|
||||
DocSummary,
|
||||
@@ -101,10 +112,18 @@ def list_indexed_documents(
|
||||
Document.path,
|
||||
Document.title,
|
||||
func.count(Chunk.id).label("chunks"),
|
||||
Document.created_at,
|
||||
Document.indexed_at,
|
||||
)
|
||||
.outerjoin(Chunk, Chunk.document_id == Document.id)
|
||||
.group_by(Document.id, Document.source, Document.path, Document.title, Document.indexed_at)
|
||||
.group_by(
|
||||
Document.id,
|
||||
Document.source,
|
||||
Document.path,
|
||||
Document.title,
|
||||
Document.created_at,
|
||||
Document.indexed_at,
|
||||
)
|
||||
.order_by(Document.source, Document.path)
|
||||
).all()
|
||||
return DocList(
|
||||
@@ -115,6 +134,7 @@ def list_indexed_documents(
|
||||
path=row.path,
|
||||
title=row.title,
|
||||
chunks=row.chunks,
|
||||
created_at=row.created_at.isoformat(),
|
||||
indexed_at=row.indexed_at.isoformat(),
|
||||
)
|
||||
for row in rows
|
||||
@@ -157,6 +177,7 @@ def get_document_content(
|
||||
title=doc.title,
|
||||
format=doc_format(doc.path, get_settings().import_extension_set),
|
||||
summary=doc.summary,
|
||||
created_at=doc.created_at.isoformat(),
|
||||
content=doc.content,
|
||||
indexed_at=doc.indexed_at.isoformat(),
|
||||
chunks=chunks,
|
||||
@@ -231,6 +252,75 @@ async def update_document_summary(
|
||||
)
|
||||
|
||||
|
||||
@router.patch("/documents/date", response_model=DateResult)
|
||||
def update_document_date(
|
||||
payload: DateUpdate,
|
||||
db: Session = Depends(get_db), # noqa: B008
|
||||
_admin: None = Depends(require_admin), # noqa: B008
|
||||
) -> DateResult:
|
||||
"""Set or clear the owner's correction of a document's creation date.
|
||||
|
||||
Admin-only (phase 106, D7 — the phase-57 split): the document
|
||||
viewer itself stays user-gated (``/documents/content`` — admin OR
|
||||
token holder); only this edit affordance is admin-gated. DB-only
|
||||
(the ``/documents/content`` row-lookup rule): the ``(source, path)``
|
||||
pair is looked up as a row — an unknown pair, including traversal
|
||||
strings such as ``../../etc/passwd``, is simply not a row (→ 404
|
||||
``document not found``), and there is no filesystem access. NO
|
||||
LLM/embedding call — a date is never embedded (no chunk, no
|
||||
retrieval role) — the deliberate contrast with the phase-57
|
||||
``is_summary`` re-embed in :func:`update_document_summary`.
|
||||
|
||||
* **Set** (``date`` present) — ``datetime.fromisoformat`` accepts a
|
||||
bare ``YYYY-MM-DD`` (midnight) and full ISO datetimes; a
|
||||
MALFORMED value 422s here (the model field is an unconstrained
|
||||
``str | None`` on purpose, so the detail can name the field). The
|
||||
parse goes through
|
||||
:func:`app.rag.doc_dates.normalize_doc_date` (D3 — the single
|
||||
choke point: naive → UTC, aware → converted, a manually set
|
||||
FUTURE date also folds to today — consistency with the sourced
|
||||
path) and stores ``created_at`` + ``created_at_manual = True``
|
||||
(the owner's correction — the sync-time importer then SKIPS the
|
||||
refresh on this row, D1/D4).
|
||||
* **Clear** (``date`` null/absent — the "revert to sync"
|
||||
operation) — ``created_at_manual = False`` ONLY: the stored date
|
||||
stands until the next sync refreshes it (the API cannot
|
||||
re-read the source, D7).
|
||||
|
||||
The response echoes the stored state — ``created_at`` (ISO-8601)
|
||||
+ ``created_at_manual`` — the viewer re-renders its Created badge
|
||||
from it (no second fetch).
|
||||
"""
|
||||
doc = db.scalar(
|
||||
select(Document).where(
|
||||
Document.source == payload.source, Document.path == payload.path
|
||||
)
|
||||
)
|
||||
if doc is None:
|
||||
raise HTTPException(status_code=404, detail="document not found")
|
||||
if payload.date:
|
||||
try:
|
||||
parsed = datetime.fromisoformat(payload.date)
|
||||
except ValueError:
|
||||
raise HTTPException(
|
||||
status_code=422,
|
||||
detail="date must be an ISO date or datetime (e.g. 2024-06-15)",
|
||||
) from None
|
||||
doc.created_at = normalize_doc_date(parsed)
|
||||
doc.created_at_manual = True
|
||||
else:
|
||||
# The CLEAR (D7): drop the manual flag only — the stored date
|
||||
# stands until the next sync refreshes it.
|
||||
doc.created_at_manual = False
|
||||
db.commit()
|
||||
return DateResult(
|
||||
source=doc.source,
|
||||
path=doc.path,
|
||||
created_at=doc.created_at.isoformat(),
|
||||
created_at_manual=doc.created_at_manual,
|
||||
)
|
||||
|
||||
|
||||
@router.patch("/folders/summary", response_model=FolderSummaryResult)
|
||||
def update_folder_summary(
|
||||
payload: FolderSummaryUpdate,
|
||||
@@ -305,16 +395,17 @@ def update_folder_summary(
|
||||
|
||||
|
||||
#: One catalogue row the tree builder consumes:
|
||||
#: ``(source, path, title, chunks, indexed_at)`` — the ``GET /api/docs``
|
||||
#: query's columns minus the document ``id`` (the tree has no document
|
||||
#: ids), in the same ``(source, path)`` order; ``indexed_at`` is the
|
||||
#: ISO-8601 string the endpoint converts (the builder stays pure over
|
||||
#: plain types — unit-testable without a DB).
|
||||
TreeDocRow = tuple[str, str, str, int, str]
|
||||
#: ``(source, path, title, chunks, indexed_at, created_at)`` — the
|
||||
#: ``GET /api/docs`` query's columns minus the document ``id`` (the
|
||||
#: tree has no document ids), in the same ``(source, path)`` order;
|
||||
#: ``indexed_at`` / ``created_at`` are the ISO-8601 strings the
|
||||
#: endpoint converts (the builder stays pure over plain types —
|
||||
#: unit-testable without a DB).
|
||||
TreeDocRow = tuple[str, str, str, int, str, str]
|
||||
|
||||
#: One of a source's file rows, already source-scoped:
|
||||
#: ``(path, title, chunks, indexed_at)``.
|
||||
TreeFileRow = tuple[str, str, int, str]
|
||||
#: ``(path, title, chunks, indexed_at, created_at)``.
|
||||
TreeFileRow = tuple[str, str, int, str, str]
|
||||
|
||||
|
||||
def _folder_counts(
|
||||
@@ -339,13 +430,13 @@ def _folder_counts(
|
||||
own descendants, or the file wearing its name, exist).
|
||||
"""
|
||||
folders: set[str] = set()
|
||||
for path, _title, _chunks, _indexed_at in rows:
|
||||
for path, _title, _chunks, _indexed_at, _created_at in rows:
|
||||
folder = folder_of(path)
|
||||
while folder:
|
||||
folders.add(folder)
|
||||
folder = folder_of(folder)
|
||||
counts: dict[str, int] = {folder: 0 for folder in folders}
|
||||
for path, _title, _chunks, _indexed_at in rows:
|
||||
for path, _title, _chunks, _indexed_at, _created_at in rows:
|
||||
if path in folders:
|
||||
counts[path] += 1
|
||||
folder = folder_of(path)
|
||||
@@ -381,27 +472,60 @@ def _level_children(
|
||||
flag: the recursive count ≥ :data:`MIN_DOCS_PER_FOLDER` AND no
|
||||
stored ``folder_summaries`` row for ``(source, sub)`` — the same
|
||||
rule the source node applies (see :func:`build_kb_tree`).
|
||||
|
||||
And, since phase 106 (D9), each folder node carries ``updated_at``
|
||||
= the subtree's MAX document ``created_at``: the max over this
|
||||
folder's direct files' dates and its subfolder children's (already
|
||||
recursive) ``updated_at`` values, via :func:`_subtree_max`.
|
||||
"""
|
||||
children: list[KbTreeFolder | KbTreeFile] = []
|
||||
for sub in sorted(g for g in folders if folder_of(g) == folder):
|
||||
sub_children = _level_children(source, sub, folders, counts, rows, summaries)
|
||||
children.append(
|
||||
KbTreeFolder(
|
||||
path=sub,
|
||||
documents=counts[sub],
|
||||
updated_at=_subtree_max(sub_children),
|
||||
summary=summaries.get((source, sub)),
|
||||
summary_pending=counts[sub] >= MIN_DOCS_PER_FOLDER
|
||||
and (source, sub) not in summaries,
|
||||
children=_level_children(source, sub, folders, counts, rows, summaries),
|
||||
children=sub_children,
|
||||
)
|
||||
)
|
||||
for path, title, chunks, indexed_at in rows:
|
||||
for path, title, chunks, indexed_at, created_at in rows:
|
||||
if folder_of(path) == folder:
|
||||
children.append(
|
||||
KbTreeFile(path=path, title=title, chunks=chunks, indexed_at=indexed_at)
|
||||
KbTreeFile(
|
||||
path=path,
|
||||
title=title,
|
||||
chunks=chunks,
|
||||
created_at=created_at,
|
||||
indexed_at=indexed_at,
|
||||
)
|
||||
)
|
||||
return children
|
||||
|
||||
|
||||
def _subtree_max(children: Sequence[KbTreeFolder | KbTreeFile]) -> str | None:
|
||||
"""A node's ``updated_at`` (phase 106, D9): the subtree's MAX
|
||||
document ``created_at``, computed from the node's direct children —
|
||||
files contribute their ``created_at``, subfolder nodes contribute
|
||||
their (already recursive) ``updated_at``.
|
||||
|
||||
All values share the same UTC ``isoformat()`` shape (the endpoint
|
||||
converts every ``created_at`` before the builder runs), so the
|
||||
LEXICOGRAPHIC max is the chronological max — ISO-8601 strings of
|
||||
one offset order by instant. ``None`` when no child carries a date
|
||||
(a node with no documents at all — the 0-document source).
|
||||
"""
|
||||
dates = [
|
||||
child.created_at if child.kind == "file" else child.updated_at
|
||||
for child in children
|
||||
]
|
||||
dates = [d for d in dates if d is not None]
|
||||
return max(dates) if dates else None
|
||||
|
||||
|
||||
def build_kb_tree(
|
||||
names: Sequence[str],
|
||||
doc_rows: Sequence[TreeDocRow],
|
||||
@@ -413,8 +537,9 @@ def build_kb_tree(
|
||||
|
||||
*names* — the registry source names in order (``app.rag.agent.
|
||||
list_source_names`` — deduped, registry order). *doc_rows* — the
|
||||
catalogue ``(source, path, title, chunks, indexed_at)`` tuples in
|
||||
the ``GET /api/docs`` query order (``source, path``). *summaries* —
|
||||
catalogue ``(source, path, title, chunks, indexed_at, created_at)``
|
||||
tuples (both stamps ISO-8601) in the ``GET /api/docs`` query order
|
||||
(``source, path``). *summaries* —
|
||||
``{(source, folder_path): summary}`` over the stored
|
||||
``folder_summaries`` rows (``folder_path = ""`` = the source root;
|
||||
rows for sources the tree does not list are simply never
|
||||
@@ -433,13 +558,21 @@ def build_kb_tree(
|
||||
source node IS the root); direct subfolders only, in path
|
||||
(sorted) order, a folder existing only under the phase-94
|
||||
existence rule; ``documents`` = the recursive subtree count;
|
||||
``summary`` = the stored row (AI OR manual — any row) or null;
|
||||
``children`` = the folder's own subfolders + direct files, same
|
||||
shape.
|
||||
``updated_at`` (phase 106, D9) = the subtree's MAX document
|
||||
``created_at`` — DERIVED as the builder recurses (the max of the
|
||||
direct files' dates and the children's ``updated_at`` values via
|
||||
:func:`_subtree_max`; the ISO-8601 strings share one
|
||||
``isoformat()`` shape, so the lexicographic max is the
|
||||
chronological one), never stored — ``null`` when the node has no
|
||||
documents at all; ``summary`` = the stored row (AI OR manual —
|
||||
any row) or null; ``children`` = the folder's own subfolders +
|
||||
direct files, same shape.
|
||||
* **File nodes** — direct files only, in input (catalog) order;
|
||||
``path`` source-relative; ``title`` / ``chunks`` / ``indexed_at``
|
||||
verbatim from the catalogue row. File nodes carry NO pending
|
||||
flag (the file table has no description column).
|
||||
``path`` source-relative; ``title`` / ``chunks`` / ``created_at``
|
||||
(phase 106) / ``indexed_at`` verbatim from the catalogue row.
|
||||
File nodes carry NO pending flag (the file table has no
|
||||
description column) and no ``updated_at`` (a file's date IS its
|
||||
``created_at``).
|
||||
* **Pending** — ``summary_pending`` on the SOURCE and every FOLDER
|
||||
node (phase 98, decision D3 — ONE concept): true iff the node's
|
||||
recursive ``documents`` count ≥
|
||||
@@ -460,13 +593,20 @@ def build_kb_tree(
|
||||
existence / count rules, so — for a single-source dataset — its
|
||||
level equals :func:`app.rag.agent.group_folder_listing`'s output
|
||||
(same subfolder ``(path, count, summary)`` triples in order, same
|
||||
file ``(path, title)`` pairs in order — the "UI shows what the
|
||||
agent sees" cross-check, unit-pinned at the root and a nested
|
||||
level).
|
||||
file ``(source, path, title, date)`` 4-tuples in order — phase
|
||||
106, D5: the agent's file lines carry the appended ``date`` field
|
||||
and the tree's file nodes carry ``created_at``; the "UI shows
|
||||
what the agent sees" cross-check, unit-pinned at the root and a
|
||||
nested level, compares the extended shapes).
|
||||
|
||||
D9 (phase 106): ``updated_at`` on every SOURCE and FOLDER node is
|
||||
the subtree's MAX document ``created_at`` — derived, never stored;
|
||||
``None`` for a node with no documents at all (the registered
|
||||
0-document source).
|
||||
"""
|
||||
by_source: dict[str, list[TreeFileRow]] = {}
|
||||
for source, path, title, chunks, indexed_at in doc_rows:
|
||||
by_source.setdefault(source, []).append((path, title, chunks, indexed_at))
|
||||
for source, path, title, chunks, indexed_at, created_at in doc_rows:
|
||||
by_source.setdefault(source, []).append((path, title, chunks, indexed_at, created_at))
|
||||
tree: list[KbTreeSource] = []
|
||||
listed: set[str] = set()
|
||||
for name in names:
|
||||
@@ -499,14 +639,20 @@ def _source_node(
|
||||
``summary_pending`` (phase 98, D3): the whole-source count ≥
|
||||
:data:`MIN_DOCS_PER_FOLDER` AND no stored ``(source, "")`` row —
|
||||
the source-root arm of the rule :func:`build_kb_tree` documents.
|
||||
|
||||
``updated_at`` (phase 106, D9): the source's subtree max —
|
||||
:func:`_subtree_max` over the root level's children; ``None`` for a
|
||||
0-document source (no children, no dates).
|
||||
"""
|
||||
folders, counts = _folder_counts(rows)
|
||||
children = _level_children(source, "", folders, counts, rows, summaries)
|
||||
return KbTreeSource(
|
||||
name=source,
|
||||
documents=len(rows),
|
||||
updated_at=_subtree_max(children),
|
||||
summary=summaries.get((source, "")),
|
||||
summary_pending=len(rows) >= MIN_DOCS_PER_FOLDER and (source, "") not in summaries,
|
||||
children=_level_children(source, "", folders, counts, rows, summaries),
|
||||
children=children,
|
||||
)
|
||||
|
||||
|
||||
@@ -540,15 +686,23 @@ def list_kb_tree(
|
||||
Document.path,
|
||||
Document.title,
|
||||
func.count(Chunk.id).label("chunks"),
|
||||
Document.created_at,
|
||||
Document.indexed_at,
|
||||
)
|
||||
.outerjoin(Chunk, Chunk.document_id == Document.id)
|
||||
.group_by(Document.id, Document.source, Document.path, Document.title, Document.indexed_at)
|
||||
.group_by(
|
||||
Document.id,
|
||||
Document.source,
|
||||
Document.path,
|
||||
Document.title,
|
||||
Document.created_at,
|
||||
Document.indexed_at,
|
||||
)
|
||||
.order_by(Document.source, Document.path)
|
||||
).all()
|
||||
doc_rows: list[TreeDocRow] = [
|
||||
(source, path, title, chunks, indexed_at.isoformat())
|
||||
for source, path, title, chunks, indexed_at in rows
|
||||
(source, path, title, chunks, indexed_at.isoformat(), created_at.isoformat())
|
||||
for source, path, title, chunks, created_at, indexed_at in rows
|
||||
]
|
||||
summaries: dict[tuple[str, str], str] = {
|
||||
(source, folder_path): summary
|
||||
|
||||
Reference in New Issue
Block a user