[posthog]

💡 Motivation and Context

Contract: local-feature-flag-evaluator — "Experiment holdouts precede release conditions" and "Holdout membership uses backend-compatible bucketing" (added in sdk-specs#76). Acceptance: acceptance/private/local-feature-flag-holdouts.feature.

Why: the SDK compliance matrix in PostHog/sdk-specs records this as an open Local Feature Flag Evaluator gap for posthog-go:

Experiment holdouts entirely unimplemented (grep holdout: zero matches); every other new matching requirement is met.

The matrix calls holdouts "the highest-value single fix in the matrix right now": it is a silent correctness divergence, not a missing feature. A flag configured with a holdout was evaluated locally as though the holdout did not exist, so every identity inside the holdout got an answer that disagreed with the backend — no error, no fallback, and experiment results contaminated by users who should have been excluded. The definition loader already preserved the data; the evaluator simply never read it.

What changed

• Filter gains Holdout *Holdout (id, exclusion_percentage), so the field already present on the wire is decoded.
• matchFeatureFlagProperties resolves the holdout right after the flag-level bucketing id is computed and before any condition is evaluated, returning the synthetic holdout-<id> variant. That variant does not need to appear in filters.multivariate.variants. The inactive-flag short circuit in computeFlagLocally still wins, so an inactive flag with a holdout stays false.
• Membership uses the flag-level bucketing identity — group key for group-aggregated flags, device id for device-bucketed flags, distinct id otherwise. Per-condition aggregation does not change it.
• Hashing uses holdout-<bucketing_value>: calculateHash is refactored into calculateHashWithSeparator, and holdoutHash joins with - rather than the . used for rollout hashing. Neither the flag key nor the holdout id participates. The separator is the whole point — the dot-separated hash is equally uniform and deterministic while holding out a different set of people than the server.
• exclusion_percentage is clamped to 0–100 without truncating fractions, compared inclusively (hash <= pct/100), and a clamped 100 includes everyone without hashing.
• A holdout object missing or null on id or exclusion_percentage is skipped and ordinary evaluation continues, matching posthog-python's _get_holdout_variant.

Single-flag, bulk (GetAllFlags), and dependency evaluation all route through matchFeatureFlagProperties, so they share these semantics automatically — a flag depending on checkout == "holdout-727" compares against the holdout string.

Backwards compatibility

Additive. Holdout is a new exported type and a new field on Filter; no existing field, method, or signature changes, and api/public-api.txt is regenerated accordingly. Behavior only changes for flags whose definitions actually carry filters.holdout — and for those, the previous answer was wrong. Flags without a holdout take an extra nil check and are otherwise untouched. The API-shape agreement is the published sdk-spec, per CONTRIBUTING.md.

💚 How did you test it?

New featureflags_holdouts_test.go covers the spec's scenarios: holdout beating unavailable person properties, 0% rollout and a variant override; inactive flag staying false; no-holdout and incomplete-holdout flags keeping their ordinary assignment; partial membership pinned to the server's hash vectors (user-1 → 0.17805599206573022 held out, user-5 → 0.6563813925994418 not); inclusive and clamped percentage boundaries; flag-level group identity and device-id bucketing each overriding the distinct id; the holdout value flowing into a dependent flag; holdout fields surviving definition decoding and a refresh that removes them; and numeric and string ids both rendering as holdout-727.

Run at the repository root: go build ./..., go vet ./..., gofmt -l ., make api-diff, and go test -race -count=1 ./... — all green. No nested-module (otel/, posthogmcpsdk/) code is touched. No manual testing against a live project was performed.

📝 Checklist

• I reviewed the submitted code.
• I added tests to verify the changes.
• I updated the docs if needed.
• No breaking change or entry added to the changelog.

If releasing new changes

• Ran pnpm changeset to generate a changeset file (.changeset/local-eval-holdouts.md, minor)

Follow-up work

• The same gap is open in posthog-ruby and posthog-dotnet (PHP is covered by feat(flags): honor experiment holdouts in local flag evaluation posthog-php#254); posthog-python already has the fix and was the reference here.
• Not addressed, and still open for posthog-go per the matrix: Evaluate Flags negative-knowledge retention, and the Feature Flag Called Tracker session-attribution allowlist.

🤖 Agent context

Autonomy: Fully autonomous

Produced by an unattended Claude Code run that reads the compliance matrices in PostHog/sdk-specs and implements one open, backwards-compatible gap per run. This gap was chosen over the other open posthog-go candidates because the matrix ranks it highest-value, the verdict is backward-compatible, and the spec pins the exact hash, precedence, and clamping rules — leaving little room for interpretation. posthog-python's implementation was read as the reference for the skip-incomplete-holdout and clamping semantics; its hash vectors are reused as test fixtures. Deliberately left alone: the calculateHash fast path (refactored to take a separator rather than duplicated) and every non-holdout evaluation rule.

Agent-authored — requires human review; not self-merged.

---

Created with PostHog Desktop

🤖 Generated with Claude Code

[github-actions]

posthog-go Compliance Report

Date: 2026-10-17 01:52:58 UTC
Duration: 169227ms

✅ All Tests Passed!

111/111 tests passed

---

Capture_V1 Tests

✅ 94/94 tests passed

View Details

Test	Status	Duration
Endpoint And Method.Targets V1 Endpoint	✅	18ms
Endpoint And Method.Does Not Use Legacy Endpoints	✅	6ms
Required Headers.Has Authorization Bearer Header	✅	6ms
Required Headers.Has Content Type Json	✅	7ms
Required Headers.Has Posthog Sdk Info Format	✅	6ms
Required Headers.Has Posthog Attempt Header	✅	6ms
Required Headers.Has Posthog Request Id	✅	6ms
Required Headers.Has Posthog Request Timestamp	✅	6ms
Required Headers.Has User Agent	✅	7ms
Body Format.Body Has Created At And Batch	✅	6ms
Body Format.No Api Key In Body	✅	6ms
Body Format.No Sent At In Body	✅	6ms
Event Format.Event Has Required Root Fields	✅	6ms
Event Format.Event Uuid Is Valid	✅	6ms
Event Format.Event Timestamp Is Rfc3339	✅	6ms
Event Format.Distinct Id Is String	✅	6ms
Event Format.Distinct Id At Root Not Properties	✅	16ms
Event Format.Custom Properties Preserved	✅	7ms
Event Format.Set Properties Preserved	✅	6ms
Event Format.Set Once Properties Preserved	✅	6ms
Event Format.Groups Properties Preserved	✅	6ms
Event Format.Sdk Generates Uuid If Not Provided	✅	6ms
Event Format.Event Has Required Root Fields Batch	✅	9ms
Event Format.Event Uuid Is Valid Batch	✅	9ms
Event Format.Event Timestamp Is Rfc3339 Batch	✅	8ms
Event Format.Distinct Id Is String Batch	✅	9ms
Event Format.Distinct Id At Root Not Properties Batch	✅	9ms
Event Format.Custom Properties Preserved Batch	✅	9ms
Event Format.Set Properties Preserved Batch	✅	10ms
Event Format.Set Once Properties Preserved Batch	✅	9ms
Event Format.Groups Properties Preserved Batch	✅	9ms
Event Format.Sdk Generates Uuid If Not Provided Batch	✅	9ms
Batch Behavior.Multiple Events In Single Batch	✅	32ms
Batch Behavior.Batch Envelope Smoke	✅	11ms
Batch Behavior.Flush With No Events Sends Nothing	✅	3ms
Batch Behavior.Flush At Triggers Batch	✅	1008ms
Batch Behavior.Created At Reflects Batch Creation Time	✅	7ms
Deduplication.Generates Unique Uuids	✅	16ms
Deduplication.Different Events Same Content Different Uuids	✅	8ms
Deduplication.Preserves Uuid On Retry	✅	5135ms
Deduplication.Preserves Timestamp On Retry	✅	5136ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry	✅	5138ms
Deduplication.No Duplicate Events In Batch	✅	32ms
Header Behavior On Retry.Attempt Header Starts At One	✅	7ms
Header Behavior On Retry.Attempt Header Increments On Retry	✅	10139ms
Header Behavior On Retry.Request Id Preserved On Retry	✅	5135ms
Header Behavior On Retry.Different Requests Have Different Request Ids	✅	2013ms
Header Behavior On Retry.Request Timestamp Changes On Retry	✅	5133ms
Response Format Validation.Success Response Has Uuid Keyed Results	✅	8ms
Response Format Validation.Success Response Has Ok For Each Event	✅	28ms
Response Format Validation.Success No Retry After When All Ok	✅	27ms
Response Format Validation.Success Retry After Present When Retry Events	✅	131ms
Response Format Validation.Success No Retry After When Drop Only	✅	27ms
Response Format Validation.Response Echoes Request Id	✅	6ms
Retry Behavior.Retries On 408	✅	5136ms
Retry Behavior.Retries On 500	✅	5134ms
Retry Behavior.Retries On 503	✅	5133ms
Retry Behavior.Retries On 504	✅	5134ms
Retry Behavior.Retryable Errors Have Retry After	✅	2132ms
Retry Behavior.Respects Retry After On Retryable Error	✅	8141ms
Retry Behavior.Does Not Retry On 400	✅	2132ms
Retry Behavior.Does Not Retry On 401	✅	2134ms
Retry Behavior.Does Not Retry On 402	✅	2133ms
Retry Behavior.Does Not Retry On 413	✅	2133ms
Retry Behavior.Does Not Retry On 415	✅	2133ms
Retry Behavior.Non Retryable Errors Have No Retry After	✅	2133ms
Retry Behavior.Implements Backoff	✅	15146ms
Retry Behavior.Max Retries Respected	✅	15146ms
Partial Batch Handling.Handles 200 Full Success	✅	2009ms
Partial Batch Handling.Handles 200 With All Ok	✅	3033ms
Partial Batch Handling.Does Not Retry Dropped Events	✅	3029ms
Partial Batch Handling.Does Not Retry Limited Events	✅	3031ms
Partial Batch Handling.Prunes Ok Events On Partial Retry	✅	5136ms
Partial Batch Handling.Prunes Dropped Events On Partial Retry	✅	5137ms
Partial Batch Handling.Retries Only Retry Events From Partial	✅	5133ms
Partial Batch Handling.Partial Retry Preserves Uuids	✅	5136ms
Partial Batch Handling.Partial Retry Attempt Header Increments	✅	5136ms
Partial Batch Handling.Partial Retry Request Id Preserved	✅	5135ms
Partial Batch Handling.Respects Retry After On Partial	✅	5136ms
Partial Batch Handling.Unknown Result Treated As Terminal	✅	3029ms
Partial Batch Handling.Mixed Ok Drop Limited No Retry	✅	3033ms
Compression.Sends Gzip Content Encoding	✅	18ms
Compression.No Content Encoding When Disabled	✅	6ms
Compression.Compressed Body Is Decompressible	✅	7ms
Error Handling.Does Not Retry On Unknown 4Xx	✅	2131ms
Event Options.Cookieless Mode Override	✅	8ms
Event Options.Disable Skew Correction Override	✅	6ms
Event Options.Process Person Profile Override	✅	6ms
Event Options.Product Tour Id Override	✅	6ms
Event Options.Unset Options Omitted	✅	5ms
Event Options.Options Override In Batch	✅	9ms
Geoip And Historical Migration.Geoip Disable Injected Into Properties	✅	6ms
Geoip And Historical Migration.Historical Migration Set In Body	✅	6ms
Geoip And Historical Migration.Historical Migration Absent By Default	✅	6ms

Feature_Flags Tests

✅ 17/17 tests passed

View Details

Test	Status	Duration
Request Payload.Request With Person Properties Device Id	✅	16ms
Request Payload.Flags Request Uses V2 Query Param	✅	16ms
Request Payload.Flags Request Hits Flags Path Not Decide	✅	15ms
Request Payload.Flags Request Omits Authorization Header	✅	17ms
Request Payload.Token In Flags Body Matches Init	✅	16ms
Request Payload.Groups Round Trip	✅	15ms
Request Payload.Groups Default To Empty Object	✅	17ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False	✅	15ms
Request Payload.Disable Geoip Omitted Defaults To False	✅	15ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key	✅	17ms
Request Lifecycle.No Flags Request On Init Alone	✅	2ms
Request Lifecycle.No Flags Request On Normal Capture	✅	6ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests	✅	30ms
Request Lifecycle.Mock Response Value Is Returned To Caller	✅	15ms
Retry Behavior.Retries Flags On 502	✅	17ms
Retry Behavior.Retries Flags On 504	✅	17ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event	✅	18ms

[ioannisj]

🤖 An AI agent wrote this on behalf of @ioannisj.

⚠️ PUBLIC API CHANGE
Holdout (new exported struct: ID interface{}, ExclusionPercentage *float64) — added (featureflags.go:169-178)
Filter.Holdout *Holdout — field added (featureflags.go:152)
Changeset level: minor (.changeset/local-eval-holdouts.md)

Nothing to fix here. Went through the holdout path against the flags server and posthog-python, and the order (after the inactive check, before release conditions), the holdout-<bucketing_value> hash with the - separator, the clamping and the flag-level bucketing identity all line up. The user-1 / user-5 vectors in the tests also reproduce with the server's SHA-1 formula.

One gap: payload lookup for a holdout-<id> variant works through the existing variant-keyed path, but nothing tests it. Fine as a follow-up I think.

[dustinbyrne]

The holdout implementation otherwise looks sound, including precedence, hashing, clamping, payload lookup, and definition replacement. Consider addressing the missing-device case noted inline: partial holdouts currently resolve locally using distinct ID despite unavailable required device identity.

AI-assisted review.

[dustinbyrne]

[P1 / IMPORTANT] Do not assign a partial holdout using distinct ID when required device identity is missing

Location: featureflags.go:1229–1230.

An active person-level flag with bucketing_identifier: "device_id" and a complete partial holdout can resolve conclusively without a device ID.

For example, after loading a flag with holdout ID 727 at 20%:

client.GetFeatureFlag(posthog.FeatureFlagPayload{
    Key:                 "checkout",
    DistinctId:          "user-1",
    OnlyEvaluateLocally: true,
})

getBucketingID substitutes "user-1" when DeviceId is nil (featureflags.go:1187–1192). The new holdout branch hashes that substituted identity and returns "holdout-727", nil before release conditions: the pinned "user-1" hash is approximately 0.17805599206573022.

This is publicly reachable input. Validation requires a key and distinct ID, but does not require DeviceId (feature_flag_config.go:29–68).

Governing contract: The pinned holdout bucketing requirement explicitly says missing required identity must follow inconclusive/fallback rules rather than substitute an unrelated identity. Python’s resolve_bucketing_value independently rejects unavailable device identity.

Base-to-head causality: Ordinary missing-device substitution predates this PR; it is not independently reported here. This diff newly applies it to holdout membership. If an ordinary release condition requires an unavailable region property, base evaluation reaches that condition and remains inconclusive. Head instead returns a successful holdout assignment before checking it, suppressing otherwise eligible fallback.

The shared path affects single getters, GetAllFlags, EvaluateFlags and its context wrapper, capture enrichment with SendFeatureFlags, and supported same-context dependencies. Snapshot population stores the assignment as locally resolved; access events can report it with locally_evaluated: true. The public FeatureFlagResult itself does not expose that marker.

Impact qualification: This can select a holdout population using the wrong identity and conceal unavailable evaluation context. However, pinned Rust backend code also substitutes distinct ID when device identity is missing. This is therefore a canonical-contract violation—not a demonstrated nil-device disagreement with current backend master. Remote evaluation may return the same string.

Consider returning an inconclusive error before partial holdout hashing when required device context is missing or empty. Scope the check to usable, person-level device-bucketed holdouts; preserve group bucketing, incomplete-holdout skipping, ordinary no-holdout behavior, and the hash-free full-holdout path.

Regression sketch — not executed

This exact unit regression uses the helpers already added in featureflags_holdouts_test.go:

func TestHoldoutMissingDeviceIsInconclusive(t *testing.T) {
    poller := &FeatureFlagsPoller{}
    flag := holdoutFlag(&Holdout{
        ID:                  float64(727),
        ExclusionPercentage: floatPtr(20),
    })
    flag.BucketingIdentifier = stringPtr(bucketingIdentifierDevice)
    flag.Filters.Groups[0].Properties = []FlagProperty{{
        Key:      "region",
        Type:     "person",
        Operator: "exact",
        Value:    "USA",
    }}

    value, err := poller.computeFlagLocally(
        flag, "user-1", nil, nil, nil, nil, nil,
    )

    var inconclusive *InconclusiveMatchError
    require.ErrorAs(t, err, &inconclusive)
    require.NotEqual(t, "holdout-727", value)
}

Expected: Inconclusive required-context error.
Head result implied by source: "holdout-727", nil; the assertions fail.
Proposed command, not run: go test -count=1 -run '^TestHoldoutMissingDeviceIsInconclusive$' .

For public-entry regression coverage:

1. Serve the same definition from /flags/definitions, including the unavailable region condition and ordinary control/test variants.
2. Create a client with a fixture-only secret key and mock endpoint.
3. Count /flags requests atomically and return {"featureFlags":{"checkout":false}}.
4. Disable single-getter access events to isolate evaluation traffic.

Action	Expected after fix	Head behavior implied by source
Local-only getter, no device ID	Inconclusive error; no remote request	Holdout string; nil error
Getter with fallback allowed	One remote request; mocked false	No request; holdout string
Local-only EvaluateFlags, scoped to checkout	checkout absent from Keys()	Holdout assignment present
Snapshot evaluation with fallback allowed	Remote response fills checkout	Assignment remains locally resolved

The remote fixture verifies fallback eligibility, not deployed backend assignment.

Retain controls for explicit device "user-1"; device "user-5" with matching region properties; group-key bucketing without a device ID; absent/incomplete holdouts; and empty device identity.

These regressions were not executed; the described results are derived from static source inspection.

[ioannisj]

🤖 An AI agent wrote this on behalf of @ioannisj.

Fixed in 32151f5. getHoldoutVariant now takes the distinct and device ids itself and returns an InconclusiveMatchError when a device-bucketed person flag has a partial holdout and no device id, so the distinct id can't stand in for the device identity in the holdout hash and the caller falls back to the server like any other missing context.

Full holdouts, incomplete holdouts, no-holdout flags and group flags behave as before. TestHoldoutMissingDeviceIsInconclusive covers the nil and empty device id cases and fails on c0937ee.

Kept it scoped to holdouts: the ordinary distinct id fallback in getBucketingID predates this PR, and posthog-node's blanket inconclusive for any device-bucketed flag without a device id is I think worth its own change.

[github-actions]

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week.

[marandaneto]

The code looks good; no qualifying findings. Verdict: correct.