feat(posthog): add relayfile adapter - #255
Conversation
|
Warning Review limit reached
Next review available in: 45 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
📝 WalkthroughWalkthroughAdds a new ECMAScript-module PostHog adapter with canonical paths, resource discovery, webhook normalization, digest classification, auxiliary index/alias emission, package exports, and build integration. ChangesPostHog adapter contracts and package setup
Path discovery and sync surfaces
Webhook and digest processing
Auxiliary file emission
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant PostHogWebhook
participant normalizePostHogWebhook
participant computePostHogPath
participant emitPostHogAuxiliaryFiles
participant AuxiliaryEmitterClient
PostHogWebhook->>normalizePostHogWebhook: alert payload
normalizePostHogWebhook->>computePostHogPath: object type, object ID, project ID
computePostHogPath-->>normalizePostHogWebhook: canonical JSON path
normalizePostHogWebhook->>emitPostHogAuxiliaryFiles: normalized record and file event
emitPostHogAuxiliaryFiles->>AuxiliaryEmitterClient: write indexes and aliases
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 76b1d3c395
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export const resources = [ | ||
| { | ||
| name: "projects", | ||
| path: "/posthog/projects/{projectId}.json", |
There was a problem hiding this comment.
These entries cause the generators to publish PostHog in WRITEBACK_PATH_CATALOG and expose posthogClient, but a repo-wide search finds no PostHog writeback resolver, no discovery/posthog/.adapter.md, and none of the referenced schemas or create examples. Consequently consumers are told these paths support mutations even though writes cannot be validated or translated into PostHog API calls; either implement the complete writeback contract or leave resources empty so the adapter is cataloged as read-only.
AGENTS.md reference: AGENTS.md:L210-L218
Useful? React with 👍 / 👎.
| const projectId = readString(payload.project_id) ?? readString(payload.projectId); | ||
| const objectId = readString(payload.id) ?? readString(payload.alert_id); |
There was a problem hiding this comment.
PostHog project and resource IDs are commonly JSON numbers, but readString rejects them, so a normal alert payload with numeric project_id returns null here instead of producing an event. The auxiliary emitter has the same string-only assumption in readProjectId/readObjectId, causing numeric projects, dashboards, insights, flags, and similar records to be silently skipped during sync; normalize finite string/number IDs to strings before path construction.
AGENTS.md reference: AGENTS.md:L108-L108
Useful? React with 👍 / 👎.
| const eventType = | ||
| readString(payload.event_type) ?? | ||
| readString(payload.eventType) ?? | ||
| readString(payload.type) ?? | ||
| "posthog.alert.triggered"; |
There was a problem hiding this comment.
When an accepted payload supplies state: "resolved" or status: "resolved" but omits an explicit event-type field, this fallback still emits posthog.alert.triggered; the later state normalization only changes fileEventType. Workflows subscribed to the declared resolved trigger therefore miss the resolution while triggered workflows run incorrectly, so select posthog.alert.resolved after normalizing the lifecycle state.
AGENTS.md reference: AGENTS.md:L21-L24
Useful? React with 👍 / 👎.
| return `${POSTHOG_PATH_ROOT}/projects/${projectId}/${posthogAggregateCollection( | ||
| objectType, | ||
| )}/${id}.json`; |
There was a problem hiding this comment.
All project-scoped entities are emitted as bare <id>.json files even though these flat records have human-readable names or titles and no child artifacts. That conflicts with the workspace path contract requiring <slug>__<id>.json, leaving PostHog paths inconsistent with cross-adapter consumers; the mapper, templates, layout documentation, indexes, and aliases should use the canonical slug-and-ID filename together.
AGENTS.md reference: AGENTS.md:L94-L99
Useful? React with 👍 / 👎.
| ], | ||
| "scripts": { | ||
| "build": "tsc", | ||
| "test": "node --import tsx --test 'src/**/*.test.ts'", |
There was a problem hiding this comment.
This test command currently discovers zero files because the new package contains no tests, so none of the path helpers have compose/parse round-trip coverage, none of the alias trees have collision coverage, and the LAYOUT.md emitter has no length or content assertions. The package test therefore passes while the adapter's required filesystem contracts remain entirely unverified.
AGENTS.md reference: AGENTS.md:L127-L132
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
packages/posthog/src/emit-auxiliary-files.ts (3)
354-379: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
groupByProjecthardcodes"insight"regardless of the actual resource type.
groupByProjectis used for every project-scoped resource (dashboards, feature flags, annotations, experiments, surveys, alert events) but always callsreadProjectId(record, "insight")(Line 359). It's harmless today only becausereadProjectId's non-"project"branch is type-agnostic — but the hardcoded literal is misleading and becomes a silent bug the momentreadProjectIdgains per-type branching for a non-project type.♻️ Thread the actual objectType through
-function groupByProject( - records: readonly PostHogRecord[], -): Map<string, PostHogRecord[]> { +function groupByProject( + records: readonly PostHogRecord[], + objectType: ProjectScopedObjectType, +): Map<string, PostHogRecord[]> { const grouped = new Map<string, PostHogRecord[]>(); for (const record of records) { - const projectId = readProjectId(record, "insight"); + const projectId = readProjectId(record, objectType);- const grouped = groupByProject(records); + const grouped = groupByProject(records, objectType);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/posthog/src/emit-auxiliary-files.ts` around lines 354 - 379, Update groupByProject to accept the actual PostHogPathObjectType and pass that objectType to readProjectId instead of hardcoding "insight". Update every groupByProject call site to provide its resource type, preserving the existing grouping behavior for each project-scoped resource.
112-194: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftFully sequential per-record I/O in both emit loops.
emitProjectsandemitProjectScopedCollectionawaita read + up to several writes/deletes for every record, one at a time, insidefor...ofloops. For larger PostHog workspaces (many insights/dashboards/feature flags) this serializes all network round-trips and will materially slow down each sync.Since the
Mapmutations (rows.set/delete) are synchronous and independent per record, the inner loops are good candidates for controlled concurrency:Use a small limiter (e.g. `p-limit`) rather than unbounded `Promise.all` to avoid overwhelming the underlying client.⚡ Example: bounded concurrency for the per-record loop
- for (const record of projectRecords) { - const objectId = readObjectId(record, objectType); - ... - } + await runWithConcurrencyLimit(projectRecords, CONCURRENCY, async (record) => { + const objectId = readObjectId(record, objectType); + ... + });Also applies to: 196-352
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/posthog/src/emit-auxiliary-files.ts` around lines 112 - 194, Update emitProjects and emitProjectScopedCollection to process records with bounded concurrency using the repository’s limiter pattern (such as p-limit), rather than awaiting each record sequentially in for...of loops. Keep each record’s read, Map mutation, and related writes/deletes within one limited task, then await completion of all tasks before writing the sorted index; preserve existing per-record behavior and avoid unbounded Promise.all.
450-464: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
readUpdatedfalls back to the current timestamp when no date field exists.For records lacking every known timestamp field,
updatedbecomesnew Date().toISOString()(Line 462), which skews the sort order inwriteSortedIndex(records with no real timestamp always sort as "most recently updated") and changes every sync run even if nothing changed. Falling back to the previously stored row'supdatedvalue (from the existing index) instead of "now" would be more stable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/posthog/src/emit-auxiliary-files.ts` around lines 450 - 464, The readUpdated fallback should not use the current timestamp for records without date fields. Update readUpdated and its callers, including writeSortedIndex, to reuse the record’s previously stored updated value from the existing index when available, preserving stable sorting and sync output; only use a non-time fallback when no prior value exists.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/posthog/posthog.mapping.yaml`:
- Around line 1-5: Add a scopeKeys declaration to the posthog mapping, setting
it to project_id to advertise and validate the supported project-level
connection scope for personas.
In `@packages/posthog/src/emit-auxiliary-files.ts`:
- Around line 120-121: Update the merge flow around readIndex and
writeSortedIndex so a missing or unavailable prior index cannot be treated as an
empty Map and overwrite existing rows; distinguish an absent prior state from a
genuinely empty index, then guard or fail the incremental write unless the
emitter is explicitly operating on a complete dataset.
In `@packages/posthog/src/path-mapper.ts`:
- Around line 46-104: Extend the path-mapper helpers beyond
posthogProjectByNameAliasPath to provide deterministic by-name paths for
dashboards, experiments, and surveys using their project-scoped collection,
slugifyAlias, aliasCollisionSuffix, and encoded name/suffix/project-ID format.
Update the alias emission and documentation for these resources so the new
by-name aliases are generated and described, while preserving existing ID-based
aliases and excluding resources without a natural human-readable key.
In `@packages/posthog/src/resources.ts`:
- Around line 11-84: Update the mapping configuration to define the canonical
PostHog mount templates under the appropriate resources or writebacks block,
then change the resources export to derive each { name, path } entry from those
mapping templates instead of hardcoding them. Preserve the existing resource
names and downstream metadata such as patterns and schema paths, while ensuring
routing uses the mapping-derived paths.
In `@packages/posthog/src/webhook-normalizer.ts`:
- Around line 48-59: Update the record construction in the webhook normalizer so
the raw payload is spread before the normalized fields. Ensure normalized id,
project_id, source, kind, title, occurred_at, event_type, state, and severity
values remain authoritative and cannot be overwritten by payload properties.
---
Nitpick comments:
In `@packages/posthog/src/emit-auxiliary-files.ts`:
- Around line 354-379: Update groupByProject to accept the actual
PostHogPathObjectType and pass that objectType to readProjectId instead of
hardcoding "insight". Update every groupByProject call site to provide its
resource type, preserving the existing grouping behavior for each project-scoped
resource.
- Around line 112-194: Update emitProjects and emitProjectScopedCollection to
process records with bounded concurrency using the repository’s limiter pattern
(such as p-limit), rather than awaiting each record sequentially in for...of
loops. Keep each record’s read, Map mutation, and related writes/deletes within
one limited task, then await completion of all tasks before writing the sorted
index; preserve existing per-record behavior and avoid unbounded Promise.all.
- Around line 450-464: The readUpdated fallback should not use the current
timestamp for records without date fields. Update readUpdated and its callers,
including writeSortedIndex, to reuse the record’s previously stored updated
value from the existing index when available, preserving stable sorting and sync
output; only use a non-time fallback when no prior value exists.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c1e6b9b7-87f8-40a9-85fa-bee760a855f1
⛔ Files ignored due to path filters (8)
package-lock.jsonis excluded by!**/package-lock.jsonpackages/core/src/scope-keys/adapters-without-known-scope-keys.generated.jsonis excluded by!**/*.generated.*packages/core/src/scope-keys/catalog.generated.tsis excluded by!**/*.generated.*packages/core/src/triggers/catalog.generated.jsonis excluded by!**/*.generated.*packages/core/src/triggers/catalog.generated.tsis excluded by!**/*.generated.*packages/core/src/writeback-paths/catalog.generated.jsonis excluded by!**/*.generated.*packages/core/src/writeback-paths/catalog.generated.tsis excluded by!**/*.generated.*packages/relay-helpers/src/generated/clients.tsis excluded by!**/generated/**
📒 Files selected for processing (13)
packages/posthog/package.jsonpackages/posthog/posthog.mapping.yamlpackages/posthog/src/digest.tspackages/posthog/src/emit-auxiliary-files.tspackages/posthog/src/index.tspackages/posthog/src/layout-prompt.tspackages/posthog/src/path-mapper.tspackages/posthog/src/resources.tspackages/posthog/src/sync-bucketing.tspackages/posthog/src/types.tspackages/posthog/src/webhook-normalizer.tspackages/posthog/tsconfig.jsonturbo.json
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/posthog/src/emit-auxiliary-files.test.ts`:
- Around line 65-72: Update the rows[0] expected object in the
emitProjectScopedCollection test to include both state and status properties
with undefined values, matching the keys emitted on project-local rows while
preserving the existing assertions.
In `@packages/posthog/src/path-mapper.test.ts`:
- Around line 61-79: Extend the “named aliases use deterministic collision
suffixes” test to cover the by-id, by-short-id, and by-key alias helpers,
asserting repeated inputs remain stable and distinct inputs produce distinct
paths with the expected collision suffix. If any subtree cannot collide by
design, document that provider-uniqueness exception instead of adding a test.
In `@packages/posthog/src/webhook-normalizer.test.ts`:
- Around line 19-20: Update the webhook normalization logic exercised by
normalized.eventType and normalized.fileEventType so posthog.alert.resolved maps
to the repository’s supported terminal lifecycle action and its corresponding
digest classification, not file.updated. Adjust the test assertions to verify
the terminal action and classification while preserving ordinary update behavior
for non-terminal events.
- Around line 44-50: Update the test case “explicit upstream event type remains
authoritative” to use a status that derives a conflicting event type, such as
“resolved,” while keeping the explicit event_type unchanged. Add assertions for
the expected resolved state and file semantics so the test verifies both
event-type precedence and the resulting normalized output.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: aa0deb2e-fba8-4bdb-991c-d9f025ad3c9c
⛔ Files ignored due to path filters (6)
package-lock.jsonis excluded by!**/package-lock.jsonpackages/core/src/scope-keys/catalog.generated.jsonis excluded by!**/*.generated.*packages/core/src/scope-keys/catalog.generated.tsis excluded by!**/*.generated.*packages/core/src/writeback-paths/adapters-without-writeback-paths.generated.jsonis excluded by!**/*.generated.*packages/core/src/writeback-paths/catalog.generated.tsis excluded by!**/*.generated.*packages/posthog/src/resource-paths.generated.tsis excluded by!**/*.generated.*
📒 Files selected for processing (15)
CHANGELOG.mdpackages/posthog/package.jsonpackages/posthog/posthog.mapping.yamlpackages/posthog/scripts/generate-resource-paths.mjspackages/posthog/src/emit-auxiliary-files.test.tspackages/posthog/src/emit-auxiliary-files.tspackages/posthog/src/layout-prompt.test.tspackages/posthog/src/layout-prompt.tspackages/posthog/src/path-mapper.test.tspackages/posthog/src/path-mapper.tspackages/posthog/src/read-only-resources.tspackages/posthog/src/resources.test.tspackages/posthog/src/resources.tspackages/posthog/src/webhook-normalizer.test.tspackages/posthog/src/webhook-normalizer.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/posthog/posthog.mapping.yaml
- packages/posthog/package.json
- packages/posthog/src/webhook-normalizer.ts
| assert.deepEqual(rows[0], { | ||
| id: "73", | ||
| title: "Checkout funnel", | ||
| updated: "2026-07-25T08:10:00.000Z", | ||
| canonicalPath: | ||
| "/posthog/projects/17/dashboards/checkout-funnel__73.json", | ||
| archived: false, | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
node -e "
const assert = require('node:assert/strict');
try {
assert.deepEqual({ id: '73', archived: false }, { id: '73', state: undefined, archived: false });
console.log('PASSED (objects considered equal)');
} catch (e) {
console.log('FAILED:', e.message);
}
"Repository: AgentWorkforce/relayfile-adapters
Length of output: 308
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file and relevant symbols =="
fd -a 'emit-auxiliary-files.*' . | sed 's#^\./##'
echo
echo "== test snippet =="
sed -n '45,80p' packages/posthog/src/emit-auxiliary-files.test.ts || true
echo
echo "== main emitter references =="
rg -n "emitProjectScopedCollection|rows\\[0\\]|state:\\s*readString|status:\\s*readString" packages/posthog/src -S
echo
echo "== implementation snippet around collection emitters =="
sed -n '320,365p' packages/posthog/src/emit-auxiliary-files.ts || trueRepository: AgentWorkforce/relayfile-adapters
Length of output: 7468
Include state/status in the project-local row assertion.
emitProjectScopedCollection writes state and status onto project rows, so rows[0] has those own keys with undefined values. assert.deepEqual compares object key sets, so the expected literal must include them.
🔧 Proposed fix
assert.deepEqual(rows[0], {
id: "73",
title: "Checkout funnel",
updated: "2026-07-25T08:10:00.000Z",
canonicalPath:
"/posthog/projects/17/dashboards/checkout-funnel__73.json",
+ state: undefined,
+ status: undefined,
archived: false,
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| assert.deepEqual(rows[0], { | |
| id: "73", | |
| title: "Checkout funnel", | |
| updated: "2026-07-25T08:10:00.000Z", | |
| canonicalPath: | |
| "/posthog/projects/17/dashboards/checkout-funnel__73.json", | |
| archived: false, | |
| }); | |
| assert.deepEqual(rows[0], { | |
| id: "73", | |
| title: "Checkout funnel", | |
| updated: "2026-07-25T08:10:00.000Z", | |
| canonicalPath: | |
| "/posthog/projects/17/dashboards/checkout-funnel__73.json", | |
| state: undefined, | |
| status: undefined, | |
| archived: false, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/posthog/src/emit-auxiliary-files.test.ts` around lines 65 - 72,
Update the rows[0] expected object in the emitProjectScopedCollection test to
include both state and status properties with undefined values, matching the
keys emitted on project-local rows while preserving the existing assertions.
| assert.equal(normalized.eventType, "posthog.alert.resolved"); | ||
| assert.equal(normalized.fileEventType, "file.updated"); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Do not encode resolved as file.updated.
This assertion locks in generic update semantics for a terminal PostHog state. Normalize resolved webhooks to the repository’s supported terminal lifecycle action and update the corresponding digest classification, rather than treating them as ordinary edits.
As per coding guidelines, terminal states such as resolved must not fall through to a generic updated line unless the provider has no terminal concept.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/posthog/src/webhook-normalizer.test.ts` around lines 19 - 20, Update
the webhook normalization logic exercised by normalized.eventType and
normalized.fileEventType so posthog.alert.resolved maps to the repository’s
supported terminal lifecycle action and its corresponding digest classification,
not file.updated. Adjust the test assertions to verify the terminal action and
classification while preserving ordinary update behavior for non-terminal
events.
Source: Coding guidelines
2 participants
What changed
@relayfile/adapter-posthogpackageWhy
Cloud needs a real adapter package to materialize PostHog sync and webhook records into Relayfile. The trigger catalog updates are also required for webhook autocomplete and deploy-time lint coverage.
Validation
npm run build --workspace=packages/posthognpm run buildnode packages/core/dist/src/cli.js triggers generate --repo-root .node packages/core/dist/src/cli.js triggers check --repo-root .