Skip to content

fix(signals): verify replay vision output fields before scout aggregates - #111232

Open
posthog[bot] wants to merge 5 commits into
masterfrom
posthog-self-driving/fixscout-verify-replay-vision-output-333adf
Open

posthog[bot] wants to merge 5 commits into
masterfrom
posthog-self-driving/fixscout-verify-replay-vision-output-333adf

Conversation

@posthog

@posthog posthog Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Problem

  • The Replay Vision scout can misread or skip real scanner findings on projects whose scanner output differs from what its SQL templates assume.
  • Each scanner type writes different scanner_output_* fields: a monitor writes verdict, a scorer score, a classifier tags, and a summarizer title/summary. Older observations lack newer fields.
  • HogQL reads an absent property as NULL without an error. So a yes_rate template returns 0 and a mean_score returns NULL or a partial average. Both look like a real shift.
  • The skill told the scout to use read-data-schema, but it did not require that check before each scanner-specific aggregate.

Origin

  • Scout: a custom scout
  • First signal: 2026-10-03
  • Inbox report: open
  • Task started by: auto-start, after the report was rated P3 and ready to fix

Changes

  • New footgun references need removing #6 maps each scanner type to the output fields it writes.
  • New "Verify the output schema" step runs before any monitor, scorer, classifier, or summarizer aggregate:
    • read-data-schema on $recording_observed.
    • One SQL query for per-scanner field coverage, split into this week and the prior 3 weeks.
    • A fallback table per scanner type. Monitors, scorers, and classifiers fall back to vision-scanners-observations-stats with date_from/date_to, which reads the stored scanner result, not the event. Summarizers fall back to summary_line from the list tool.
    • With no fallback, the scout writes a pattern: entry and does not report a shift.
  • The verdict/score and tag queries now say to keep only verified columns.
  • New disqualifier: a field that appears or disappears between windows is a coverage step, not an output shift.
  • Prompt-only change. No code paths change.

How did you test this code?

  • Ran the new coverage query through execute-sql on a project with live scanners. It parsed and returned per-type coverage of 1.0 for the expected field and 0.0 for the others, for each scanner type.
  • Formatted the file with oxfmt.
  • Did not run a full scout run with the edited skill.

Test rationale: No automated test reads scout skill prose. The query check above is the closest verification.

Release status

  • No feature flag controls this change
  • This change is behind a feature flag and is not available to users
  • This change makes a previously flagged feature available to everyone

Automatic notifications

  • Publish to changelog?

Docs update

None. The skill file is the documentation for this scout.

🤖 Agent context

Autonomy: Fully autonomous

Agent: PostHog Desktop (Claude Code), claude-opus-5-5

  • Skills invoked: /writing-simplified-technical-english.
  • Field-per-type mapping comes from the scanner output models in products/replay_vision/backend/temporal/scanners/.
  • Duplicate search: open PRs that touch this skill (#103892) change tool availability wording, not output-field checks.
  • Considered adding coverage columns to the roster query instead of a separate step. Rejected because a separate per-window query also catches fields that cover only part of the comparison window.

Created with PostHog Desktop from this inbox report.

🤖 Generated with Claude Code

Each scanner type writes different scanner_output_* fields, and HogQL reads an absent property as NULL. The scout now checks the event schema and per-window field coverage before a monitor, scorer, classifier, or summarizer aggregate, and falls back to the stored-result stats tool when a field is absent.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Generated-By: PostHog Desktop
Task-Id: 3428bd44-ed1c-420c-8aa7-ea4a7d68cae1
@posthog posthog Bot added the self-driving label Oct 3, 2026
@posthog
posthog Bot marked this pull request as ready for review October 3, 2026 05:47
@posthog

posthog Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor Author

🦔 PostHog Review reviewed this pull request

Found 0 must fix, 1 should fix, 1 consider.

Published 2 findings (view the review).

Resolved comments: 2 fixed, 1 already settled

@trunk-io

trunk-io Bot commented Oct 3, 2026

Copy link
Copy Markdown

Merging to master in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Generated-By: PostHog Desktop
Task-Id: 3428bd44-ed1c-420c-8aa7-ea4a7d68cae1
stamphog[bot]

This comment was marked as outdated.

@coderabbitai

coderabbitai Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used
📚 Code guidelines (5)
.agents/security.md — configured
products/signals/skills/AGENTS.md — auto-discovered
.agents/skills/writing-skills/SKILL.md — configured
.claude/commands/conventions.md — configured
.agents/skills/writing-code-comments/SKILL.md — configured

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
  • Review profile: QUIET
  • Plan: Enterprise
  • Run ID: a871686f-aee7-4c61-8da1-0f00464d0546
📥 Commits

Reviewing files that changed from the base of the PR and between 0430b91 and 59b215a.

📒 Files selected for processing (1)
  • products/signals/skills/signals-scout-replay-vision/SKILL.md

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 6 remain after this review.


📝 Walkthrough

Walkthrough

The skill adds a procedure to verify scanner output properties and their coverage in current and prior windows. It specifies stats and observations-list fallbacks for absent or partial fields, and directs the scout to skip aggregates when no fallback exists. Aggregate, classifier, and summarizer guidance now limits field use to schema-verified fields.

Priority: ⬇️ Low

Merge Risk: 🔵 Low · up to 59b21

On unusually large teams, some scanner comparisons may fall back or be skipped because the coverage query can omit selected scanners. The impact is bounded, so merging carries low risk.

Architecture Summary

Architecture risk: 🔵 Low · up to 59b21

The change affects 1 system.

Changed systems: products

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — products (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in products/signals/skills/signals-scout-replay-vision/SKILL.md: The SQL footguns list adds a sixth case: output fields differ by scanner type and may be absent or partial, making aggregates such as countIf and avg misleading. The guidance now requires verifying each field before scanner-specific aggregation.
  • observed — Modified behavior in products/signals/skills/signals-scout-replay-vision/SKILL.md: Adds a schema-verification procedure: inspect available event properties, measure per-scanner field coverage across current and prior windows, and use a field only when coverage is near 1.0 in both. When coverage is absent or partial, use the specified stats or observations-list fallback for that scanner type; if none is available, skip the aggregate and record the missing field.
  • observed — Modified behavior in products/signals/skills/signals-scout-replay-vision/SKILL.md: Aggregate guidance now requires retaining only the monitor or scorer column verified for that scanner’s schema; the previous text did not condition these comparisons on schema verification.
  • observed — Modified behavior in products/signals/skills/signals-scout-replay-vision/SKILL.md: Classifier tag-distribution guidance now requires verifying tag fields against the schema and dropping any unverified field before running the aggregate.
🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The description explains the problem, user-visible changes, test steps and limits, release status, and agent context. It is complete enough to stand alone, though it omits the session link, CodeRabbit…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

🤖 CI report

⚠️ Trunk lane — backend Python lane

This PR is assigned to the backend Python lane. It runs backend Python tests and may merge in parallel with PRs in other lanes.

@hosthog

hosthog Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

HostHog preview — posthog-desktop-web

Latest build (59b215a): https://58bd2dc437da4e1c93eebd1fbb31efbf.hosthog.dev

Earlier builds of this PR, still serving:

Employee-gated; every push gets a fresh URL whose content never changes. All previews stop serving when the PR closes.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
  • Review profile: QUIET
  • Plan: Enterprise
  • Run ID: 24448b5a-18ac-4199-a834-331e441fa76b
📥 Commits

Reviewing files that changed from the base of the PR and between e7ba672 and 0430b91.

📒 Files selected for processing (1)
  • products/signals/skills/signals-scout-replay-vision/SKILL.md

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 5 remain after this review.

Comment thread products/signals/skills/signals-scout-replay-vision/SKILL.md Outdated
@posthog

posthog Bot commented Oct 3, 2026

Copy link
Copy Markdown
Contributor Author

PostHog Review alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏

@posthog posthog Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PostHog Review

Found 1 should fix, 1 consider.

Comment thread products/signals/skills/signals-scout-replay-vision/SKILL.md
Comment thread products/signals/skills/signals-scout-replay-vision/SKILL.md Outdated
…rately

An OR across two fields reports full coverage when one field is absent from every row. Each field now has its own coverage column, and label coverage is measured before the scorer fallback uses it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Generated-By: PostHog Desktop
Task-Id: 3428bd44-ed1c-420c-8aa7-ea4a7d68cae1
@stamphog
stamphog Bot dismissed their stale review October 3, 2026 06:03

A new stamphog review started for this PR — the fresh verdict replaces this approval.

stamphog[bot]

This comment was marked as outdated.

…roster

execute-sql returns 100 rows when a query has no LIMIT. The coverage query returns up to two rows per scanner, so it covered at most 50 scanners while the roster holds up to 100. LIMIT 500 is the execute-sql maximum and fits both windows for the full roster.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Generated-By: PostHog Desktop
Task-Id: cb3fb405-966b-4c16-8ba8-9cd3057405e8
@stamphog
stamphog Bot dismissed their stale review October 3, 2026 06:05

A new stamphog review started for this PR — the fresh verdict replaces this approval.

read-data-schema lists properties from the newest events only, so it can miss the output field of a less frequent scanner type. The coverage query now keeps every field column, and its coverage, not the sample, decides which fields a scanner writes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Generated-By: PostHog Desktop
Task-Id: cb3fb405-966b-4c16-8ba8-9cd3057405e8
stamphog[bot]

This comment was marked as outdated.

@stamphog
stamphog Bot dismissed their stale review October 3, 2026 06:06

A new stamphog review started for this PR — the fresh verdict replaces this approval.

@stamphog stamphog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved.

Prompt-only edit to one scout skill markdown file, outside risky territory and easy to reverse. The referenced stats tool exists, and I found no suppression markers or unaddressed substantive review concerns.

Gate mechanics and policy version
Gate Result
prerequisites ✓ all clear
deny-list ✓ no deny categories matched
size ✓ 0L, 0F substantive, 57L/1F incl. docs/generated/snapshots — within ceiling
tier ✓ T0 auto-approve: T0-deterministic (57L, 1F, single-area, fix)
stamphog 2.3.1 .stamphog/policy.yml @ 59b215a · reviewed head 59b215a

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant