Skip to content

feat: resolve Review items via the API (PATCH + bulk) + a Refresh button#492

Merged
nashsu merged 1 commit into
nashsu:mainfrom
AndrewDongminYoo:feat/review-resolve-api
Jun 28, 2026
Merged

feat: resolve Review items via the API (PATCH + bulk) + a Refresh button#492
nashsu merged 1 commit into
nashsu:mainfrom
AndrewDongminYoo:feat/review-resolve-api

Conversation

@AndrewDongminYoo

@AndrewDongminYoo AndrewDongminYoo commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Problem

The Review tab is the one stateful surface with no way to act on it from outside the WebView, and no way to re-sync once loaded:

  • The API could read review items (GET .../reviews) but not resolve them.
  • Unlike the Lint tab (which has a re-run), the Review tab never re-reads review.json after the initial load — an external change stays invisible until the project is reopened.

This matters for headless / scripted wiki maintenance: you can export the review queue but can't close items programmatically, and you can't see changes made outside the running app.

What this adds

1. Single-item update — PATCH /api/v1/projects/{id}/reviews/{reviewId}

PATCH .../reviews/review-3
{ "resolved": true, "action": "Skip" }

Partial update of one item. resolved defaults to true (the common case); pass false to reopen. action is an optional label. Mark-only — it deliberately does not replicate the WebView's resolve side effects (page creation, deep research, save-to-wiki). 404 on unknown id.

PATCH (rather than a POST .../resolve action path) is the REST-correct shape for a resource state change. It needed the server's method gate + CORS Allow-Methods opened to PATCH — verified tiny_http 0.12 parses PATCH into Method::Patch so the route actually fires.

2. Bulk resolve — POST /api/v1/projects/{id}/reviews/resolve

POST .../reviews/resolve
{ "ids": ["review-1", "review-2"], "action": "Skip" }
→ 200 { "resolved": [...], "notFound": [...], "count": N }

Partial success is the norm for bulk, so this returns 200 with resolved / notFound rather than 404. Reads review.json once and writes once (no per-id race window from looping the single-item path).

Both write paths edit the raw review.json array rather than the sanitizing reader used by GET, so fields the sanitizer drops (e.g. internalSecret) survive write-back. Covered by tests.

3. Refresh button on the Review tab

Reloads review.json from disk. The missing counterpart to lint's re-run, and the intended way an external resolve surfaces: API writes disk → user hits Refresh → store reloads. No bespoke event channel for v1.

Tests

Rust unit tests: PATCH resolve + unsanitized-field preservation, reopen via resolved:false, unknown id, missing file; bulk resolve + notFound reporting, missing file. (Also hardened the shared test_project_dir() helper with an atomic sequence — nanosecond-only temp dirs collided under parallel cargo test.)

Docs / i18n

Endpoints list in Settings → API Server, README.md, and en/zh locale notes updated. Other locale READMEs left to the maintainer.

Possible follow-ups (not in this PR)

Live auto-reload via a Rust app.emit → WebView listener if manual refresh proves insufficient; an mcp-server bulk-resolve tool mirroring the existing llm_wiki_reviews tool.

…ew tab

The Review tab is the one stateful surface with no way to act on it from
outside the WebView and no way to re-sync once loaded:

- The API could read review items (GET .../reviews) but not resolve them.
- Unlike the Lint tab (which has a re-run), the Review tab never re-read
  review.json after the initial load, so an external change stayed
  invisible until the project was reopened.

What this adds:

- PATCH /api/v1/projects/{id}/reviews/{reviewId} — partial update of one
  item: body { resolved?, action? } (resolved defaults to true; pass
  false to reopen). 404 on unknown id. Mark-only — it does not replicate
  the WebView's resolve side effects (page creation, deep research).
- POST /api/v1/projects/{id}/reviews/resolve — bulk resolve: body
  { ids, action? }. Partial success is normal, so returns 200 with
  { resolved, notFound, count }. Reads review.json once and writes once.
- Both edit the RAW review.json array, preserving unsanitized fields on
  write-back (verified by tests).
- PATCH is allowed through the method gate + CORS (tiny_http 0.12 parses
  it into Method::Patch).
- Review tab gains a Refresh button (reload review.json from disk) — the
  counterpart to lint's re-run and the way an external resolve surfaces.
- Endpoints list in Settings → API Server, README, and en/zh notes
  updated. Rust unit tests cover PATCH + bulk paths.

Possible follow-ups (not here): live auto-reload via app.emit → WebView
listener; an mcp-server bulk-resolve tool mirroring llm_wiki_reviews.
@AndrewDongminYoo AndrewDongminYoo force-pushed the feat/review-resolve-api branch from 3d6701b to 68800ee Compare June 27, 2026 03:55
@AndrewDongminYoo AndrewDongminYoo changed the title feat: resolve Review items via the API + a Refresh button on the Review tab feat: resolve Review items via the API (PATCH + bulk) + a Refresh button Jun 27, 2026
AndrewDongminYoo added a commit to AndrewDongminYoo/llm_wiki that referenced this pull request Jun 27, 2026
Review ids were a module-level incrementing counter (review-N). Any
queue rebuild (e.g. shrinking the ingest queue) re-ran review generation,
which re-numbered every review and — because addItems only deduped
against *pending* items — discarded resolved state. A resolved review
came back as a brand-new pending one, and the resolve API's id-targeting
pointed at a number that no longer meant the same thing.

Make the id content-derived and stable:

- reviewIdFor(item) = FNV-1a(type + normalizeReviewTitle(title)). Same
  logical review → same id across regeneration, file moves, reloads.
  Deliberately excludes sourcePath (mutable — a rename would re-id the
  review, the very instability being removed).
- addItems dedups on the stable id against ALL items incl. resolved
  (resolved wins, array fields merged) — this is the actual fix for
  resolved state being dropped on re-surface.
- setItems migrates on load: remaps old counter ids to stable ids and
  collapses duplicates (resolved wins, union pages/queries, earliest
  createdAt). Idempotent — id computed from content, no version flag.
- addItem dedups by identity too (no revive of a resolved item).

Removes the counter machinery. Tests rewritten for stable-id behaviour,
incl. acceptance cases: resolve-then-reingest keeps id+resolved, and two
counter-id rows with identical content collapse to one resolved item on
load. Makes PR nashsu#492's id-based resolve actually reliable.
@nashsu nashsu merged commit 6fa4de6 into nashsu:main Jun 28, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants