fix(flags): tolerate numeric flag dependency keys - #71802
Merged
Conversation
A flag dependency with a numeric `key` was persisted as a raw JSON number and then failed deserialization in the Rust flags service (where PropertyFilter.key is typed as String), which took down flag evaluation for the entire project with an HTTP 500. Write side: coerce the flag-dependency key to a string in the serializer so numbers are never persisted. Read side: add a serde deserializer that accepts a string or a number for PropertyFilter.key, healing data already stored as numbers. Generated-By: PostHog Code Task-Id: 88538b27-811a-4f6d-ae0f-d36b93c7eab3
haacked
force-pushed
the
posthog-code/flag-dependency-numeric-key
branch
from
July 17, 2026 21:18
1a52888 to
7bc2fae
Compare
Refactor the property key deserialization to use an untagged enum for cleaner, more maintainable code. Remove redundant type assertion in test.
haacked
marked this pull request as ready for review
July 17, 2026 23:50
Contributor
🤖 CI report
|
| File | Patch | Uncovered changed lines |
|---|---|---|
products/feature_flags/backend/api/feature_flag.py |
83.3% | 1360 |
🤖 Agents: add a test covering the lines above, or note why under "How did you test this code?". Machine-readable gap list: the patch-coverage artifact on this run (gh run download 29623142390 -n patch-coverage), or the coverage-data block at the end of this comment.
Per-product line coverage (touched products)
| Product | Coverage | Lines |
|---|---|---|
platform_features |
██░░░░░░░░░░░░░░░░░░ 12.1% |
7 / 58 |
batch_exports |
████████░░░░░░░░░░░░ 39.7% |
8,416 / 21,225 |
demo |
███████████░░░░░░░░░ 56.2% |
1,497 / 2,663 |
warehouse_sources_queue |
████████████░░░░░░░░ 59.2% |
148 / 250 |
tasks |
█████████████░░░░░░░ 67.0% |
25,933 / 38,686 |
data_tools |
██████████████░░░░░░ 70.0% |
63 / 90 |
ai_gateway |
███████████████░░░░░ 75.0% |
9 / 12 |
signals |
████████████████░░░░ 79.2% |
19,296 / 24,350 |
data_modeling |
████████████████░░░░ 80.0% |
4,834 / 6,045 |
cdp |
████████████████░░░░ 80.7% |
3,118 / 3,864 |
wizard |
████████████████░░░░ 82.5% |
772 / 936 |
notebooks |
█████████████████░░░ 85.1% |
7,096 / 8,338 |
cohorts |
█████████████████░░░ 86.2% |
4,065 / 4,717 |
agent_platform |
█████████████████░░░ 86.4% |
3,807 / 4,405 |
actions |
█████████████████░░░ 86.6% |
717 / 828 |
product_tours |
█████████████████░░░ 87.5% |
1,266 / 1,447 |
exports |
██████████████████░░ 88.4% |
6,933 / 7,842 |
visual_review |
██████████████████░░ 88.5% |
5,565 / 6,287 |
business_knowledge |
██████████████████░░ 88.5% |
4,400 / 4,969 |
conversations |
██████████████████░░ 89.0% |
16,183 / 18,186 |
mcp_analytics |
██████████████████░░ 89.2% |
2,514 / 2,819 |
dashboards |
██████████████████░░ 89.3% |
5,841 / 6,540 |
engineering_analytics |
██████████████████░░ 89.5% |
5,507 / 6,154 |
alerts |
██████████████████░░ 89.9% |
4,045 / 4,499 |
early_access_features |
██████████████████░░ 90.1% |
1,031 / 1,144 |
error_tracking |
██████████████████░░ 90.1% |
9,855 / 10,935 |
streamlit_apps |
██████████████████░░ 90.4% |
2,501 / 2,767 |
slack_app |
██████████████████░░ 90.6% |
9,018 / 9,955 |
links |
██████████████████░░ 90.6% |
183 / 202 |
marketing_analytics |
██████████████████░░ 90.8% |
11,514 / 12,684 |
stamphog |
██████████████████░░ 91.0% |
3,993 / 4,387 |
product_analytics |
██████████████████░░ 91.3% |
5,757 / 6,304 |
mcp_store |
██████████████████░░ 91.8% |
3,685 / 4,012 |
data_warehouse |
██████████████████░░ 92.4% |
18,771 / 20,309 |
notifications |
███████████████████░ 92.7% |
1,031 / 1,112 |
workflows |
███████████████████░ 92.8% |
5,482 / 5,909 |
ai_observability |
███████████████████░ 92.8% |
14,916 / 16,077 |
web_analytics |
███████████████████░ 92.9% |
13,837 / 14,897 |
surveys |
███████████████████░ 93.0% |
5,724 / 6,157 |
posthog_ai |
███████████████████░ 93.2% |
1,325 / 1,421 |
approvals |
███████████████████░ 93.3% |
3,395 / 3,640 |
reminders |
███████████████████░ 93.4% |
468 / 501 |
tracing |
███████████████████░ 93.4% |
2,546 / 2,725 |
managed_migrations |
███████████████████░ 93.8% |
1,220 / 1,300 |
legal_documents |
███████████████████░ 94.1% |
1,568 / 1,667 |
endpoints |
███████████████████░ 94.1% |
8,606 / 9,143 |
revenue_analytics |
███████████████████░ 94.5% |
3,598 / 3,809 |
skills |
███████████████████░ 94.5% |
2,881 / 3,049 |
messaging |
███████████████████░ 94.5% |
2,530 / 2,677 |
review_hog |
███████████████████░ 94.6% |
6,806 / 7,191 |
logs |
███████████████████░ 95.2% |
9,439 / 9,911 |
experiments |
███████████████████░ 95.7% |
24,417 / 25,527 |
replay_vision |
███████████████████░ 95.8% |
13,776 / 14,383 |
growth |
███████████████████░ 95.8% |
2,837 / 2,960 |
annotations |
███████████████████░ 96.2% |
732 / 761 |
feature_flags |
███████████████████░ 96.3% |
16,216 / 16,842 |
user_interviews |
███████████████████░ 96.4% |
2,242 / 2,325 |
warehouse_sources |
███████████████████░ 96.4% |
287,393 / 297,985 |
access_control |
███████████████████░ 96.8% |
849 / 877 |
customer_analytics |
███████████████████░ 97.2% |
7,482 / 7,700 |
data_catalog |
███████████████████░ 97.4% |
2,304 / 2,365 |
analytics_platform |
████████████████████ 98.0% |
2,102 / 2,145 |
metrics |
████████████████████ 98.2% |
2,404 / 2,448 |
pulse |
████████████████████ 98.4% |
2,017 / 2,049 |
live_debugger |
████████████████████ 99.2% |
613 / 618 |
field_notes |
████████████████████ 99.4% |
158 / 159 |
Report-only. Patch coverage = changed backend lines covered vs origin/master. Sorted lowest first.
Known gaps: lines covered only by Temporal tests show as uncovered; core line numbers may drift if master changed the same file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A single feature flag with a numeric dependency
keycould take down flag evaluation for an entire project, returning HTTP 500 to all SDKs.There's a type mismatch between how flag dependencies are written and read. On the write side,
_validate_flag_referenceruns the dependency reference throughsafe_int(), which accepts a numeric key, so the raw JSON number gets stored in the flag's filters. On the read side, the Rust flags service declaresPropertyFilter.keyasString, and serde won't coerce a JSON number into a string, so deserialization fails. Because the whole team's flag set is parsed as one payload, one bad field fails all of them — the entire project's flags return no data to SDKs.Changes
Two complementary fixes:
products/feature_flags/backend/api/feature_flag.py): coerce the flag-dependencykeyto a string during serializer validation so numbers are never persisted.rust/feature-flags/src/properties/property_models.rs): add a serde deserializer that accepts a string or a number forPropertyFilter.key, normalizing to a string. This also heals flags already persisted with numeric keys, so one malformed flag can no longer sink the whole team's cached payload. The evaluator already re-parses the key to an id at match time.How did you test this code?
Added targeted regression tests:
keynow returns 201 and the persisted key is stored as a string (guards against dropping the coercion, which would re-introduce the numeric write that crashes Rust deserialization). No existing test asserted the stored form.PropertyFilterdeserializes both a numeric and a stringkey, andget_feature_flag_idstill resolves the numeric case.The agent could not run the Rust test suite (no cargo toolchain in the sandbox) or the DB-backed Python test (no Postgres); the Python test collected cleanly and
ruff checkpasses.Automatic notifications
Docs update
🤖 Agent context
Autonomy: Fully autonomous
Investigated a reported production HTTP 500 in flag evaluation. Traced the root cause to a write/read type mismatch for flag-dependency keys between the Django serializer and the Rust flags service. Chose a two-sided fix: normalize on write (so new data is always a string) and tolerate on read (so already-persisted numeric keys stop breaking the whole project's payload). Invoked the
/writing-testsskill before adding tests. Considered a broader "skip one malformed flag, serve the rest" change to the hypercache parse path, but the string-or-number deserializer is the surgical fix that directly resolves the reported symptom and self-heals existing data, so that broader hardening was left out to keep the change focused.Created with PostHog Code from this inbox report.