Skip to content

feat(serve): add ingestion API routes + integration tests (GH-328)#372

Merged
fagemx merged 3 commits intomainfrom
feat/gh-328-ingestion-api-routes
Mar 27, 2026
Merged

feat(serve): add ingestion API routes + integration tests (GH-328)#372
fagemx merged 3 commits intomainfrom
feat/gh-328-ingestion-api-routes

Conversation

@fagemx
Copy link
Copy Markdown
Owner

@fagemx fagemx commented Mar 27, 2026

Summary

  • Add 6 HTTP API routes to edda-serve exposing the ingestion trigger engine and suggestion queue from edda-ingestion (feat(ingestion): add trigger tables + evaluator engine #326, feat(ingestion): add suggestion queue + workflow #327):
    • POST /api/ingestion/evaluate — evaluate trigger (auto/suggest/skip)
    • POST /api/ingestion/records — manual ingestion (bypass trigger)
    • GET /api/ingestion/records — list ingestion records (with filters)
    • GET /api/ingestion/suggestions — list pending suggestions
    • POST /api/ingestion/suggestions/{id}/accept — accept and write to ledger
    • POST /api/ingestion/suggestions/{id}/reject — reject without writing
  • Add 8 integration tests covering all three ingestion flows (auto/suggest/skip), manual ingestion, error cases (404/400), and suggest-then-reject
  • Thin pass-through handler pattern: all business logic stays in edda-ingestion, handlers are mechanical adapters
  • Pre-check pattern for accept/reject to produce proper HTTP status codes (404/409) consistent with post_draft_approve

Closes #328

Test plan

  • cargo fmt --check -p edda-serve — formatting clean
  • cargo clippy -p edda-serve -- -D warnings — zero warnings
  • cargo test -p edda-serve — 105 tests pass (8 new ingestion tests)
  • cargo test -p edda-ingestion — 53 tests pass (no regressions)

🤖 Generated with Claude Code

Expose the ingestion trigger engine and suggestion queue through 6 HTTP
API routes: evaluate trigger, manual ingest, list records, list pending
suggestions, accept suggestion, and reject suggestion. Includes 8
integration tests covering auto/suggest/skip flows and error cases.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Clippy 1.94 catches needless &format!() on .uri() calls that implement
the required traits. Remove the unnecessary borrows.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@fagemx
Copy link
Copy Markdown
Owner Author

fagemx commented Mar 27, 2026

Code Review: PR #372 (Round 1)

Summary

This PR adds 6 ingestion API routes to edda-serve exposing the trigger engine and suggestion queue from edda-ingestion. The implementation follows the established thin pass-through handler pattern, with all business logic staying in edda-ingestion. Code quality is high overall — proper error handling, workspace locking, and input validation.

Key Findings

Critical Issues (P0)

None.

High Priority (P1)

  1. Inefficient trigger_type filter (crates/edda-serve/src/lib.rs:4316): get_ingestion_records uses serde_json::to_value(r.trigger_type) to serialize a Copy enum into JSON just to compare it as a string. This allocates a serde_json::Value per record. A simpler approach: match against the enum variants directly or use Display/to_string().

  2. Missing test for 409 Conflict on double-accept/reject (crates/edda-serve/src/lib.rs:4351-4355, 4375-4379): The handlers have pre-check logic returning 409 Conflict when a suggestion is already accepted/rejected, but no test exercises this path. This is the only untested handler code path.

Testing Review

Coverage

  • Auto-ingest via evaluate: covered (ingestion_auto_ingest_via_evaluate)
  • Suggest-then-accept flow: covered (ingestion_suggest_then_accept)
  • Skip flow: covered (ingestion_never_ingest_skips)
  • Manual ingest: covered (ingestion_manual_record)
  • 404 on nonexistent accept/reject: covered (ingestion_accept_nonexistent_404, ingestion_reject_nonexistent_404)
  • Suggest-then-reject: covered (ingestion_suggest_then_reject)
  • Invalid source_layer 400: covered (ingestion_evaluate_invalid_layer_400)
  • Missing: 409 Conflict on already-accepted/rejected suggestion

Convention Compliance

  • All tests use tempfile::tempdir() with real SQLite (no mocking) — correct
  • All tests use #[tokio::test] — correct
  • Test naming follows existing convention — correct

Testing Verdict: Adequate (minor gap on 409 path)

Verdict: Changes Requested

Fixing P1 issues and will re-review.


Round 1 of automated review-fix loop

- Replace serde_json::to_value trigger_type filter with direct match
- Add test for 409 Conflict on double-accept of suggestion

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@fagemx
Copy link
Copy Markdown
Owner Author

fagemx commented Mar 27, 2026

Code Review: PR #372 (Round 2) — LGTM 🎉

All P0 and P1 issues have been resolved.

Summary

This PR adds 6 ingestion API routes to edda-serve with 9 integration tests (8 original + 1 added for 409 Conflict coverage). The implementation follows the established thin pass-through handler pattern — all business logic stays in edda-ingestion, handlers are mechanical adapters with proper input validation, workspace locking, and HTTP status codes.

Fixes applied in round 1:

  • Replaced serde_json::to_value trigger_type filter with direct match (avoids unnecessary allocation)
  • Added ingestion_accept_already_accepted_409 test covering the 409 Conflict code path

Verdict: LGTM ✅

No critical or high-priority issues remaining. This PR is ready for merge.


Completed after 2 round(s) of automated review-fix loop

@fagemx fagemx merged commit 3d0ea13 into main Mar 27, 2026
7 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.

feat(ingestion): add ingestion API routes + integration tests

1 participant