Skip to content

perf(policy): investigate moving stop-condition evaluation entirely post-write #235

@windoliver

Description

@windoliver

Summary

The current stop-condition evaluation runs in two places:

  1. Pre-write inside PolicyEnforcer.enforce() (src/core/policy-enforcer.ts:267) — runs inside the write mutex, costing one store.list() plus per-root store.thread() calls per contribution.
  2. Post-write recheck in contributeOperation (src/core/operations/contribute.ts:584) — runs outside the mutex to recover the threshold-crossing case (the Nth review satisfying quorum) that the pre-write check misses.

The post-write recheck exists because the pre-write check has the threshold-crossing blind spot. So the pre-write check never detects threshold crossings, and the post-write recheck always has to run anyway. The pre-write check is doing duplicated work.

Hypothesis

If we delete the pre-write call and rely solely on the post-write recheck, we:

  • Halve the stop-condition cost per contribution (one scan instead of two)
  • Move all the O(n) work outside the write mutex (no contention)
  • Lose nothing semantically, since the post-write path was already designed to be the source of truth for threshold crossings

The risk: are there any consumers that depend on policyResult.stopResult being populated before the write? If the PolicyEnforcer is called outside contributeOperation (lifecycle paths, grove_check_stop tool), they may need their own evaluation.

Investigation steps

  1. Audit all callers of PolicyEnforcer.enforce() to see whether they read stopResult.
  2. Check whether any pre-write rejection logic depends on stop conditions being met (the comment at policy-enforcer.ts:255-258 suggests no).
  3. Benchmark: measure throughput of contributeOperation with and without the pre-write call against a 10K-contribution store.
  4. If safe: delete the pre-write block and the skipStopConditions option (added by Issue 13A in the protocol: plan/message operations bypass policy enforcement and topology routing #228 PR), since plans/messages would no longer pay the cost in the first place.

Context

Surfaced during the #228 review (Issue 13D in the locked-in plan, deferred as a separate investigation). Issue 13A landed in the #228 PR as a tactical fix (skipStopConditions for plan + ephemeral kinds), which is a good targeted patch but not the structural fix.

References

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions