Pull request overview

This PR introduces a per-record auto-apply hook to the reconciler’s plan-only ingestion log processing flow, allowing specific planned change sets to bypass the OPEN review queue and be applied immediately (without changing the existing tenant-level AUTO_APPLY_CHANGESETS processor selection).

Changes:

• Add AutoApplyPolicy + WithAutoApplyPolicy(...) functional option to IngestionLogProcessor, and invoke it after BulkPlan.
• Extend IngestionProcessorOps with ApplyChangeSetForLog, with an OSS implementation delegating to BulkPlanApply for single items.
• Add tests covering the policy match/no-match paths and ensuring apply + metrics behavior.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f86d4cd0e2

Reviewed the auto-apply hook. Structure is clean — AutoApplyPolicy + WithAutoApplyPolicy functional option are idiomatic, and "ship the hook only, wire the predicate on the diode-pro side" is a good way to land it. One main change plus a safety caveat. (Overlaps the Copilot/Codex threads; consolidating into a single recommendation.)

1. Drop ApplyChangeSetForLog entirely; call BulkPlanApply with the matched item(s) in processBatch. The method is a thin pass-through to BulkPlanApply([]{item}) that collapses the result into error/nil — and that collapse is what produces the metric bug below (a NO_CHANGES outcome returns nil and gets counted as a successful apply of N changes). AutoApplyProcessor.processBatch already consumes BulkPlanApply results directly with the right PlanErr / ApplyErr / changes > 0 switch — the plan-only processor can do the same. That removes the misleading name, the ignored *changeset.ChangeSet param, the extra IngestionProcessorOps method, and its mock, and fixes the metric for free. Worth extracting the shared result→metrics handling into one helper both processors call. (Simplest: BulkPlanApply([]{item}) per matched record; even better: collect the matched items and issue one BulkPlanApply after the loop.) A dedicated single-item method earns its place only once a real apply-only endpoint (no re-plan) exists.

2. The re-plan divergence is a safety caveat, not just perf. The policy is evaluated against the BulkPlan change set, but the apply re-plans and applies a possibly-different set. For an auto-apply gate, a divergent re-plan can apply something the policy never approved. Either document this as an accepted interim limitation or close it with the deferred apply-only endpoint (there's no interception point while BulkPlanApply does plan+apply in one server call).

Nit: the tests sync with time.Sleep(200ms) after go p.Start — flaky under load; a channel fired from a mock would be deterministic.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

@mfiedorowicz addressed your review across df449d2, 6d740b2, 7c07bdd:

1. Drop ApplyChangeSetForLog — done in df449d2: method, interface entry, and mock removed; processBatch consumes BulkPlanApplyResult directly with the PlanErr / ApplyErr / changes > 0 switch, which also fixes the NO_CHANGES metric miscount. 6d740b2 then takes your "even better" variant: matched items are collected during the plan loop and applied with a single BulkPlanApply call after it — one round-trip per batch instead of one per matched record.

2. Re-plan divergence — documented as an accepted interim limitation at the apply call site in 6d740b2, to be closed properly by the deferred apply-only endpoint.

Nit (test sync) — 7c07bdd drops the time.Sleep(200ms) in favor of channels closed from the final expected mock call in each flow (RecordChangeSetApply / RecordChangeSetCreate); verified stable with -count=5 -race.