chore(agent): phase 89 roadmap from TODO.md — per-source ignore paths for imports
This commit is contained in:
@@ -0,0 +1,72 @@
|
|||||||
|
# Phase 89 — Per-source ignore paths: exclude files/folders from import
|
||||||
|
|
||||||
|
**Source:** `TODO.md` L3 — "The user should be able to ignore/exclude specific files and folders when importing from a source. They should be able to type these files and folders into a box on the sources page. Each source should support a list of ignored files and folders. These ignored files/folders should not be embedded and should not be summarized. The format of the ignored files/folders should be a path, so \"/my/files/\" or \"my/files/\" or \"my/files\" would ignore all files that start with the prefix \"my/files\". There shouldn't be any support for \"match in the middle\" of a path, so \"myfile.txt\" would match \"/myfile.txt\" but not \"some/path/myfile.txt\". The ignore is just a prefix ignore."
|
||||||
|
**Story:** n/a (TODO-derived — owner roadmap confirmation 2026-09-08).
|
||||||
|
**Context:** Sources are the `git_sources` rows (kind `git` / `local`, phase 35/38) managed on the Sources page (`/git-sources.html`, view module `frontend/assets/git-sources.js`, skeleton in `frontend/index.html` `#view-git-sources`). Every import entry point funnels through `import_sources` (`app/rag/importer.py`): the in-app Sync button (`app/api/sync.py::_run_sync`), the archive-upload background scan (`app/api/git_sources.py::_run_upload`), and the CLI (`scripts/import_docs.py`). `iter_importable_files` is the single walk choke point (extension filter + hidden-dir rule + `EXCLUDED_DIRS`); `import_sources` also runs a pre-walk with the same rules for the phase-64 progress `total`; ignored files must be excluded from BOTH. Non-markdown files get a `lite` summary in `_store_summary` (phase 30) — a file that never enters the walk is never embedded and never summarized, by construction. Pruning (`_prune`, `prune=True`) deletes every indexed document of the imported sources whose `(source, rel)` is not in `seen` — so files that newly match an ignore pattern leave the index on the next sync, exactly like the A9 out-of-scope-junk behavior. The Sources page is admin-gated (phase 16 pattern); env-fallback rows (`from_env: true`, `id: null`) have no DB row to store a list on.
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
Each stored source carries an **ignore list** of source-relative path prefixes, edited in a per-row box on the Sources page; matching is pure prefix (no mid-path matching, no globs — the spec's examples verbatim); ignored files are never walked, embedded, or summarized, and previously indexed files that newly match an ignore pattern are pruned on the next sync. All three import entry points honor the lists; the API is admin-only like the rest of the router.
|
||||||
|
|
||||||
|
## Dependencies
|
||||||
|
- `88_mobile_chat_hamburger_boot` (complete) — pipeline predecessor (execution order) only; no code dependency (this phase touches the importer, the git-sources API, the sync/upload/CLI pipelines, and the Sources view — none of which phase 88's pins reach; its suites must stay green unchanged).
|
||||||
|
|
||||||
|
## Design (shared by all tasks — the executor reads this, not the chat)
|
||||||
|
|
||||||
|
- **Match semantics (locked, A1).** One pure helper pair in `app/rag/importer.py`:
|
||||||
|
- `normalize_ignore_path(entry: str) -> str` — `entry.strip().strip("/")` (whitespace + all leading/trailing slashes — so `"/my/files/"`, `"my/files/"`, and `"my/files"` normalize to the same `"my/files"`; `"//"` and `""` normalize to `""` and are dropped).
|
||||||
|
- `is_ignored(rel: str, prefixes: tuple[str, ...]) -> bool` — `any(rel.startswith(p) for p in prefixes)` where `rel` is the **source-relative POSIX path without a leading slash** (`path.relative_to(root).as_posix()` — the same string `documents.path` stores) and `prefixes` are the normalized, non-empty entries.
|
||||||
|
- **Raw string prefix — deliberately no component-boundary check** (A1, owner-confirmed): `"my/files"` also matches `"my/files2/x.md"`; `"myfile.txt"` matches `"myfile.txt"` (root-level file) but NOT `"some/path/myfile.txt"` (no mid-path matching — the string simply does not start with the pattern). No globs, no `**`, no case folding beyond what the walk already does for extensions.
|
||||||
|
- The importer normalizes its input itself (single choke point — callers may pass raw box lines): an internal `_ignore_for_root(root, ignore_by_root)` yields `tuple(p for p in (normalize_ignore_path(x) for x in raw) if p)`.
|
||||||
|
- **`iter_importable_files` / `import_sources` signature (task 02).**
|
||||||
|
- `iter_importable_files(root, extensions, excluded=EXCLUDED_DIRS, ignore: tuple[str, ...] = ())` — one more skip: `if ignore and is_ignored(rel.as_posix(), ignore): continue` (after the hidden/excluded check, before/after the extension check — either order is fine; keep the existing checks byte-identical). Default `()` keeps every existing caller (CLI manual `--source`, tests) behaving exactly as before.
|
||||||
|
- `import_sources(sources, llm, *, prune=False, limit=None, session=None, progress=None, ignore_by_root: dict[str, list[str]] | None = None)` — the map is keyed by **`str(root)`** (the root path string exactly as passed in `sources` — unambiguous even when two rows resolve to the same source *name*; on a root-string collision the caller has already merged the lists, Design "Callers" below). The pre-walk that computes the phase-64 progress `total` and the processing loop MUST use the same per-root ignore tuple (same helper), so `files_total` never counts ignored files. `seen` is untouched in shape → `_prune` prunes newly-ignored documents automatically (A2).
|
||||||
|
- **Callers (task 04).**
|
||||||
|
- `app/api/sync.py::_run_sync` — while building `sources` per row, build `ignore_by_root: dict[str, list[str]]` in the same loop: git row → key `str(sources_root / repo_name(row.url))`, local row → key `str(Path(row.path or row.url).expanduser())`; value `row.ignore_paths` — **extend on key collision** (two rows sharing a root string get the union of their lists). Pass `ignore_by_root=…` to `import_sources`.
|
||||||
|
- `app/api/git_sources.py::_run_upload` — step 6 already fetches the row by path (existing or the newly created one); capture it and pass `ignore_by_root={str(final_dir): row.ignore_paths}` to the step-8 `import_sources` (a re-upload of an existing source respects the list the owner already saved).
|
||||||
|
- `scripts/import_docs.py` — `_resolve_sources` returns `tuple[list[Path], dict[str, list[str]]]` (manual `--source` → `({},)` empty map — manual dirs have no row; env-fallback rows have no lists). `main` unpacks and passes the map to `import_sources`; docstrings updated (module + `_resolve_sources`).
|
||||||
|
- **Storage (task 01).** `git_sources.ignore_paths` — JSONB NOT NULL, server default `'[]'`, `Mapped[list]` (the `saved_chats.messages` JSONB precedent, `app/models.py`). Alembic `0013_git_source_ignore_paths.py` (revises `0012`) — `op.add_column("git_sources", sa.Column("ignore_paths", postgresql.JSONB(astext_type=sa.Text()), server_default=sa.text("'[]'"), nullable=False))`; downgrade drops the column. Existing rows read `[]`.
|
||||||
|
- **API contract (task 03).**
|
||||||
|
- Schemas (`app/schemas.py`): `GitSourceIn.ignore_paths: list[str] | None = None` (create-time, optional — absent → `[]`); `GitSourceOut.ignore_paths: list[str]`; `GitSourceRow.ignore_paths: list[str]` (env-fallback rows report `[]`); new `GitSourceIgnoreIn` body model for PATCH with `ignore_paths: list[str]` (required — the box replaces the whole list, A5).
|
||||||
|
- `GET /api/git-sources` — each DB row reports its (normalized, non-empty) list; env rows `[]`.
|
||||||
|
- `POST /api/git-sources` — both kinds accept `ignore_paths`; stored **normalized** (each entry `normalize_ignore_path`, empties dropped) — the UI may send raw box lines and the stored list is canonical.
|
||||||
|
- `PATCH /api/git-sources/{source_id}` (NEW, behind the existing `require_admin` router dependency) — 404 unknown id; **replace** semantics (A5): the body list, normalized, becomes the row's list (empty list = clear all); 200 → `GitSourceOut` (id, url, added_at, ignore_paths).
|
||||||
|
- Validation (422, fixed details that never echo the input — the router's credential-safety discipline, applied for consistency): at most **200 entries** (A4) → `detail="a source has at most 200 ignore paths"`; an entry **empty after normalization** → `detail="ignore paths must be non-empty"`; an entry **>500 chars** (pre- or post-normalization — check post-normalization) → `detail="an ignore path exceeds 500 characters"`.
|
||||||
|
- **UI (task 05).** Sources page = the `git-sources` view. Per **stored** row (`s.id` truthy) in `makeRow`: an **"Ignore paths" button** (actions cell, left of Remove — same `.git-source-remove`-style button idiom, new class `.git-source-ignore`) whose `aria-label` is `Edit ignored paths for source: <value>`; when `s.ignore_paths.length > 0` the row also shows a text count tag `N ignored` (text, never color alone) next to the location cell. Env-fallback rows (`id` null) get **no** button — the existing "from .env" tag stays (A3).
|
||||||
|
- The button opens a **page-local editor dialog** following the phase-69 `#remove-confirm-dialog` pattern exactly (new `#ignore-editor-dialog`, `role="dialog"`, dim backdrop, focus management: focus lands on Cancel — the safe default — Escape / Cancel / backdrop close as CANCEL and return focus to the trigger; the dim backdrop click handler is the same idiom). Inside: a visible `<label for="ignore-editor-textarea">` ("Ignored files and folders — one path per line"), a `<textarea id="ignore-editor-textarea">` (mono, ~6 rows, `spellcheck="false"`, placeholder `my/files/`), a helper line explaining the prefix rule in plain words ("A file is ignored when its path in the source starts with any of these. No wildcards."), Save + Cancel buttons, and a `role="alert"` error line (server 422 details, same rendering as the remove modal's error line).
|
||||||
|
- `frontend/assets/git-sources.js` wiring (all scoped to the view root like everything else in the module): `openIgnoreEditor(s, triggerBtn)` — prefills `s.ignore_paths.join("\n")`; `saveIgnorePaths()` — the **§7.4 never-stale lifecycle**: Save disables + relabels "Saving…" while the PATCH is out, re-enables on success AND failure; on 200 → close the dialog, `loadSources()`, `announce("Ignored paths updated for <value>")`; on non-2xx → the server detail in the `role="alert"` line, textarea content KEPT (the instruction survives, tuning-form convention). Lines are parsed client-side: split on `\n`, trim, **drop empty lines** (a blank line in the box is a separator, not an entry) before submitting.
|
||||||
|
- `frontend/index.html` — the dialog skeleton inside `#view-git-sources` (next to `#remove-confirm-dialog`); `frontend/assets/styles.css` — dialog/textarea/button/count-tag rules reusing the remove-confirm-dialog and `.git-source-remove` idioms (house comment style, citing phase 89).
|
||||||
|
- **NOT touched:** the RAG view (`sources.js` — the catalog stays as-is; the Sync button already exists there), `app/rag/retriever.py`, the chunker, `mock_llm.py`/`slow_llm.py`, chat/SSE, `AGENTS.md`, any completed phase.
|
||||||
|
|
||||||
|
## Tasks
|
||||||
|
1. `01_ignore_paths_column.md` — `git_sources.ignore_paths` JSONB column (model + alembic `0013`) + unit tests.
|
||||||
|
2. `02_ignore_prefix_matching.md` — the pure matcher + `iter_importable_files`/`import_sources` `ignore_by_root` support (walk + progress pre-walk + prune interaction) + unit & integration tests.
|
||||||
|
3. `03_ignore_paths_api.md` — schemas + `GET`/`POST` fields + new admin-only `PATCH /api/git-sources/{id}` (replace semantics, 404/422 contracts) + integration tests.
|
||||||
|
4. `04_ignore_paths_in_import_pipelines.md` — wire the per-row lists into `_run_sync`, the `_run_upload` background scan, and `scripts/import_docs.py` + integration tests.
|
||||||
|
5. `05_ignore_paths_sources_ui.md` — the per-row "Ignore paths" box on the Sources page (dialog + §7.4 save lifecycle + a11y) + source-level unit pins.
|
||||||
|
6. `06_e2e_ignore_paths.md` — dedicated Playwright suite `tests/e2e/test_source_ignore_paths.py` (run in isolation), regressions, full gate, atomic commit.
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- Unit — `tests/unit/test_importer_ignore.py` (new, tasks 01–02): normalization table, the spec's prefix examples verbatim, no-mid-path, raw-prefix A1 edge, empty/drop rules, `iter_importable_files` on a tmp fixture tree; `tests/unit/test_source_ignore_paths.py` (new, task 05): source-level pins for the JS/HTML wiring (house pattern — read the assets as text).
|
||||||
|
- Integration — `tests/integration/test_importer_ignore.py` (new, task 02): `import_sources` against a fixture dir — ignored files produce no `Document`/`Chunk` rows (not embedded, not summarized), previously indexed file newly ignored → pruned, progress `total` excludes ignored files, sources not in the map behave exactly as before; `tests/integration/test_git_sources_api.py` (extended, task 03) and `tests/integration/test_sync_api.py` + the CLI/upload suites (extended, task 04).
|
||||||
|
- E2E (mandatory, A16) — `tests/e2e/test_source_ignore_paths.py` (task 06), run in isolation with the DB up: `uv run pytest tests/e2e/test_source_ignore_paths.py -v --no-cov`.
|
||||||
|
- Coverage: **>90%** on `app/` (the validate.sh gate — the importer/API additions are fully unit+integration covered).
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] A stored source's ignore list (git or local row) is editable in the box on the Sources page: one path per line; Save → `PATCH` 200 → the row shows the `N ignored` count; the list round-trips through `GET /api/git-sources`.
|
||||||
|
- [ ] A sync (button, upload scan, or CLI) with `ignore_paths: ["ignore/"]` (any of the three spellings) indexes nothing under `ignore/` — no `documents`/`chunks` rows, no embeddings, no summary calls for those files — and a previously indexed file under `ignore/` is pruned from the KB on that sync (the RAG catalog no longer lists it).
|
||||||
|
- [ ] The spec's matching examples hold end-to-end: `myfile.txt` ignores the root-level `myfile.txt` only, never `some/path/myfile.txt` (E2E proof).
|
||||||
|
- [ ] `PATCH` contracts hold: 404 unknown id; 422 fixed details for >200 entries / empty entry / >500-char entry; replace semantics (clearing works); anonymous 403.
|
||||||
|
- [ ] `uv run pytest` green; `uv run pytest --cov=app --cov-report=term-missing` TOTAL >90%; `uv run pytest tests/e2e/test_source_ignore_paths.py -v --no-cov` green in isolation (DB up); regression suites `test_git_sources_admin.py`, `test_archive_upload_sources.py`, `test_sync_button.py`, `test_smoke.py` green in isolation; `uv run ruff check . && uv run pyright` clean.
|
||||||
|
- [ ] One `--no-gpg-sign` commit; phase dir moved to `.agents/phases/complete/` by the pipeline gate.
|
||||||
|
|
||||||
|
## Locked decisions
|
||||||
|
- **A1 — raw prefix semantics (owner-confirmed 2026-09-08).** Normalize = trim whitespace + strip leading/trailing `/`; match = `rel.startswith(normalized)` on the source-relative POSIX path. No component boundary (`"my/files"` also ignores `"my/files2/x.md"`), no globs, no mid-path matching.
|
||||||
|
- **A2 — previously indexed files leave the index (owner-confirmed).** A file that newly matches an ignore pattern is pruned from the KB on the next sync (the `seen`-set mechanism, same as A9 out-of-scope junk) — "not embedded, not summarized" also cleans the existing index, not just future imports.
|
||||||
|
- **A3 — box placement (owner-confirmed).** The editor is a per-row control on the Sources page (`/git-sources.html`) for stored rows only; env-fallback rows have no DB row and get no editor.
|
||||||
|
- **A4 — limits (owner-confirmed).** At most 200 entries per source, each ≤500 chars after normalization; entries empty after normalization are rejected with a fixed 422 detail (the UI drops blank lines client-side; the API stays defensive).
|
||||||
|
- **A5 — replace semantics (owner-confirmed).** Saving the box replaces the whole list (textarea content = the list; empty box clears).
|
||||||
|
|
||||||
|
## Commit
|
||||||
|
```bash
|
||||||
|
git add app/ alembic/versions/0013_git_source_ignore_paths.py frontend/ tests/ .agents/phases/ && git commit --no-gpg-sign -m "feat(sources): per-source ignore paths — prefix-excluded files are never embedded, summarized, or re-indexed"
|
||||||
|
```
|
||||||
@@ -0,0 +1,53 @@
|
|||||||
|
# Task 01 — `git_sources.ignore_paths` column (model + migration 0013)
|
||||||
|
|
||||||
|
**Phase:** `89_source_ignore_paths` · **Source:** `TODO.md` L3 — "Each source should support a list of ignored files and folders."
|
||||||
|
**Story:** n/a (TODO-derived).
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
Give every `git_sources` row a JSONB `ignore_paths` list (default empty) so the Sources page can store one ignore list per source — the storage half of the feature; no importer or API behavior changes in this task.
|
||||||
|
|
||||||
|
## Work
|
||||||
|
1. `app/models.py` — `GitSource` (between `SourcesMeta` and `DocDraft`): add the column after `path`, with a docstring comment in the house style (cite phase 89 + the A1 normalization):
|
||||||
|
```python
|
||||||
|
#: Per-source ignore paths (phase 89, A1/A4): the normalized,
|
||||||
|
#: non-empty source-relative path prefixes the owner types into the
|
||||||
|
#: box on the Sources page. A file is ignored when its
|
||||||
|
#: source-relative POSIX path starts with any entry (raw prefix —
|
||||||
|
#: no mid-path matching, no globs). Server default '[]' — every
|
||||||
|
#: pre-phase-89 row imports exactly as before.
|
||||||
|
ignore_paths: Mapped[list] = mapped_column(
|
||||||
|
JSONB, default=list, server_default=text("'[]'"), nullable=False
|
||||||
|
)
|
||||||
|
```
|
||||||
|
(`app/models.py` imports named symbols from `sqlalchemy` — add `text` to that existing import list; `JSONB` is already imported from `sqlalchemy.dialects.postgresql`.) Update the module docstring's `git_sources` bullet with one sentence: "``ignore_paths`` (phase 89 — JSONB list of normalized path prefixes, server default ``'[]'``)".
|
||||||
|
2. `alembic/versions/0013_git_source_ignore_paths.py` (NEW — house header style, mirror `0007_git_sources_kind.py`'s docstring shape: Revision ID `0013`, `Revises: 0012`, one-sentence rationale per column):
|
||||||
|
```python
|
||||||
|
def upgrade() -> None:
|
||||||
|
op.add_column(
|
||||||
|
"git_sources",
|
||||||
|
sa.Column(
|
||||||
|
"ignore_paths",
|
||||||
|
postgresql.JSONB(astext_type=sa.Text()),
|
||||||
|
server_default=sa.text("'[]'"),
|
||||||
|
nullable=False,
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
def downgrade() -> None:
|
||||||
|
op.drop_column("git_sources", "ignore_paths")
|
||||||
|
```
|
||||||
|
3. `tests/unit/test_models_git_source.py` (extend the existing model unit tests if one exists — check `tests/unit/` for a models test module first; otherwise create it) — or a focused new module `tests/unit/test_ignore_paths_column.py`:
|
||||||
|
- the `GitSource.__table__.c.ignore_paths` column exists, is NOT NULL, and its server default is `'[]'` (assert `col.server_default.arg.text == "'[]'"` and `col.nullable is False`);
|
||||||
|
- a freshly constructed `GitSource(url="https://example.com/r.git", kind="git")` has `ignore_paths == []` (the Python-side default) — the ORM default path, no DB needed.
|
||||||
|
4. Apply the migration to the dev DB so later tasks run against the real schema: `uv run alembic upgrade head` (record the head revision in the task report; the E2E/integration suites use the same DB via `podman compose up -d db`).
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- `uv run pytest tests/unit/ -q` green (the new pins + the untouched suite).
|
||||||
|
- Coverage: **>90%** on `app/` unaffected (model column + migration are config, not logic).
|
||||||
|
- `uv run ruff check . && uv run pyright` clean.
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] `uv run alembic upgrade head` applies `0013` cleanly; `alembic downgrade -1 && alembic upgrade head` round-trips (reversible — the 0007 precedent).
|
||||||
|
- [ ] Existing `git_sources` rows (dev DB) read `ignore_paths == []` — no importer/API change yet: `uv run pytest tests/integration/test_git_sources_api.py -q` green UNCHANGED.
|
||||||
|
- [ ] `uv run pytest` green; `uv run ruff check . && uv run pyright` clean.
|
||||||
|
- [ ] `git diff --stat` limited to `app/models.py`, the new migration, and the unit test.
|
||||||
@@ -0,0 +1,88 @@
|
|||||||
|
# Task 02 — Prefix matching + `ignore_by_root` in the importer
|
||||||
|
|
||||||
|
**Phase:** `89_source_ignore_paths` · **Source:** `TODO.md` L3 — "These ignored files/folders should not be embedded and should not be summarized… \"/my/files/\" or \"my/files/\" or \"my/files\" would ignore all files that start with the prefix \"my/files\"… \"myfile.txt\" would match \"/myfile.txt\" but not \"some/path/myfile.txt\". The ignore is just a prefix ignore."
|
||||||
|
**Story:** n/a (TODO-derived).
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
Make the import walk honor per-source ignore lists: ignored files are never indexed (hence never embedded, never summarized), the phase-64 progress `total` never counts them, and previously indexed files that newly match an ignore pattern are pruned on the next `prune=True` run (A2) — while every caller that passes no map behaves byte-identically to today.
|
||||||
|
|
||||||
|
## Work
|
||||||
|
1. `app/rag/importer.py` — add the pure helpers (module level, near `iter_importable_files`; docstrings in the house style, each citing the spec example it encodes):
|
||||||
|
```python
|
||||||
|
def normalize_ignore_path(entry: str) -> str:
|
||||||
|
"""One ignore-path entry → canonical form (phase 89, A1).
|
||||||
|
|
||||||
|
Trim surrounding whitespace, then strip ALL leading/trailing
|
||||||
|
``/`` — so ``"/my/files/"``, ``"my/files/"`` and ``"my/files"``
|
||||||
|
all become ``"my/files"``. ``""`` / ``"//"``,
|
||||||
|
``" "`` normalize to ``""`` (callers drop empties).
|
||||||
|
"""
|
||||||
|
return entry.strip().strip("/")
|
||||||
|
|
||||||
|
|
||||||
|
def is_ignored(rel: str, prefixes: tuple[str, ...]) -> bool:
|
||||||
|
"""Phase 89, A1 — the pure prefix rule, nothing else.
|
||||||
|
|
||||||
|
``rel`` is the source-relative POSIX path WITHOUT a leading
|
||||||
|
slash (the ``documents.path`` string). Match = ``rel`` STARTS
|
||||||
|
WITH a normalized entry: raw string prefix — deliberately NO
|
||||||
|
component-boundary check (``"my/files"`` also matches
|
||||||
|
``"my/files2/x.md"``) and NO mid-path matching (``"myfile.txt"``
|
||||||
|
matches ``"myfile.txt"`` but not ``"some/path/myfile.txt"``).
|
||||||
|
"""
|
||||||
|
return any(rel.startswith(p) for p in prefixes)
|
||||||
|
|
||||||
|
|
||||||
|
def _ignore_for_root(
|
||||||
|
root: Path, ignore_by_root: dict[str, list[str]] | None
|
||||||
|
) -> tuple[str, ...]:
|
||||||
|
"""The normalized, non-empty prefix tuple for one root (phase 89).
|
||||||
|
|
||||||
|
Keyed by ``str(root)`` — the root string exactly as the caller
|
||||||
|
passed it in ``sources`` (unambiguous when two rows share a
|
||||||
|
source *name* but different dirs). Callers may pass RAW box
|
||||||
|
lines: the importer normalizes + drops empties here, the single
|
||||||
|
choke point — stored lists (already normalized) normalize to
|
||||||
|
themselves.
|
||||||
|
"""
|
||||||
|
raw = (ignore_by_root or {}).get(str(root)) or []
|
||||||
|
return tuple(p for p in (normalize_ignore_path(e) for e in raw) if p)
|
||||||
|
```
|
||||||
|
2. `app/rag/importer.py` — `iter_importable_files(root, extensions, excluded=EXCLUDED_DIRS, ignore: tuple[str, ...] = ())`: inside the `for path in sorted(root.rglob("*"))` loop, after the hidden/`excluded`-parts check and before/after the extension check (your choice — keep both existing checks byte-identical), add:
|
||||||
|
```python
|
||||||
|
if ignore and is_ignored(rel.as_posix(), ignore):
|
||||||
|
continue
|
||||||
|
```
|
||||||
|
Update the docstring: one sentence on the `ignore` param (phase 89, A1; default `()` = no behavior change).
|
||||||
|
3. `app/rag/importer.py` — `import_sources(…, ignore_by_root: dict[str, list[str]] | None = None)` (new keyword-only param, default `None`):
|
||||||
|
- compute `ignore_by_root` once per root inside the `for root in sources:` loop: `ignore = _ignore_for_root(root, ignore_by_root)`;
|
||||||
|
- the phase-64 pre-walk (`if progress is not None:` block) must use it — `iter_importable_files(root, llm.settings.import_extension_set, ignore=ignore_for_this_root)` (compute the tuple in the pre-walk too, or hoist: the pre-walk runs before the loop, so call `_ignore_for_root` there as well);
|
||||||
|
- the processing loop passes the same tuple to `iter_importable_files`;
|
||||||
|
- `seen`, `_prune`, and everything else are UNCHANGED — newly-ignored files simply never enter `seen`, so the next `prune=True` run deletes their rows (A2, the A9 junk-precedent);
|
||||||
|
- docstring: one paragraph — the map is keyed by `str(root)`, the pre-walk and the loop use the same tuple (so `files_total` excludes ignored files), and pruning follows automatically.
|
||||||
|
4. `tests/unit/test_importer_ignore.py` (NEW — the spec is the test table; import the three helpers from `app.rag.importer`):
|
||||||
|
- **normalize:** `"/my/files/"`, `"my/files/"`, `"my/files"`, `" my/files "` all → `"my/files"`; `"//"` → `""`; `""` → `""`; `" "` → `""`.
|
||||||
|
- **is_ignored (spec examples verbatim):** prefixes `("my/files",)` ignore `my/files/a.md`, `my/files/sub/b.md`, and `my/files` (a file literally named that); prefixes `("myfile.txt",)` ignore `myfile.txt` but NOT `some/path/myfile.txt` (no mid-path matching — the string does not start with the pattern) and NOT `xmyfile.txt` (no suffix/partial matching either). Also `("files",)` ignores `files/x.md` (a root-level dir — that IS prefix matching) but not `some/files/x.md`.
|
||||||
|
- **raw-prefix A1 edge (documented, owner-confirmed):** `("my/files",)` also ignores `my/files2/x.md`; `("a",)` ignores `ab.md` (raw string startswith — no component boundary).
|
||||||
|
- **empty prefixes:** `is_ignored(anything, ())` → False.
|
||||||
|
- **iter_importable_files:** a tmp_path fixture tree (`root/keep.md`, `root/ignore/secret.md`, `root/ignore/deep/x.yaml`, `root/notes.txt`, `root/myfiles.md`) with `extensions=frozenset({".md", ".txt", ".yaml"})`: default `ignore=()` → all five; `ignore=("ignore",)` → only `keep.md`, `notes.txt`, `myfiles.md`; `ignore=("//ignore/ ")` → same (normalization is the caller's job at this level — this function receives ALREADY-normalized tuples; pin that contract: passing raw `"//ignore/ "` does NOT match, proving normalization happens in `_ignore_for_root`, not here).
|
||||||
|
- **_ignore_for_root:** `{"str(root)": ["/a/", "b//", "", " "]} ` → `("a", "b")`; missing root → `()`; `None` map → `()`.
|
||||||
|
5. `tests/integration/test_importer_ignore.py` (NEW — mirror the fixture-tree + mock-LLM pattern of `tests/integration/test_importer_e2e.py`; use the session-scoped test DB and a `tmp_path` source dir named `IgnoreFix`):
|
||||||
|
- **never indexed:** tree `keep.md` + `ignore/secret.md` + `ignore/notes.txt` + `top.txt`; run `import_sources([root], fake_llm, ignore_by_root={str(root): ["ignore/"]})` → `ImportSummary.files == 2`; `documents` has exactly `keep.md` + `top.txt`; NO `chunks` row for `ignore/secret.md` (not embedded) and its `summary` column is NULL (not summarized); the fake LLM's `chat` call count (summary) excludes it.
|
||||||
|
- **prune (A2):** re-run with `prune=True` and a file previously indexed — pre-import `ignore/secret.md` WITHOUT the map (so it is indexed), then re-run WITH `ignore_by_root={str(root): ["ignore"]}` (no trailing slash — A1 normalization) → `summary.pruned == 1` and the doc row is gone from the DB.
|
||||||
|
- **progress pre-walk excludes ignored:** pass a `progress` hook + the map → `total` == count of non-ignored files only (2 in the first scenario).
|
||||||
|
- **no-map regression:** the same tree, `ignore_by_root=None` → all four files indexed (byte-identical behavior), `summary.files == 4`.
|
||||||
|
- **unlisted source unaffected:** two roots, map keys only the first → the second root imports everything.
|
||||||
|
- Clean up `documents` rows of the fixture source between tests (the house cleanup pattern in `test_importer_e2e.py`).
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- `uv run pytest tests/unit/test_importer_ignore.py tests/integration/test_importer_ignore.py -v` green.
|
||||||
|
- `uv run pytest -q` green UNCHANGED (every existing importer/sync/upload/CLI test passes no map — the default `()`/`None` contract).
|
||||||
|
- Coverage: **>90%** on `app/` (the helpers are exercised by the unit table; the importer branches by the integration suite).
|
||||||
|
- `uv run ruff check . && uv run pyright` clean.
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] The spec's own examples pass as unit tests verbatim (`/my/files/` ≡ `my/files/` ≡ `my/files`; `myfile.txt` matches root-level only, never mid-path).
|
||||||
|
- [ ] An ignored file yields no `documents` row, no `chunks` rows, no embedding call, and no summary call; a previously indexed file matching a new pattern is pruned on the next `prune=True` run.
|
||||||
|
- [ ] `files_total` (progress hook) never counts ignored files.
|
||||||
|
- [ ] `git diff --stat` limited to `app/rag/importer.py` + the two new test modules.
|
||||||
@@ -0,0 +1,75 @@
|
|||||||
|
# Task 03 — API: expose + update ignore lists (GET / POST / PATCH)
|
||||||
|
|
||||||
|
**Phase:** `89_source_ignore_paths` · **Source:** `TODO.md` L3 — "They should be able to type these files and folders into a box on the sources page. Each source should support a list of ignored files and folders."
|
||||||
|
**Story:** n/a (TODO-derived).
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
The admin-only sources API carries the ignore list: `GET` reports it per row, `POST` stores it at create time, and a new `PATCH /api/git-sources/{source_id}` replaces it (A5) — with the fixed-detail 422 validation of the A4 limits. No importer behavior changes here (the pipelines are wired in task 04).
|
||||||
|
|
||||||
|
## Work
|
||||||
|
1. `app/schemas.py` — extend the git-source schemas (house docstrings, cite phase 89):
|
||||||
|
- `GitSourceIn`: `ignore_paths: list[str] | None = Field(default=None)` — optional at create time (absent → `[]`); entries are the RAW box lines (trimmed/normalized by the API layer, not the schema — the 422 details must stay fixed strings, the router's credential-safety discipline applied for consistency).
|
||||||
|
- `GitSourceOut`: `ignore_paths: list[str]` (the stored, normalized list — non-null).
|
||||||
|
- `GitSourceRow`: `ignore_paths: list[str]` (env-fallback rows report `[]`).
|
||||||
|
- NEW `GitSourceIgnoreIn(BaseModel)`: `ignore_paths: list[str]` — the `PATCH` body; the list is REQUIRED (replace semantics, A5 — an absent field is a FastAPI 422 with the model's own detail; keep the A4 422s for the *value* checks).
|
||||||
|
2. `app/api/git_sources.py`:
|
||||||
|
- a module-level validation helper (private, near `_commit_new`), shared by POST and PATCH:
|
||||||
|
```python
|
||||||
|
def _validate_ignore_paths(raw: list[str] | None) -> list[str]:
|
||||||
|
"""Normalize + enforce the A4 limits; fixed details never echo input."""
|
||||||
|
entries = [normalize_ignore_path(e) for e in (raw or [])]
|
||||||
|
if any(not e for e in entries):
|
||||||
|
raise HTTPException(status_code=422, detail="ignore paths must be non-empty")
|
||||||
|
if len(entries) > MAX_IGNORE_PATHS: # = 200 (A4)
|
||||||
|
raise HTTPException(status_code=422, detail="a source has at most 200 ignore paths")
|
||||||
|
if any(len(e) > 500 for e in entries):
|
||||||
|
raise HTTPException(status_code=422, detail="an ignore path exceeds 500 characters")
|
||||||
|
return entries
|
||||||
|
```
|
||||||
|
(Import `normalize_ignore_path` from `app.rag.importer` — task 02 owns the helper; define `MAX_IGNORE_PATHS = 200` and the per-entry cap `500` as module constants with an A4 comment. NOTE the order: empty check FIRST — a whitespace-only entry must 422, not silently drop.)
|
||||||
|
- `list_git_sources` (GET): DB rows → `GitSourceRow(…, ignore_paths=row.ignore_paths or [])`; env-fallback rows → `ignore_paths=[]`.
|
||||||
|
- `_create_git_row` / `_create_local_row` (POST): `row = GitSource(…, ignore_paths=_validate_ignore_paths(payload.ignore_paths))` (both kinds — the stored value is always the normalized list).
|
||||||
|
- `GitSourceOut` responses in `create_git_source` gain `ignore_paths=row.ignore_paths`.
|
||||||
|
- NEW route (place it after the POST, before `/upload` — pathless params keep the `/upload` and `/upload/status` routes matching their own literal prefixes first; FastAPI matches in declaration order, so the `PATCH /{source_id}` declaration must NOT shadow anything — PATCH and POST/GET are different methods, and DELETE `/{source_id}` already coexists, so a plain `@router.patch("/{source_id}", response_model=GitSourceOut)` is safe):
|
||||||
|
```python
|
||||||
|
@router.patch("/{source_id}", response_model=GitSourceOut)
|
||||||
|
def patch_git_source(
|
||||||
|
source_id: uuid.UUID,
|
||||||
|
payload: GitSourceIgnoreIn,
|
||||||
|
db: Session = Depends(get_db), # noqa: B008
|
||||||
|
) -> GitSourceOut:
|
||||||
|
"""Replace one source's ignore list (phase 89, A5).
|
||||||
|
|
||||||
|
404 unknown id. The body list (required) is normalized +
|
||||||
|
A4-validated (fixed 422 details) and REPLACES the row's list
|
||||||
|
wholesale — an empty list clears all. Returns the updated
|
||||||
|
row's public shape (id, url, added_at, ignore_paths).
|
||||||
|
"""
|
||||||
|
row = db.get(GitSource, source_id)
|
||||||
|
if row is None:
|
||||||
|
raise HTTPException(status_code=404, detail="git source not found")
|
||||||
|
row.ignore_paths = _validate_ignore_paths(payload.ignore_paths)
|
||||||
|
db.commit()
|
||||||
|
db.refresh(row)
|
||||||
|
return GitSourceOut(id=row.id, url=row.url, added_at=row.added_at,
|
||||||
|
ignore_paths=row.ignore_paths)
|
||||||
|
```
|
||||||
|
- Router module docstring: add the PATCH route to the "Routes:" paragraph (house style — the existing paragraph already lists every route).
|
||||||
|
3. `tests/integration/test_git_sources_api.py` — extend (follow the existing test module's fixture/auth pattern — admin vs anonymous, DB truncation between tests):
|
||||||
|
- GET: a row created via POST without `ignore_paths` reports `ignore_paths == []`; env-fallback rows (empty table + `BOR_GIT_SOURCES` env) report `[]` per row and `from_env: true`.
|
||||||
|
- POST: `ignore_paths: ["/my/files/", " my/files2 ", "x"]` → 201, stored list `["my/files", "my/files2", "x"]` (normalized round-trip through GET); both kinds (git + local) accept it.
|
||||||
|
- PATCH: 200 replace semantics — set `["a/", "b"]` → GET shows `["a", "b"]`; then PATCH `[]` → GET shows `[]` (clear works); the row's `id`/`url`/`added_at` are unchanged; response shape is `GitSourceOut` incl. the new list.
|
||||||
|
- PATCH 404 on an unknown uuid; anonymous PATCH → 403 (the router dependency).
|
||||||
|
- PATCH 422s (fixed details, asserted as exact strings): `[" "]` → "ignore paths must be non-empty"; 201 entries → "a source has at most 200 ignore paths"; `["a" * 501]` → "an ignore path exceeds 500 characters"; 500-char entry is ACCEPTED (boundary).
|
||||||
|
- regression: every pre-existing test in the module passes UNCHANGED (the new fields are additive on the response models).
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- `uv run pytest tests/integration/test_git_sources_api.py -v` green (new + untouched tests).
|
||||||
|
- `uv run pytest -q` green (no pipeline wiring yet — the lists are stored but not yet consulted by imports).
|
||||||
|
- Coverage: **>90%** on `app/` (every new branch — 404/422/replace — is a tested path).
|
||||||
|
- `uv run ruff check . && uv run pyright` clean.
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] `GET /api/git-sources` reports `ignore_paths` on every row (DB rows: their list; env rows: `[]`).
|
||||||
|
- [ ] `POST` stores the normalized list for both kinds; `PATCH /api/git-sources/{id}` replaces it (including clearing to `[]`), 404/403/422 contracts hold with the exact fixed details above.
|
||||||
|
- [ ] `git diff --stat` limited to `app/schemas.py`, `app/api/git_sources.py`, and the integration test module.
|
||||||
@@ -0,0 +1,57 @@
|
|||||||
|
# Task 04 — Wire per-row ignore lists into all three import entry points
|
||||||
|
|
||||||
|
**Phase:** `89_source_ignore_paths` · **Source:** `TODO.md` L3 — "The user should be able to ignore/exclude specific files and folders when importing from a source… These ignored files/folders should not be embedded and should not be summarized."
|
||||||
|
**Story:** n/a (TODO-derived).
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
Every import entry point consults its source rows' ignore lists: the in-app Sync button, the archive-upload background scan, and the CLI. Rows with empty lists and manual `--source` dirs behave exactly as before.
|
||||||
|
|
||||||
|
## Work
|
||||||
|
1. `app/api/sync.py::_run_sync` — while the per-row loop builds `sources` (the `for row in rows:` block that appends `clone_or_pull(…)` for git rows and the re-verified dir for local rows), build the map in the SAME loop, per the phase overview's Design/Callers:
|
||||||
|
```python
|
||||||
|
ignore_by_root: dict[str, list[str]] = {}
|
||||||
|
…
|
||||||
|
for row in rows:
|
||||||
|
if row.kind == "git":
|
||||||
|
root = clone_or_pull(row.url, sources_root / repo_name(row.url))
|
||||||
|
else:
|
||||||
|
root = Path(row.path or row.url).expanduser()
|
||||||
|
if not root.is_dir():
|
||||||
|
raise GitSyncError(f"local source missing: {root}")
|
||||||
|
sources.append(root)
|
||||||
|
# phase 89: the row's ignore list, keyed by the SAME root string
|
||||||
|
# the importer sees; two rows sharing a root string get the
|
||||||
|
# union (extend, not replace) — the sibling/repo-name edge.
|
||||||
|
if row.ignore_paths:
|
||||||
|
ignore_by_root.setdefault(str(root), []).extend(row.ignore_paths)
|
||||||
|
```
|
||||||
|
(Refactor the existing loop body to the `root = …` shape above — behavior byte-identical; the existing re-verify-`.is_dir()` raise and its message are kept EXACTLY.) Then: `summary = await import_sources(sources, llm, prune=True, progress=_hook, ignore_by_root=ignore_by_root)`. Update the module docstring's pipeline step 4 with a clause: "honoring each row's `ignore_paths` (phase 89)".
|
||||||
|
2. `app/api/git_sources.py::_run_upload` — step 6 already opens a short-lived session and upserts the row by path; CAPTURE the row object on both branches (existing row from the `db.scalar(…)`; the newly created `GitSource` instance before `db.close()`) and keep it out of the session (the attributes are plain values — copy `row.ignore_paths or []` into a local `ignore_paths: list[str]`). Step 8 becomes:
|
||||||
|
```python
|
||||||
|
summary = await import_sources(
|
||||||
|
[final_dir], llm, prune=True, progress=_hook,
|
||||||
|
ignore_by_root={str(final_dir): ignore_paths},
|
||||||
|
)
|
||||||
|
```
|
||||||
|
Update the route's numbered docstring (steps 6/8) with one clause each (phase 89: a re-upload of an existing source honors the ignore list the owner already saved).
|
||||||
|
3. `scripts/import_docs.py` — `_resolve_sources` returns `tuple[list[Path], dict[str, list[str]]]`:
|
||||||
|
- manual `--source` branch → `([expanded paths], {})` (manual dirs have no row — no ignore applies);
|
||||||
|
- rows branch: same `root = …` per-row shape as `_run_sync`, building `ignore_by_root` with the same `setdefault(…).extend(…)` collision rule; return `(sources, ignore_by_root)`;
|
||||||
|
- legacy `DEFAULT_SOURCES` fallback → `([…], {})`;
|
||||||
|
- module + `_resolve_sources` docstrings: one sentence each (phase 89 — the map is keyed by the resolved root string; manual/fallback paths import with no ignore).
|
||||||
|
- `main`: `sources, ignore_by_root = _resolve_sources(args.source, settings)` and `import_sources(sources, llm, prune=args.prune, limit=args.limit, ignore_by_root=ignore_by_root)` (inside the `_run` closure — the variable is captured like `args` already is).
|
||||||
|
4. `tests/integration/test_sync_api.py` — extend (follow the module's existing local-source + mock-LLM fixture pattern; check how it seeds a `kind='local'` row and drives `POST /api/sync` + status polling):
|
||||||
|
- a local fixture dir with `keep.md` + `ignore/secret.md`; seed a `kind='local'` row with `ignore_paths=["ignore/"]`; run the sync to `success`; `documents` contains `keep.md`, NOT `ignore/secret.md`; the success `detail` counts are consistent (`files` excludes the ignored file).
|
||||||
|
- regression: the module's existing tests pass UNCHANGED (rows with `[]` lists → byte-identical sync).
|
||||||
|
5. `tests/integration/test_import_docs_git.py` (or `test_importer_e2e.py` if the CLI driver helper lives there — check which module already drives `import_docs.main` and extend THAT) — a rows-branch run with a `kind='local'` row carrying `ignore_paths` → the CLI import skips the ignored file (summary/files assertions); a `--source` manual run is unaffected (no map).
|
||||||
|
6. `tests/integration/test_git_sources_upload.py` — extend with the re-upload-respects-lists case: upload an archive (the module's existing fixture pattern) whose unpacked tree includes an ignored path; after the first scan, set the row's `ignore_paths` (PATCH or direct DB update — use the API), re-upload a same-name archive containing the file again → the background run lands `success` with the file NOT indexed; a second assertion: an upload of a NEW source name with no list imports everything (regression).
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- `uv run pytest tests/integration/test_sync_api.py tests/integration/test_import_docs_git.py tests/integration/test_git_sources_upload.py -v` green (new + existing).
|
||||||
|
- `uv run pytest -q` green; coverage **>90%** on `app/` (the new branches — map build, collision extend, upload capture — are covered by the integration tests; the CLI path by its test).
|
||||||
|
- `uv run ruff check . && uv run pyright` clean.
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] `POST /api/sync` (button), the upload background scan, and `python -m scripts.import_docs` all skip the listed prefixes for their rows; nothing changes for empty lists or manual dirs.
|
||||||
|
- [ ] A re-uploaded existing source honors the ignore list saved on its row.
|
||||||
|
- [ ] `git diff --stat` limited to `app/api/sync.py`, `app/api/git_sources.py`, `scripts/import_docs.py`, and the extended integration modules.
|
||||||
@@ -0,0 +1,77 @@
|
|||||||
|
# Task 05 — The per-row "Ignore paths" box on the Sources page
|
||||||
|
|
||||||
|
**Phase:** `89_source_ignore_paths` · **Source:** `TODO.md` L3 — "They should be able to type these files and folders into a box on the sources page."
|
||||||
|
**Story:** n/a (TODO-derived).
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
A per-row "Ignore paths" control on the Sources page (`/git-sources.html`): a page-local editor dialog (the phase-69 `#remove-confirm-dialog` pattern) with a one-path-per-line textarea, the §7.4 never-stale save lifecycle, and full a11y (visible label, focus management, Escape, role=alert errors) — plus a `N ignored` count tag on rows that have a list.
|
||||||
|
|
||||||
|
## Work
|
||||||
|
1. `frontend/index.html` — inside `#view-git-sources` → `#git-sources-content`, directly AFTER the `#remove-confirm-dialog` block (the markup above — same placement logic), add the editor dialog (house comment header citing phase 89; ids/classes are the ones the JS + CSS + unit pins below key on):
|
||||||
|
```html
|
||||||
|
<!-- Phase 89: the per-source ignore-paths editor — the EXACT
|
||||||
|
#remove-confirm-dialog pattern (phase 69): page-local
|
||||||
|
alertdialog, dim backdrop, focus on Cancel (the safe default),
|
||||||
|
Escape/backdrop close as CANCEL, focus returns to the row's
|
||||||
|
trigger button. The box itself: one path per line (prefix
|
||||||
|
semantics — the phase-89 A1 rule, stated in plain words in
|
||||||
|
the helper line); Save runs the §7.4 never-stale lifecycle
|
||||||
|
(git-sources.js: "Saving…" while the PATCH is out). -->
|
||||||
|
<div class="ignore-editor" id="ignore-editor-dialog" role="alertdialog"
|
||||||
|
aria-modal="true" aria-labelledby="ignore-editor-title"
|
||||||
|
aria-describedby="ignore-editor-copy" hidden>
|
||||||
|
<div class="ignore-editor-backdrop" aria-hidden="true"></div>
|
||||||
|
<div class="ignore-editor-panel">
|
||||||
|
<h2 class="ignore-editor-title" id="ignore-editor-title">Ignored files and folders</h2>
|
||||||
|
<code class="ignore-editor-source" id="ignore-editor-source"></code>
|
||||||
|
<p class="ignore-editor-copy" id="ignore-editor-copy">
|
||||||
|
One path per line. A file is ignored when its path in the
|
||||||
|
source starts with any of these — <code>/my/files/</code>,
|
||||||
|
<code>my/files/</code> and <code>my/files</code> mean the same
|
||||||
|
thing. No wildcards, no matching in the middle of a path.
|
||||||
|
</p>
|
||||||
|
<label class="ignore-editor-label" for="ignore-editor-textarea">Ignored paths</label>
|
||||||
|
<textarea class="ignore-editor-textarea" id="ignore-editor-textarea"
|
||||||
|
rows="6" spellcheck="false"
|
||||||
|
placeholder="my/files/"></textarea>
|
||||||
|
<p class="ignore-editor-error" id="ignore-editor-error" role="alert" hidden></p>
|
||||||
|
<div class="ignore-editor-actions">
|
||||||
|
<button type="button" class="ignore-editor-btn ignore-editor-cancel"
|
||||||
|
id="ignore-editor-cancel">Cancel</button>
|
||||||
|
<button type="button" class="ignore-editor-btn ignore-editor-save"
|
||||||
|
id="ignore-editor-save">Save</button>
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
```
|
||||||
|
2. `frontend/assets/git-sources.js` — the wiring (every lookup scoped to the view `root` like the rest of the module; the module header docstring gains a phase-89 bullet in the house style):
|
||||||
|
- module-scope refs next to the remove-dialog refs: `ignoreDialog`, `ignoreSourceEl` (`#ignore-editor-source`), `ignoreTextarea` (`#ignore-editor-textarea`), `ignoreErrorEl` (`#ignore-editor-error`), `ignoreSaveBtn` (`#ignore-editor-save`), `ignoreCancelBtn` (`#ignore-editor-cancel`), `ignoreBackdrop` (`.ignore-editor-backdrop`); plus `let ignoreTarget = null` (the row object) and `let ignoreTriggerBtn = null` (for focus return).
|
||||||
|
- `makeRow(s)` — in the `s.id` branch of the actions cell, BEFORE the Remove button: an "Ignore paths" button (`type="button"`, class `git-source-ignore`, `aria-label` = `` `Edit ignored paths for ${kindLabel} source: ${value}` ``, built with `textContent`-safe content — the label is the only place `value` appears; follow the Remove button's `innerHTML` icon+text idiom ONLY if you reuse a static icon constant — never inject `value` via innerHTML). Its click → `openIgnoreEditor(s, btn)`. When `s.id && s.ignore_paths.length > 0`: append a count tag to the LOCATION cell (`urlTd`), after the `<code>`: `<span class="git-source-ignore-count">N ignored</span>` (text via textContent; N = `s.ignore_paths.length` — text + a distinct background, never color alone, WCAG).
|
||||||
|
- `openIgnoreEditor(s, triggerBtn)`: set `ignoreTarget`/`ignoreTriggerBtn`; `ignoreSourceEl.textContent = value` (the same value expression as makeRow); `ignoreTextarea.value = (s.ignore_paths || []).join("\n")`; clear the error line (`hidden` + empty text); show the dialog (`hidden = false`); focus `ignoreCancelBtn` (the safe default — the remove-dialog precedent); bind Escape via a keydown listener on the dialog (see the remove dialog's `onRemoveDialogKeydown` shape: Escape → `closeIgnoreEditor()`, return focus to the trigger).
|
||||||
|
- `closeIgnoreEditor()`: hide the dialog, reset the textarea value + error, `ignoreTarget = ignoreTriggerBtn = null`, return focus to the trigger button.
|
||||||
|
- Cancel button + backdrop click → `closeIgnoreEditor()` (the remove-dialog wiring shape).
|
||||||
|
- `saveIgnorePaths()` — the §7.4 lifecycle:
|
||||||
|
- parse: `ignoreTextarea.value.split("\n").map(l => l.trim()).filter(Boolean)` (a blank line is a separator, not an entry — the server still rejects empty entries defensively);
|
||||||
|
- disable Save + Cancel, relabel Save "Saving…";
|
||||||
|
- `fetch(`/api/git-sources/${ignoreTarget.id}`, {method: "PATCH", headers: {"Content-Type": "application/json"}, body: JSON.stringify({ignore_paths: lines})})` with the module's existing cookie/auth behavior (plain fetch — same as the add/remove calls);
|
||||||
|
- 200 → `closeIgnoreEditor()` (focus return included), `await loadSources()` (the `N ignored` tag lands), `announce(`Ignored paths updated for ${value}`)` (the existing `#git-sources-announcer`);
|
||||||
|
- non-2xx → clear Save/Cancel disabled states, restore the "Save" label, show the server detail in `#ignore-editor-error` (via the existing `apiDetail(r, fallback)` helper — 422 shape-aware like the tuning forms), KEEP the textarea content (the instruction survives);
|
||||||
|
- network throw → the same error-line path (the module's existing try/catch idiom for the other requests).
|
||||||
|
- env-fallback rows (`s.id` null): NO ignore button (A3) — the existing "from .env" tag stays as-is.
|
||||||
|
3. `frontend/assets/styles.css` — the ignore-editor rules (place next to the `.remove-confirm` rules; house comment style citing phase 89): `.ignore-editor` mirrors `.remove-confirm` (fixed overlay, dim backdrop `.ignore-editor-backdrop` = the `.remove-confirm-backdrop` treatment, centered panel `.ignore-editor-panel` = `.remove-confirm-panel` sizing); `.ignore-editor-source` = the `.remove-confirm-source` mono treatment; `.ignore-editor-label` a visible block label (WCAG — never `aria-label`-only); `.ignore-editor-textarea` — full panel width, `min-height: ~9rem`, `font-family: mono stack` (the existing code/mono variable), theme-consistent border + focus-visible ring (3px, the house focus convention); `.ignore-editor-error` = `.remove-confirm-error` treatment; `.ignore-editor-btn` pair = `.remove-confirm-btn` (Cancel secondary / Save brand — 44px min targets, the ≥44px rule); the row button `.git-source-ignore` = the `.git-source-remove` idiom (same size/spacing, a neutral/secondary fill so it reads distinct from the destructive Remove — text label included, never icon-only); `.git-source-ignore-count` — a small inline tag next to the location `<code>` (text + distinct background, 4.5:1 contrast on both theme surfaces).
|
||||||
|
4. `tests/unit/test_source_ignore_paths.py` (NEW — house source-level pattern: read the assets as text via `Path(__file__).parents[2] / "frontend" / "assets" / …`; module docstring cites TODO.md L3 + phase 89):
|
||||||
|
- `index.html`: `#ignore-editor-dialog` exists with `role="alertdialog"`, `aria-modal="true"`, `aria-labelledby="ignore-editor-title"`; `#ignore-editor-textarea` has a visible `<label … for="ignore-editor-textarea">` (the `ignore-editor-label` class); the error line carries `role="alert"`.
|
||||||
|
- `git-sources.js`: the PATCH call exists (`method: "PATCH"` + `` `/api/git-sources/${` ``); the §7.4 lifecycle markers exist (the "Saving…" relabel + disabling of the save button — assert the string `"Saving…"` appears in the save function's region); the parse chain `split("\n")` + trim + `filter(Boolean)`; `ignore_paths` round-trip markers (the GET rows render `(s.ignore_paths || [])` and the count tag class `git-source-ignore-count`); the env-row guard — the ignore button is created only in the `s.id` branch (assert the `openIgnoreEditor` binding sits after the `if (s.id)` — a string-position check, the phase-88 pin idiom).
|
||||||
|
- `styles.css`: the `.ignore-editor-textarea` rule exists (mono + width), and `git-source-ignore` / `git-source-ignore-count` rules exist.
|
||||||
|
- guard pin (the phase-46/76 no-collision contract): no id in the new markup duplicates an existing id — assert each `id="ignore-editor-…"` appears exactly once in `index.html`.
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- `uv run pytest tests/unit/test_source_ignore_paths.py -v` green; full `uv run pytest tests/unit/ -q` green UNCHANGED (no other suite reads `git-sources.js` — verify with `rg "git-sources" tests/unit` first; the `header.js`/`styles.css` pins must survive the additive CSS).
|
||||||
|
- Coverage: **>90%** on `app/` unaffected (frontend-only task).
|
||||||
|
- `uv run ruff check . && uv run pyright` clean.
|
||||||
|
- Manual smoke (the executor's own eyes, headless is fine): boot the app, add a local source, open the box, type `my/files/`, save → the tag appears; refresh the page → the box prefills.
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] Every stored row (git or local) shows "Ignore paths"; env-fallback rows do not. Rows with a list show `N ignored`.
|
||||||
|
- [ ] The dialog: visible label, mono textarea prefilled from the row, Escape/Cancel/backdrop close with focus returned to the trigger, Save runs §7.4 ("Saving…" while the PATCH is out; error line keeps the content on 422).
|
||||||
|
- [ ] `git diff --stat` limited to `frontend/index.html`, `frontend/assets/git-sources.js`, `frontend/assets/styles.css`, and the new unit test module.
|
||||||
@@ -0,0 +1,75 @@
|
|||||||
|
# Task 06 — E2E story suite, full gate, and the atomic commit
|
||||||
|
|
||||||
|
**Phase:** `89_source_ignore_paths` · **Source:** `TODO.md` L3 (the whole item — this task is its end-to-end proof).
|
||||||
|
**Story:** n/a (TODO-derived).
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
Prove the feature end-to-end through the real page + real API + real sync pipeline (mock LLM, no git, no network — the `test_git_sources_admin.py` policy): the box on the Sources page sets a list, the sync honors it (nothing embedded/summarized), previously indexed files are pruned (A2), and the spec's no-mid-path rule holds. Then run the complete gate and land ONE atomic commit.
|
||||||
|
|
||||||
|
## Work
|
||||||
|
1. `tests/e2e/test_source_ignore_paths.py` (NEW story suite — conftest app-server + mock-LLM + admin-login pattern from `tests/e2e/test_git_sources_admin.py` / `tests/e2e/auth_helpers.py`; module docstring: story source `TODO.md` L3, isolation run command, the no-git/no-network policy, the contract-under-test list, the test→contract mapping):
|
||||||
|
- **Fixtures:** a module-scoped source dir (`tmp_path_factory` — same reasoning as `test_sync_button.py`'s module-scoped fixture note) with this tree: `keep.md` (a `# Keep` doc with a sentinel word), `notes.yaml` (non-markdown → exercises the summary path), `myfile.md` (root level), `sub/myfile.md`, `ignore/secret.md`, `ignore/deep/x.txt`. A helper that (re)seeds the DB row: truncate `git_sources`/`documents` between tests (the house `db_ready` truncate pattern, the `test_sync_button.py` module-env note shows the DSN), then `POST /api/git-sources {"kind": "local", "path": <dir>}` (201) and capture the row id. A `run_sync()` helper: `POST /api/sync` → poll `GET /api/sync/status` to a terminal state (the `test_sync_button.py` polling idiom, ~2 s ticks, 60 s budget).
|
||||||
|
- `test_anonymous_gate_and_403s` — anonymous visit to `/git-sources.html`: the sign-in gate visible, the manager hidden, NO `/api/git-sources` request on load (route listener), and the page's request client gets 403 on `GET/POST /api/git-sources` AND `PATCH /api/git-sources/<uuid>` (the `test_git_sources_admin.py` anonymous pin).
|
||||||
|
- `test_ignore_box_excludes_from_import` — admin: seed the row (no list); open the box on the row (the "Ignore paths" button), type `ignore/` (single line), Save → wait for 200 (the `N ignored` tag: "1 ignored" appears on the row); `run_sync()` → `success`; `GET /api/docs` (the RAG catalog's data source — or read the RAG table after a nav to `/sources.html`, whichever the existing suites use) contains `keep.md`, `notes.yaml`, `myfile.md`, `sub/myfile.md` and contains NO path starting with `ignore/`; the sync `detail` `files` count == 5 (the two `ignore/` files excluded from the walk).
|
||||||
|
- `test_prefix_rule_no_mid_path` — admin: fresh seed; set the list to `myfile.md` via the box; `run_sync()` → the catalog contains `sub/myfile.md` (NOT ignored — no mid-path matching, the spec's own example) and NOT `myfile.md`; the `ignore/` files ARE indexed this time (no list matches them — regression guard that the box replaced, not appended, the previous test's list).
|
||||||
|
- `test_newly_ignored_file_is_pruned` (A2) — admin: fresh seed; `run_sync()` once with NO list (all six files indexed — assert 6 in the catalog); then set the box to `ignore/`, Save, `run_sync()` again → `success`, `detail.pruned == 2`, the catalog has the four remaining paths and no `ignore/` path.
|
||||||
|
- `test_editor_a11y_and_error_path` — admin: open the box → the dialog `role="alertdialog"` is visible, the visible label ("Ignored paths") is present, focus is on Cancel; Escape closes and focus returns to the trigger button (the `aria-label` matches the row); re-open, type an entry >500 chars (501 a's), Save → the `role="alert"` error line shows `an ignore path exceeds 500 characters`, the textarea content is KEPT, the save button is re-enabled with its "Save" label; then clear it, type `keep.md`, Save → "1 ignored" tag (the happy path heals the error state).
|
||||||
|
- `test_env_fallback_rows_have_no_box` — admin: truncate `git_sources` (the app booted with a `BOR_GIT_SOURCES` env URL, the `test_git_sources_admin.py` module-env pattern) → the env row renders with the "from .env" tag and NO "Ignore paths" button; `#git-sources-env-note` visible (A3).
|
||||||
|
2. **Gate (all must pass; any failure → fix within this task, re-run):**
|
||||||
|
- `uv run pytest` (unit + integration, green).
|
||||||
|
- `uv run pytest --cov=app --cov-report=term-missing` — **>90%** on `app/` (the new importer + API logic is covered by tasks 02–04's suites; verify the TOTAL line, not just the new files).
|
||||||
|
- `uv run pytest tests/e2e/test_source_ignore_paths.py -v --no-cov` (the NEW story — in isolation, DB up: `podman compose up -d db`).
|
||||||
|
- `uv run pytest tests/e2e/test_git_sources_admin.py -v --no-cov` (the existing Sources-page story — in isolation).
|
||||||
|
- `uv run pytest tests/e2e/test_archive_upload_sources.py -v --no-cov` (the upload pipeline — in isolation).
|
||||||
|
- `uv run pytest tests/e2e/test_sync_button.py -v --no-cov` (the sync button story — in isolation).
|
||||||
|
- `uv run pytest tests/e2e/test_smoke.py -v --no-cov` (in isolation).
|
||||||
|
- `uv run ruff check . && uv run pyright` clean.
|
||||||
|
3. **Diff hygiene:** `git diff --stat` must be limited to: `app/models.py`, `app/schemas.py`, `app/api/git_sources.py`, `app/api/sync.py`, `app/rag/importer.py`, `alembic/versions/0013_git_source_ignore_paths.py` (new), `scripts/import_docs.py`, `frontend/index.html`, `frontend/assets/git-sources.js`, `frontend/assets/styles.css`, `tests/unit/test_ignore_paths_column.py` (or the extended models module), `tests/unit/test_importer_ignore.py` (new), `tests/unit/test_source_ignore_paths.py` (new), `tests/integration/test_importer_ignore.py` (new), the extended integration modules (`test_git_sources_api.py`, `test_sync_api.py`, `test_import_docs_git.py`, `test_git_sources_upload.py`), `tests/e2e/test_source_ignore_paths.py` (new), `TODO.md` (cleared — Phase 4 of the conversion), and the `89_source_ignore_paths/` phase files. Anything else → stop and fix.
|
||||||
|
4. **Commit (one atomic, Conventional Commits, `--no-gpg-sign` per AGENTS.md rule 8):**
|
||||||
|
```
|
||||||
|
feat(sources): per-source ignore paths — prefix-excluded files are never embedded, summarized, or re-indexed
|
||||||
|
|
||||||
|
TODO.md L3: the owner can type ignored files/folders into a box on
|
||||||
|
the Sources page; each source stores a list of source-relative path
|
||||||
|
prefixes, and matching is a pure prefix (no mid-path matching, no
|
||||||
|
globs — "/my/files/" ≡ "my/files/"; "myfile.txt" never matches
|
||||||
|
"some/path/myfile.txt").
|
||||||
|
|
||||||
|
- git_sources.ignore_paths: JSONB NOT NULL, server default '[]'
|
||||||
|
(migration 0013, reversible);
|
||||||
|
- importer: normalize_ignore_path / is_ignored / iter_importable_files
|
||||||
|
ignore filter / import_sources ignore_by_root (walk AND the
|
||||||
|
progress pre-walk; previously indexed files that newly match are
|
||||||
|
pruned on the next prune=True sync, the A9 junk precedent);
|
||||||
|
- API: GET/POST carry the list; new admin-only
|
||||||
|
PATCH /api/git-sources/{id} (replace semantics; fixed 422 details
|
||||||
|
for >200 entries, empty entry, >500-char entry);
|
||||||
|
- pipelines: _run_sync, the _run_upload background scan (a re-upload
|
||||||
|
honors the saved list), and scripts/import_docs.py all pass the
|
||||||
|
per-row lists;
|
||||||
|
- Sources page: per-row "Ignore paths" editor (the phase-69
|
||||||
|
dialog pattern, one path per line, §7.4 save lifecycle, a11y) +
|
||||||
|
the "N ignored" row tag; env-fallback rows have no box.
|
||||||
|
|
||||||
|
Unit: tests/unit/test_ignore_paths_column.py,
|
||||||
|
tests/unit/test_importer_ignore.py,
|
||||||
|
tests/unit/test_source_ignore_paths.py.
|
||||||
|
Integration: tests/integration/test_importer_ignore.py + extended
|
||||||
|
git-sources/sync/upload/CLI suites.
|
||||||
|
E2E story: tests/e2e/test_source_ignore_paths.py (box → sync
|
||||||
|
exclusion, no-mid-path rule, prune of a newly ignored file, a11y +
|
||||||
|
422 path, env rows, anonymous gate).
|
||||||
|
Phase 89: .agents/phases/todo/89_source_ignore_paths/.
|
||||||
|
```
|
||||||
|
Stage ONLY the step-3 file set (the phase directory rides with the commit, AGENTS.md rule 8; `TODO.md` is cleared to the bare `# TODO` title as part of this commit — the conversion's Phase 4).
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- The full gate of step 2 IS this task's test contract; coverage **>90%** on `app/`.
|
||||||
|
- This task adds the E2E file only — everything else verifies and commits.
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] All five E2E tests pass in isolation (DB up); the four regression E2E suites pass in isolation; unit + integration green; TOTAL coverage >90%; ruff + pyright clean.
|
||||||
|
- [ ] `git diff --stat` (pre-commit) matches the step-3 file set exactly.
|
||||||
|
- [ ] One atomic `--no-gpg-sign` commit landed (message per step 4); `git log -1 --stat` confirms the file set.
|
||||||
|
- [ ] `TODO.md` contains exactly `# TODO` (cleared, never deleted).
|
||||||
|
- [ ] The phase directory is under `.agents/phases/todo/89_source_ignore_paths/` (the pipeline gate moves it to `complete/` after validate.sh passes).
|
||||||
Reference in New Issue
Block a user