feat(posthog): add per-metric summary reports (#3824) - #4153
feat(posthog): add per-metric summary reports (#3824)#4153sanskar-singh-2403 wants to merge 1 commit into
Conversation
Greptile code reviewThis repo uses Greptile for automated review. Before merge, aim for Confidence Score: 5/5 with zero unresolved review threads — see CONTRIBUTING.md. Run a review — add a PR comment with: Give it ~5-10 minutes (sometimes longer) for results, then fix feedback and re-trigger until you reach Confidence Score: 5/5. Optional: automate with the greploop skill. |
Greptile SummaryThis PR adds per-metric PostHog analytics summary reports, mirroring the existing Sentry morning-digest pattern: a bundled
Confidence Score: 4/5Safe to merge after adding the missing tests/integrations/posthog/init.py; all other changes are clean and follow established patterns. The implementation faithfully mirrors the Sentry digest pattern and all critical paths work correctly. The only blocking gap is the absent tests/integrations/posthog/init.py: every other integration test subdirectory in the repo has one, and tests/platform/scheduler/init.py was added in this very PR for the same reason — the omission will break test collection for the new test file. Files Needing Attention: tests/integrations/posthog/ — missing init.py that the project's test package convention requires.
|
| Filename | Overview |
|---|---|
| integrations/posthog/report_runner.py | New headless runner; directly mirrors sentry/morning_digest_runner.py pattern. Result extraction and error handling are sound. |
| integrations/posthog/report_prerequisites.py | New prerequisites module; stats_period_to_hours correctly handles h/d/w units with graceful fallback for invalid input. |
| surfaces/cli/commands/posthog_report.py | New CLI command group; prerequisite gates are correctly placed. schedule add omits rocketchat as a provider choice, inconsistent with the Sentry digest command and the underlying Provider enum. |
| tests/integrations/posthog/test_report_runner.py | Good test coverage for prompt building and prerequisite checks, but the new tests/integrations/posthog/ directory is missing init.py, required by this project's test package convention. |
| platform/scheduler/tasks.py | New _build_posthog_metric_report builder added; credential stripping, default period, and source/task_id override ordering are all correct and consistent with existing builders. |
| platform/scheduler/delivery.py | Delivery module moved from integrations/sentry/digest_delivery.py; renamed function correctly generalises it for all report types. |
| integrations/scheduled_agent_bootstrap.py | PostHog routing via 'posthog' in source correctly intercepts both CLI and scheduled task payloads before the Sentry fallback. |
| tests/scheduler/test_tasks.py | Thorough new tests for the PostHog builder: agent-runner routing, default period, credential stripping, and failure propagation all covered. |
Sequence Diagram
sequenceDiagram
participant CLI as opensre posthog report run
participant Runner as scheduled_agent_bootstrap
participant Report as run_posthog_report
participant Harness as AgentHarness
participant Agent as HeadlessAgent
participant MCP as PostHog MCP Tools
CLI->>CLI: require_posthog_integration()
CLI->>Runner: invoke_agent_runner(payload)
Runner->>Runner: posthog in source
Runner->>Report: run_posthog_report(payload)
Report->>Report: _require_posthog_configured()
Report->>Harness: startup()
Harness-->>Report: session
Report->>Agent: dispatch(prompt)
Agent->>MCP: list_posthog_tools / call_posthog_tool
MCP-->>Agent: metric data
Agent-->>Report: ShellTurnResult
Report-->>CLI: report text
Reviews (3): Last reviewed commit: "feat(posthog): add per-metric summary re..." | Re-trigger Greptile
53865a6 to
b7ad420
Compare
|
@greptile review |
|
@muddlebee can you kindly let me know your thoughts on this? |
|
There's no demo, need screen recording? |
b7ad420 to
b037138
Compare
|
Demo - ** Recording 2026-07-20 212111.mp4 - Google Drive integrations verify posthog_mcp — connection confirmed, event schema fetched |
|
thank you will take a look shortly |
There was a problem hiding this comment.
Nice work . One correctness issue I'd want fixed, left inline, plus two process gaps that have no line to anchor to:
Docs — this adds a user-facing opensre posthog report group with no docs change. docs/posthog.mdx and docs/posthog-mcp.mdx already exist and docs/sentry.mdx documents the Sentry digest, so there's a clear home for it. Needs the docs/docs.json entry too or Mintlify won't surface the page.
384d9b5 to
32c50e0
Compare
|
@Davidson3556 I have made all the changes suggested by you, kindly review this and let me know your thoughts. |
32c50e0 to
3468004
Compare
Devesh36
left a comment
There was a problem hiding this comment.
@Davidson3556 can you please check this again
3468004 to
56fc4aa
Compare
hey @Devesh36 can we re-run the pipeline i have fixed the test |
|
Hey @muddlebee @Davidson3556, just a friendly reminder whenever you get a chance to review this PR. Thanks! |
|
ci is failing. |
|
2ea7e48 to
2d938b2
Compare
| scripts_dir = Path(sys.executable).parent | ||
| lint_imports = next( | ||
| ( | ||
| candidate | ||
| for candidate in ( | ||
| scripts_dir / "lint-imports", | ||
| scripts_dir / "lint-imports.exe", | ||
| ) | ||
| if candidate.is_file() | ||
| ), | ||
| None, | ||
| ) |
There was a problem hiding this comment.
@Davidson3556 i added this for python.exe path to be taken as well in case of windows OS
Done both addressed in the latest push.
Tested locally lint-imports --strict passes (1 kept, 0 broken) and the affected suites are green. The only local failure is tests/config/test_runtime_metadata_perf.py::test_baseline_stability, which is unrelated to this change: it's a wall-clock threshold test (asserts build_runtime_metadata runs in <5ms) and it blows the budget only because I'm running under pytest -n auto on a WSL2 box where CPU contention across workers pushes it to ~240ms. It should pass on CI's dedicated runner 🤞 . |
|
Hey @Davidson3556 can we re-run the CI? if you are ok with the changes? Thanks! |
|
@sanskar-singh-2403 thank you for the efforts, will review in next 1-2 days |
hey @muddlebee @Davidson3556 bumping this up for a review, Thanks! |
|
First step ideally would be to discuss in the github issue about your approach and thoughts rather than sending PRs directly. |
|
I think I missed to mention that earlier. |
Got it, thanks for flagging @muddlebee, that makes sense. I'll write up my approach and reasoning on the linked issue so we can align there first. Happy to adjust the implementation based on that discussion, and I'll keep this PR as a draft in the meantime. |
Add PostHog analytics per-metric summary reports, mirroring the existing Sentry morning-digest (Viktor-style) pattern. - posthog-summary skill: pull metrics via PostHog MCP tools, format a per-metric report with current/previous deltas and trends - report_runner: headless posthog-summary turn returning the report text - report_prerequisites: PostHog + delivery-provider readiness checks - CLI: opensre posthog report run + report schedule add/list/remove/run (on-demand and scheduled) - scheduler: POSTHOG_METRIC_REPORT TaskKind, message builder, runner route - assistant prompt: PostHog report output-shape rule for consistent format - tests: builder + runner coverage
2d938b2 to
e26a09b
Compare
|
@larsspinetta12 this is ready for review, Thanks! |
Fixes #3824
Describe the changes you have made in this PR -
Adds PostHog analytics per-metric summary reports, mirroring the existing
Sentry morning-digest ("Viktor-style") pattern so the surface, output shape,
and scheduling behaviour stay consistent across integrations.
integrations/posthog/tools/skills/posthog-summary/SKILL.md):pulls metrics via the existing PostHog MCP tools (
list_posthog_tools/call_posthog_tool) and formats a per-metric report with current vs. previousdeltas, percent change, and trend direction.
integrations/posthog/report_runner.py): runs one headlessposthog-summary turn and returns the assistant report text; guards on PostHog
being configured.
integrations/posthog/report_prerequisites.py):PostHog integration + delivery-provider readiness checks.
surfaces/cli/commands/posthog_report.py):opensre posthog report run(on-demand) andopensre posthog report schedule add/list/remove/run(scheduled), registeredin the CLI command list.
POSTHOG_METRIC_REPORTTaskKind, a message builderin
platform/scheduler/tasks.py(strips credential keys, defaults to a 7dwindow), and a route in
integrations/scheduled_agent_bootstrap.py.table format stays deterministic.
tests/scheduler/test_tasks.pyand runnercoverage in
tests/integrations/posthog/test_report_runner.py.Acceptance criteria from the issue: connect to PostHog data source ✅,
per-metric summary report ✅, Viktor-style output format ✅, on-demand and
scheduled triggering ✅.
Demo/Screenshot for feature changes and bug fixes -
Code Understanding and AI Usage
Did you use AI assistance (ChatGPT, Claude, Copilot, etc.) to write any part of this code?
If you used AI assistance:
Explain your implementation approach:
The issue asks for automated per-metric PostHog reports "like Viktor reports."
The codebase already had that exact pattern for Sentry (the morning digest), so
rather than inventing a new mechanism I mirrored it: a bundled skill defines the
report workflow/format, a headless runner dispatches a single agent turn driven
by that skill, and the scheduler multiplexes the task onto the shared
agent-runner seam via a new
TaskKind. PostHog connectivity already existed(
integrations/posthogREST config +posthog_mcptool surface), so no newtransport code was needed — the runner just reuses those tools.
Alternative considered: querying the PostHog REST API directly and templating a
report in Python. I rejected that because it would duplicate query logic the MCP
tool surface already exposes, wouldn't reuse the shared skill/prompt formatting,
and would diverge from the Sentry precedent that reviewers already know.
Key components:
run_posthog_report(headless turn + report extraction),_build_posthog_metric_report(scheduler builder, credential stripping,default window), the
posthog reportCLI group (on-demand + schedulemanagement), and the
posthog-summaryskill/prompt rule that pin the outputshape to a
Metric | Current | Previous | Change | % | Trendtable.Checklist before requesting a review
Note: Please check Allow edits from maintainers if you would like us to assist in the PR.