fix(insights): coerce sortableSemver argument to string

[sampennington]

Problem

ClickHouse queries using a semver comparison operator against an event/person property that has a Numeric PropertyDefinition fail deterministically with Illegal type Float64 of argument of function extract (code 43).

sortableSemver inlines to extract(assumeNotNull({}), ...) — a string operation. When the comparison targets a Numeric-typed property, the property-types transform first coerces the field to Float64, so extract() receives a Float64 and ClickHouse rejects it.

Part of an effort to reduce deterministic product-analytics query-builder failures — tracking dashboard: https://metabase.prod-us.posthog.dev/dashboard/207-product-analytics-insight-query-failures

Changes

Wrap the sortableSemver argument in toString() so the function always operates on a string, regardless of how the property was typed upstream.

How did you test this code?

I'm an agent. Added an executing HogQL test (test_sortable_semver_on_numeric_property) that creates a Numeric PropertyDefinition and runs a sortableSemVer comparison against it through ClickHouse. Verified the test fails on origin/master with the exact code 43 error and passes with the fix. Ran the existing sortable_semver test suite — all pass. No snapshot files reference sortableSemver.

Publish to changelog?

no

🤖 Agent context

Agent-authored (Claude Code, Opus 4.7). Requires human review.

Root cause traced from a production failing query: the offending expression was extract(assumeNotNull(accurateCastOrNull(..., 'Float64')), '(\d+(\.\d+)+)'). The Float64 cast comes from the property-types transform, not from sortableSemver itself. Fix chosen at the function definition (posthog/hogql/functions/posthog.py) so it is robust to any upstream coercion rather than special-casing the property-types transform.

[github-actions]

🎭 Playwright didn't run on this PR — your changes touch code that could affect E2E behavior, but Playwright is opt-in via label now to keep CI cost down.

Add the run-playwright label if you want an E2E sweep before merging — CI will pick it up automatically.

Most PRs don't need this. Real regressions still get caught on master and fix-forward.

[sampennington]

Automated code review

Scope: One-line fix in posthog/hogql/functions/posthog.py wrapping the sortableSemver argument in toString(), plus a reproducing ClickHouse test in posthog/hogql/test/test_query.py.

Correctness — the fix is sound

Wrapping assumeNotNull({}) in toString() is the right layer. extract() is a string operation and was receiving a Float64 whenever the property-types transform coerced a Numeric-typed property via accurateCastOrNull(..., 'Float64'). Fixing at the function definition makes it robust to any upstream coercion rather than special-casing the transform. toString() is a no-op on already-string values, so existing behavior is unchanged.

Test — verifies "no crash" but not the interesting path

The chosen property value "170" has no dots, so the sortableSemver regex (\d+(\.\d+)+) returns '' and both sides collapse to [0] — the assertion would pass even if the semver logic were broken; it only proves "does not raise". Consider a real dotted semver (e.g. "1.7.0") and assert an ordering, which also exercises the accurateCastOrNull → NULL production scenario.

Verdict: Ship it

The production fix is correct, minimal, and well-targeted. The only gap is test strength — not a blocker.

---

Automated review — posted on behalf of a code-reviewer agent.