fix: reset flag-called tracking when definitions reload - #267
Conversation
|
maybe a better approach would be to check the results and the cached flags and only clear the values that have changed, wdyt? or is it ok to emit again? i recall those events being "expensive" but later this was improved during ingestion |
posthog-ruby Compliance ReportDate: 2026-09-05 17:27:50 UTC ✅ All Tests Passed!46/46 tests passed Capture Tests✅ 29/29 tests passed View Details
Feature_Flags Tests✅ 17/17 tests passed View Details
|
Prompt To Fix All With AI### Issue 1
lib/posthog/feature_flags.rb:1288
**Refresh Can Duplicate Events**
During a background refresh, the new definitions become visible before this callback clears the deduplication tracker. A concurrent evaluation can use those new definitions and record its `$feature_flag_called` event, but the callback then erases that record. A later read emits the event again even though no additional reload occurred. The tracker reset must be ordered with definition publication, such as by associating entries with a definition generation.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "fix: reset flag-called tracking when def..." | Re-trigger Greptile |
| logger.debug "Loaded #{@feature_flags.length} feature flags and #{@cohorts.length} cohorts" | ||
| @flag_definitions_loaded_at.value = (Time.now.to_f * 1000).to_i | ||
| @loaded_flags_successfully_once.make_true if @loaded_flags_successfully_once.false? | ||
| @on_flag_definitions_updated&.call |
There was a problem hiding this comment.
During a background refresh, the new definitions become visible before this callback clears the deduplication tracker. A concurrent evaluation can use those new definitions and record its $feature_flag_called event, but the callback then erases that record. A later read emits the event again even though no additional reload occurred. The tracker reset must be ordered with definition publication, such as by associating entries with a definition generation.
Knowledge Base Used:
Prompt To Fix With AI
This is a comment left during a code review.
Path: lib/posthog/feature_flags.rb
Line: 1288
Comment:
**Refresh Can Duplicate Events**
During a background refresh, the new definitions become visible before this callback clears the deduplication tracker. A concurrent evaluation can use those new definitions and record its `$feature_flag_called` event, but the callback then erases that record. A later read emits the event again even though no additional reload occurred. The tracker reset must be ordered with definition publication, such as by associating entries with a definition generation.
**Knowledge Base Used:**
- [Feature flag evaluation](https://app.greptile.com/posthog-org-19734/-/custom-context/knowledge-base/posthog/posthog-ruby/-/docs/feature-flags.md)
- [Feature flag definition cache](https://app.greptile.com/posthog-org-19734/-/custom-context/knowledge-base/posthog/posthog-ruby/-/docs/feature-flag-definition-cache.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
💡 Motivation and Context
Related to #189.
After local flag definitions reload, the client still remembers earlier
$feature_flag_calledevents and suppresses the next access. Clear the existing tracker when definitions are applied or when a quota response discards loaded definitions. Manual reloads, background polling, and external-cache refreshes use the same callback.The reset uses the existing mutex. Repeated reads between reloads remain deduplicated. HTTP 304 responses, failed requests, and responses without definitions leave tracking unchanged. This does not change the behavior of evaluations already in flight during a refresh.
💚 How did you test it?
bundle exec rspec: 690 examples, 0 failures.bundle exec rubocop(85 files),bundle exec rake public_api:check, and the core gem build. All passed. The gem build retains its existing missing-author warning.origin/mainpassed for4f4b1de269fc651c7a79a1ba0f09d4ed0c90bf96with no findings.Validation used Ruby 4.0.6 and Bundler 4.0.13 with frozen dependencies. The CI Ruby 3.2/3.3/3.4 matrix was not run locally. Tests use WebMock rather than a live PostHog service.
📝 Checklist
If releasing new changes
pnpm changesetto generate a changeset fileA patch changeset for
posthog-rubywas added directly and preserved from the implementation. The generator was not run.🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Pi agents implemented and independently reviewed this user-directed fix. Tools used were Git, Bundler/RSpec, RuboCop, Rake, RubyGems, GitHub CLI, and the isolated Pi autoreview helper. Session references: implementation
3efed453-bba9-4ebc-853b-b7247693a8d8, publication510ebebc-a750-45a5-88ce-92821ccd3256(local sessions, no public link).The fix keeps the existing bounded tracker and mutex rather than adding generation tracking or changing the public client API. Human review is required before merging.