phase: 105_hidden_folders_toggle
All completion criteria verified. Everything is green. **Phase 105 final verification pass — all criteria verified** - Verified the full implementation in the working tree: `git_sources.include_hidden` column + alembic `0019` (dev DB at head, column present), `iter_importable_files`/`import_sources` flag support with `str(root)`-keyed map used by both walk and progress pre-walk, `GitSourcePatchIn` rename with optional fields, sync/CLI pipeline wiring (OR-collision), and the per-row "Hidden" checkbox + tag + error line on the Sources page - Unit + integration: `uv run pytest` → exit 0 (2148 tests collected, all pass; this sandbox occasionally swallows pytest's final status line — exit codes verified) - Coverage: `uv run pytest --cov=app --cov-report=term-missing` → **TOTAL 99%** (3879 stmts, 15 miss) — >90% gate ✓ - Dedicated E2E: `uv run pytest tests/e2e/test_hidden_folders_toggle.py -v --no-cov` → **6 passed in 23.20s** (DB up, in isolation) - Regression E2E in isolation: `test_source_ignore_paths` 6 passed, `test_git_sources_admin` 6 passed, `test_local_directory_sources` 3 passed, `test_sync_button` 3 passed, `test_smoke` 3 passed - Lint/types: `uv run ruff check .` + `uv run pyright` → clean (0 errors/warnings) **Completion criteria:** (1) checkbox persists via PATCH 200 → "hidden on" tag + GET round-trips `include_hidden: true`; failure path reverts box + `role="alert"` canned message ✓; (2) flag OFF byte-identical (only `visible.md` indexed), ON indexes `.hidden/note.md` into the KB catalog, `EXCLUDED_DIRS` excluded both states ✓; (3) A2: flag OFF → `detail.pruned==1`, doc gone from catalog ✓; (4) PATCH bool-only/list-only/both/neither no-op, phase-89 fixed 422s unchanged, 404, anonymous 403 (incl. bool-only body) ✓; (5) env-fallback rows render no checkbox, WCAG-clean (aria-label, keyboard focus, visible label, text tag) ✓; (6) full gate green ✓; (7) commit left to the harness per instructions (no `git add`/`commit` run; phase files untouched). **Deviations:** none — no defects found; no code changes were needed on this pass. **Next pending phase:** `.agents/phases/todo/98_sync_summary_visibility`.
This commit is contained in:
@@ -27,15 +27,19 @@ is asserted against the canned value).
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
from collections.abc import Iterator
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
from sqlalchemy import text
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
from app.config import Settings
|
||||
from app.models import GitSource
|
||||
from app.rag.importer import ImportSummary
|
||||
from scripts import import_docs
|
||||
from scripts.git_sync import GitSyncError
|
||||
from tests.fakes import FakeEmbedder
|
||||
|
||||
|
||||
def _settings(git_sources: str = "", sources_dir: str = "~/bor-sources") -> Settings:
|
||||
@@ -47,11 +51,31 @@ def _git_row(url: str, ignore_paths: list[str] | None = None) -> GitSource:
|
||||
return GitSource(url=url, kind="git", ignore_paths=ignore_paths or [])
|
||||
|
||||
|
||||
def _local_row(path: str, ignore_paths: list[str] | None = None) -> GitSource:
|
||||
def _local_row(
|
||||
path: str,
|
||||
ignore_paths: list[str] | None = None,
|
||||
include_hidden: bool = False,
|
||||
) -> GitSource:
|
||||
"""A local row as the phase-38 API stores it: the expanded path in
|
||||
both ``path`` and the NOT-NULL ``url`` location column (plus the
|
||||
phase-89 ignore list, empty by default)."""
|
||||
return GitSource(url=path, kind="local", path=path, ignore_paths=ignore_paths or [])
|
||||
phase-89 ignore list and the phase-105 hidden-folders flag, both
|
||||
defaulting off — A4)."""
|
||||
return GitSource(
|
||||
url=path, kind="local", path=path,
|
||||
ignore_paths=ignore_paths or [], include_hidden=include_hidden,
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def clean_documents(db: Session) -> Iterator[None]:
|
||||
"""Phase 105: the real-import CLI tests write ``documents``/
|
||||
``chunks`` (the canonical KB state) — global, truncated around
|
||||
every such test."""
|
||||
db.execute(text("TRUNCATE chunks, documents"))
|
||||
db.commit()
|
||||
yield
|
||||
db.execute(text("TRUNCATE chunks, documents"))
|
||||
db.commit()
|
||||
|
||||
|
||||
class FakeImportSources:
|
||||
@@ -68,10 +92,12 @@ class FakeImportSources:
|
||||
prune: bool = False,
|
||||
limit: int | None = None,
|
||||
ignore_by_root: dict[str, list[str]] | None = None, # phase 89
|
||||
include_hidden_by_root: dict[str, bool] | None = None, # phase 105
|
||||
) -> ImportSummary:
|
||||
self.calls.append(
|
||||
{"sources": list(sources), "prune": prune, "limit": limit,
|
||||
"ignore_by_root": ignore_by_root}
|
||||
"ignore_by_root": ignore_by_root,
|
||||
"include_hidden_by_root": include_hidden_by_root}
|
||||
)
|
||||
return ImportSummary(files=1, added=1)
|
||||
|
||||
@@ -168,10 +194,16 @@ def test_resolve_sources_git_urls_cloned_into_sources_dir(
|
||||
sources_dir=str(tmp_path / "bor"),
|
||||
)
|
||||
|
||||
sources, ignore_map = import_docs._resolve_sources(None, settings)
|
||||
sources, ignore_map, hidden_map = import_docs._resolve_sources(None, settings)
|
||||
|
||||
assert sources == [tmp_path / "bor" / "homelab", tmp_path / "bor" / "deploy"]
|
||||
assert ignore_map == {} # phase 89: no row carries a list → empty map
|
||||
# Phase 105: flag-off rows still contribute their root — with False
|
||||
# (A4), keyed by the SAME root string the importer sees.
|
||||
assert hidden_map == {
|
||||
str(tmp_path / "bor" / "homelab"): False,
|
||||
str(tmp_path / "bor" / "deploy"): False,
|
||||
}
|
||||
assert calls == [
|
||||
("https://host/a/homelab.git", tmp_path / "bor" / "homelab"),
|
||||
("git@host:user/deploy.git", tmp_path / "bor" / "deploy"),
|
||||
@@ -186,10 +218,11 @@ def test_resolve_sources_cli_source_wins(
|
||||
settings = _settings(git_sources="https://host/a/repo.git")
|
||||
manual = tmp_path / "Manual"
|
||||
|
||||
sources, ignore_map = import_docs._resolve_sources([manual], settings)
|
||||
sources, ignore_map, hidden_map = import_docs._resolve_sources([manual], settings)
|
||||
|
||||
assert sources == [manual]
|
||||
assert ignore_map == {} # phase 89: manual dirs have no rows → no ignore
|
||||
assert hidden_map == {} # phase 105: manual dirs have no rows → hidden skipped
|
||||
assert calls == [] # git is never touched when --source is given
|
||||
|
||||
|
||||
@@ -210,10 +243,12 @@ def test_resolve_sources_db_rows_win_over_env(
|
||||
sources_dir=str(tmp_path / "bor"),
|
||||
)
|
||||
|
||||
sources, ignore_map = import_docs._resolve_sources(None, settings)
|
||||
sources, ignore_map, hidden_map = import_docs._resolve_sources(None, settings)
|
||||
|
||||
assert sources == [tmp_path / "bor" / "only"]
|
||||
assert ignore_map == {} # phase 89: no row carries a list → empty map
|
||||
# Phase 105: the default-flag row contributes its root with False (A4).
|
||||
assert hidden_map == {str(tmp_path / "bor" / "only"): False}
|
||||
assert calls == [("https://db.example/only.git", tmp_path / "bor" / "only")]
|
||||
|
||||
|
||||
@@ -222,9 +257,10 @@ def test_resolve_sources_defaults_when_nothing_configured(
|
||||
) -> None:
|
||||
# Both origins empty (the resolver's ``([], "env")``) → legacy dirs.
|
||||
monkeypatch.setattr(import_docs, "effective_sources", lambda db: ([], "env"))
|
||||
sources, ignore_map = import_docs._resolve_sources(None, _settings())
|
||||
sources, ignore_map, hidden_map = import_docs._resolve_sources(None, _settings())
|
||||
assert sources == [p.expanduser() for p in import_docs.DEFAULT_SOURCES]
|
||||
assert ignore_map == {} # phase 89: the legacy fallback has no rows
|
||||
assert hidden_map == {} # phase 105: the legacy fallback has no rows
|
||||
|
||||
|
||||
def test_resolve_sources_rows_branch_builds_ignore_map(
|
||||
@@ -250,11 +286,13 @@ def test_resolve_sources_rows_branch_builds_ignore_map(
|
||||
)
|
||||
settings = _settings(sources_dir=str(tmp_path / "bor"))
|
||||
|
||||
sources, ignore_map = import_docs._resolve_sources(None, settings)
|
||||
sources, ignore_map, hidden_map = import_docs._resolve_sources(None, settings)
|
||||
|
||||
assert sources == [tmp_path / "bor" / "only", local_dir]
|
||||
# Keyed by the SAME string the importer sees (the root, not the name).
|
||||
assert ignore_map == {str(local_dir): ["ignore/"]}
|
||||
# Phase 105: both rows are flag-off → per-root False entries (A4).
|
||||
assert hidden_map == {str(tmp_path / "bor" / "only"): False, str(local_dir): False}
|
||||
|
||||
|
||||
def test_resolve_sources_two_rows_sharing_root_string_extend(
|
||||
@@ -279,11 +317,14 @@ def test_resolve_sources_two_rows_sharing_root_string_extend(
|
||||
)
|
||||
settings = _settings(sources_dir=str(tmp_path / "bor"))
|
||||
|
||||
sources, ignore_map = import_docs._resolve_sources(None, settings)
|
||||
sources, ignore_map, hidden_map = import_docs._resolve_sources(None, settings)
|
||||
|
||||
shared = str(tmp_path / "bor" / "shared")
|
||||
assert sources == [tmp_path / "bor" / "shared", tmp_path / "bor" / "shared"]
|
||||
assert ignore_map == {shared: ["a/", "b"]} # union, row order
|
||||
# Phase 105 collision: the shared root gets the OR of the flags —
|
||||
# both rows off here, so one False entry for the one root string.
|
||||
assert hidden_map == {shared: False}
|
||||
|
||||
|
||||
def test_main_rows_branch_passes_ignore_map_to_import(
|
||||
@@ -318,9 +359,145 @@ def test_main_rows_branch_passes_ignore_map_to_import(
|
||||
call = fake_import.calls[0]
|
||||
assert call["sources"] == [local_dir]
|
||||
assert call["ignore_by_root"] == {str(local_dir): ["ignore/"]}
|
||||
# Phase 105: the default-flag row passes the per-root map too — a
|
||||
# False entry, not an absent key (the importer reads it per root).
|
||||
assert call["include_hidden_by_root"] == {str(local_dir): False}
|
||||
assert call["prune"] is False # the CLI's no-prune default is unchanged
|
||||
|
||||
|
||||
# --- phase 105: per-row hidden-folders flag ---------------------------------
|
||||
|
||||
|
||||
def test_main_rows_branch_passes_include_hidden_map_to_import(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
tmp_path: Path,
|
||||
capsys: pytest.CaptureFixture[str],
|
||||
) -> None:
|
||||
"""Phase 105: a ``kind=local`` row with ``include_hidden=True`` →
|
||||
``main`` passes the per-root flag map to ``import_sources`` (keyed
|
||||
by the directory string — the map is built, not lost)."""
|
||||
settings = _settings(sources_dir=str(tmp_path / "bor"))
|
||||
monkeypatch.setattr(import_docs, "get_settings", lambda: settings)
|
||||
local_dir = tmp_path / "LocalDocs"
|
||||
local_dir.mkdir()
|
||||
(local_dir / "visible.md").write_text("# Visible\nin scope\n", encoding="utf-8")
|
||||
(local_dir / ".hidden").mkdir()
|
||||
(local_dir / ".hidden" / "note.md").write_text("# Note\nhidden\n", encoding="utf-8")
|
||||
monkeypatch.setattr(
|
||||
import_docs,
|
||||
"effective_sources",
|
||||
lambda db: ([_local_row(str(local_dir), include_hidden=True)], "db"),
|
||||
)
|
||||
fake_import = FakeImportSources()
|
||||
monkeypatch.setattr(import_docs, "import_sources", fake_import)
|
||||
_stub_bump(monkeypatch)
|
||||
_stub_folder_summaries(monkeypatch)
|
||||
|
||||
rc = import_docs.main([])
|
||||
|
||||
assert rc == 0
|
||||
call = fake_import.calls[0]
|
||||
assert call["sources"] == [local_dir]
|
||||
assert call["include_hidden_by_root"] == {str(local_dir): True}
|
||||
assert call["ignore_by_root"] == {} # the row carries no ignore list
|
||||
assert call["prune"] is False # the CLI's no-prune default is unchanged
|
||||
|
||||
|
||||
def _stub_overview(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Stub the phase-31 overview regeneration (the CLI's real-import
|
||||
tests: the deterministic ``FakeEmbedder`` must not burn its canned
|
||||
``chat`` on the KB outline — the import is what is under test)."""
|
||||
|
||||
async def fake_overview(llm: object, session: object = None) -> bool:
|
||||
return False
|
||||
|
||||
monkeypatch.setattr(import_docs, "regenerate_overview", fake_overview)
|
||||
|
||||
|
||||
def _kb_docs(db: Session) -> set[tuple[str, str]]:
|
||||
"""The (source, path) pairs of the ``documents`` table."""
|
||||
rows = db.execute(text("SELECT source, path FROM documents")).all()
|
||||
return {(row.source, row.path) for row in rows}
|
||||
|
||||
|
||||
def test_main_local_row_include_hidden_true_indexes_hidden_file(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
tmp_path: Path,
|
||||
db: Session,
|
||||
capsys: pytest.CaptureFixture[str],
|
||||
clean_documents: None,
|
||||
) -> None:
|
||||
"""Phase 105 (A1): the CLI's DB-row path with the flag ON — the
|
||||
file inside the hidden folder is indexed, embedded, and counted
|
||||
like any visible file (real import over a host temp dir, the
|
||||
deterministic ``FakeEmbedder``; the regression this phase most
|
||||
plausibly breaks — the CLI's map built but lost — would leave it
|
||||
out)."""
|
||||
settings = _settings(sources_dir=str(tmp_path / "bor"))
|
||||
monkeypatch.setattr(import_docs, "get_settings", lambda: settings)
|
||||
local_dir = tmp_path / "LocalDocs"
|
||||
local_dir.mkdir()
|
||||
(local_dir / "visible.md").write_text("# Visible\nin scope\n", encoding="utf-8")
|
||||
(local_dir / ".hidden").mkdir()
|
||||
(local_dir / ".hidden" / "note.md").write_text("# Note\nhidden\n", encoding="utf-8")
|
||||
monkeypatch.setattr(
|
||||
import_docs,
|
||||
"effective_sources",
|
||||
lambda db: ([_local_row(str(local_dir), include_hidden=True)], "db"),
|
||||
)
|
||||
monkeypatch.setattr(import_docs, "LLMClient", lambda: FakeEmbedder())
|
||||
_stub_overview(monkeypatch)
|
||||
_stub_bump(monkeypatch)
|
||||
_stub_folder_summaries(monkeypatch)
|
||||
|
||||
rc = import_docs.main([])
|
||||
|
||||
assert rc == 0
|
||||
# The hidden file has a documents row next to the visible one.
|
||||
assert _kb_docs(db) == {
|
||||
("LocalDocs", "visible.md"),
|
||||
("LocalDocs", ".hidden/note.md"),
|
||||
}
|
||||
out = capsys.readouterr().out
|
||||
assert "added=2" in out # both files were imported
|
||||
|
||||
|
||||
def test_main_local_row_include_hidden_false_skips_hidden_file(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
tmp_path: Path,
|
||||
db: Session,
|
||||
capsys: pytest.CaptureFixture[str],
|
||||
clean_documents: None,
|
||||
) -> None:
|
||||
"""Phase 105 (A4): the same fixture with the default flag (off) —
|
||||
the hidden-folder file never enters the KB (the byte-identical
|
||||
pre-phase-105 walk): no documents row, and the summary line counts
|
||||
only the visible file."""
|
||||
settings = _settings(sources_dir=str(tmp_path / "bor"))
|
||||
monkeypatch.setattr(import_docs, "get_settings", lambda: settings)
|
||||
local_dir = tmp_path / "LocalDocs"
|
||||
local_dir.mkdir()
|
||||
(local_dir / "visible.md").write_text("# Visible\nin scope\n", encoding="utf-8")
|
||||
(local_dir / ".hidden").mkdir()
|
||||
(local_dir / ".hidden" / "note.md").write_text("# Note\nhidden\n", encoding="utf-8")
|
||||
monkeypatch.setattr(
|
||||
import_docs,
|
||||
"effective_sources",
|
||||
lambda db: ([_local_row(str(local_dir))], "db"), # flag defaults to False
|
||||
)
|
||||
monkeypatch.setattr(import_docs, "LLMClient", lambda: FakeEmbedder())
|
||||
_stub_overview(monkeypatch)
|
||||
_stub_bump(monkeypatch)
|
||||
_stub_folder_summaries(monkeypatch)
|
||||
|
||||
rc = import_docs.main([])
|
||||
|
||||
assert rc == 0
|
||||
assert _kb_docs(db) == {("LocalDocs", "visible.md")}
|
||||
out = capsys.readouterr().out
|
||||
assert "added=1" in out # the hidden file was never walked
|
||||
|
||||
|
||||
# --- main() ----------------------------------------------------------------
|
||||
|
||||
|
||||
@@ -428,10 +605,12 @@ def test_resolve_sources_mixed_git_and_local(
|
||||
sources_dir=str(tmp_path / "bor"),
|
||||
)
|
||||
|
||||
sources, ignore_map = import_docs._resolve_sources(None, settings)
|
||||
sources, ignore_map, hidden_map = import_docs._resolve_sources(None, settings)
|
||||
|
||||
assert sources == [tmp_path / "bor" / "only", local_dir]
|
||||
assert ignore_map == {} # phase 89: neither row carries a list
|
||||
# Phase 105: both rows default-flag → per-root False entries (A4).
|
||||
assert hidden_map == {str(tmp_path / "bor" / "only"): False, str(local_dir): False}
|
||||
assert calls == [("https://db.example/only.git", tmp_path / "bor" / "only")]
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user