Files
roboco/docs/rag/workflows/qa-review.md
T
4fb0059556 fix: journals/learnings never reached the RAG corpus + git-readonly slug 404s (#339)
* fix(rag): per-index chunk floors — journals and learnings were never indexed

The global 200-char garbage floor (sized for code/doc chunks) discarded
every templated journal note and most distilled org-memory lessons,
silently: ingest returned success with zero chunks, so agent journals
and learnings were never retrievable via RAG. IndexConfig now carries a
per-type min_chunk_length (journals 40, learnings 80, others unchanged).

* fix(mcp): git-readonly tools default project_slug from the container env

Agents 404ed /api/git/status with 'Project not found: roboco' — the
tools made the LLM supply the slug and six doc examples taught a slug
that matches no registered project. The tools now fall back to the
ROBOCO_PROJECT_SLUG the orchestrator already injects, and the stale
examples are corrected.

* feat(rag): startup backfill re-ingests zero-chunk journals and learnings

Before the per-index chunk-floor fix, ingest() returned success with
chunk_count=0 for undersized content: every historical journal entry and
distilled learning below the (then-global) 200-char floor was durably
recorded in journal_entries but silently never got a chunks_journals /
chunks_learnings row, and no exception meant the existing dead-letter
(rag_index_failures) never saw it either.

Extends the startup reconcile (roboco/api/app.py _reconcile_rag_indexes)
with a new pass: backfill_unindexed_journals (roboco/services/
rag_index_failures.py) queries journal_entries for rows missing from each
vector table and re-ingests them through the same live code paths
(_reindex_journal_entry / record_learning).

Journals and learnings are backfilled independently since a LEARNING entry
can clear the (lower) JOURNALS floor while still failing the (higher)
LEARNINGS floor — a learning's doc_source is a content hash, not the entry
id, so presence there is checked by hashing each candidate the same way
LearningsIndexPlugin.record_learning does and batch-querying chunks_learnings
for those exact sources.

Bounded to 200 rows per pass per boot (converges over restarts on a larger
backlog) and best-effort per row (one failure never aborts the pass). Rows
still under the current floor are excluded by a length filter in the SELECT
so they are never retried forever, and private entries are excluded from
the JOURNALS pass exactly like the live indexing path.

* test(rag): scope backfill assertions to their own rows

---------

Co-authored-by: Renn F <rennf93@users.noreply.github.com>
2026-07-08 16:03:00 +02:00

114 lines
3.6 KiB
Markdown

# QA Review Workflow
## Preconditions
- Task is in `awaiting_qa` status
- The developer's PR is open (the choreographer opened it during their `open_pr(task_id)` call)
- You are not the original developer of the task (self-review guard)
## Steps
```python
# 1. Pick up an awaiting-QA task
give_me_work()
# 2. Claim it for review (auto-checks-out the dev's branch in your
# workspace; auto-records original_developer for the self-review
# guard at pass/fail time)
claim_review(task_id="<task>")
# 3. Inspect the diff (project_slug is optional — omit it and your
# own project is used)
roboco_git_diff()
roboco_git_log(branch="<dev's branch>")
# 4. Run the relevant suite
# Backend: uv run pytest && uv run ruff check . && uv run mypy roboco/
# Frontend: pnpm test && pnpm lint && pnpm typecheck
# 5. Capture evidence (survives compaction; PMs can audit later)
note(text="Verified AC #1 (429 on 101st req), #2 (TTL match), #3 "
"(boundary tests). pytest 1635 passed; ruff clean; mypy clean.",
scope="evidence",
task_id="<task>")
```
There is no `roboco_task_claim / _start / _qa_pass / _qa_fail` and no `roboco_git_checkout`. The verbs above (`claim_review`, `pass`, `fail`) are the actual surface; branch checkout is a side-effect of `claim_review`.
You always claim the review yourself — the dispatcher spawns you against an `awaiting_qa` task without pre-claiming it. `claim_review` records your claim but keeps the status at `awaiting_qa` (there is no `claimed` detour), so `pass`/`fail` find the status they demand.
## Review Checklist
Before deciding:
- [ ] Read the dev's notes and journal entries on the task
- [ ] Walk every acceptance criterion against the diff
- [ ] Tests pass on the dev's branch
- [ ] Lint / typecheck clean
- [ ] No layer-separation regressions (routes/ vs services/ etc.)
- [ ] No silenced rules (`# noqa`, `# type: ignore`, `# pragma: no cover`)
- [ ] Code matches project standards in CLAUDE.md
## Passing QA
```python
pass(
task_id="<task>",
notes=(
"All 3 acceptance criteria verified against the diff. "
"pytest 1635 passed; ruff and mypy clean. "
"PR #123."
),
)
```
Result:
- Task advances to `awaiting_documentation`
- Documenter and the original dev work in parallel from here
- The PR stays open; it will be merged later by the Cell PM via `complete(task_id, ...)`
## Failing QA
```python
fail(
task_id="<task>",
issues=[
"Bug: 100th request also returns 429 — boundary off-by-one.",
"Missing: tests for Redis-down failover path; AC #3 unmet.",
],
)
```
Result:
- Task returns to `needs_revision`
- Re-assigned to the original developer (recorded at submit-for-qa time)
- Developer receives a notification
## Reflect (recommended)
After pass or fail, journal the review for future QA agents to learn from:
```python
note(
text=(
"Reviewed task <id>. Pattern: rate-limiter boundary tests "
"should always assert the off-by-one — caught it in this "
"review and last week's. Worth a regression checklist item."
),
scope="reflect",
task_id="<task>",
)
```
## Self-Review Prevention
The system blocks QA from reviewing their own dev work. The original developer is recorded in `quick_context` at submit-for-qa time. If `qa_agent_id == original_developer_id`, **all** QA actions on the task return `not_authorized`:
- `claim_review` — FORBIDDEN
- `pass` — FORBIDDEN (defence-in-depth even if claim somehow succeeded)
- `fail` — FORBIDDEN (same)
Enforced at the gateway layer in `roboco/services/gateway/choreographer/_impl.py`.