Repository navigation
Conversation
|
Merging to
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 |
|
@greptile-apps @veria-ai @parameterai |
🤖 CI report
|
| File | Comment lines | Added lines |
|---|---|---|
nodejs/src/cdp/async-functions/classify.ts |
8 | 89 |
products/workflows/backend/api/workflow_classifications.py |
8 | 130 |
nodejs/src/cdp/async-functions/classify.test.ts |
3 | 76 |
products/workflows/backend/api/hog_flow.py |
3 | 12 |
nodejs/src/cdp/config.ts |
2 | 4 |
posthog/settings/data_stores.py |
2 | 5 |
products/workflows/backend/api/test/test_workflow_classifications_api.py |
2 | 110 |
nodejs/src/cdp/async-functions/internal-api-call.ts |
1 | 4 |
This check does not block merging. It updates on every push and clears when the share drops.
⚠️ Bundle size — 🔺 +665 B (+0.0%)
Uncompressed size of every built .js bundle, compared against the base branch.
Total: 68.94 MiB · 🔺 +665 B (+0.0%)
No file changed by more than 1000 B.
Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report
✅ Eager graph — within budget
How much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy import() / React.lazy chunks are not counted.
| Root | Eager (shipped) | Δ vs base | Budget |
|---|---|---|---|
entry (logged-out pages, app bootstrap)src/index.tsx |
1.57 MiB · 22 files | no change | █████████░ 85.5% of 1.84 MiB |
logged-out boot: index + App + bootApp (preloaded by every page, including /login)src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts |
3.56 MiB · 629 files | 🔺 +60 B (+0.0%) | █████████░ 88.4% of 4.03 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
7.38 MiB · 2,332 files | 🔺 +60 B (+0.0%) | █████████░ 88.5% of 8.34 MiB |
🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/layout/navigation-3000/navigationLogic.tsx stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/scenes/dashboard/dashboardLogic.tsx stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/lemon-ui/LemonMarkdown/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/components/RichContentEditor/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/components/CodeSnippet/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/taxonomy/core-filter-definitions-by-group.json stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/scenes/session-recordings/player/sessionRecordingPlayerLogic.ts stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
Largest files eagerly shipped from src/index.tsx
| Size | File |
|---|---|
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 24.6 KiB | ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js |
| 6.3 KiB | ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js |
| 4.5 KiB | ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js |
| 3.9 KiB | ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js |
| 1.4 KiB | ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js |
| 1.3 KiB | src/index.tsx |
| 1.3 KiB | src/RootErrorBoundary.tsx |
| 912 B | ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js |
| 854 B | src/scenes/ChunkLoadErrorBoundary.tsx |
Largest files eagerly shipped from src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
| Size | File |
|---|---|
| 301.8 KiB | ../node_modules/.pnpm/posthog-js@1.434.14_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 267.6 KiB | ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 100.4 KiB | src/lib/api.ts |
| 88.4 KiB | src/products.tsx |
| 69.4 KiB | src/lib/lemon-ui/icons/icons.tsx |
| 40.1 KiB | src/lib/utils/eventUsageLogic.ts |
| 38.7 KiB | ../node_modules/.pnpm/@dnd-kit+core@6.0.8_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@dnd-kit/core/dist/core.esm.js |
| 33.9 KiB | ../node_modules/.pnpm/kea@4.0.0-pre.6_patch_hash=139b8d1f1304f9d9da452a9a1244c94ea679dbcb85687d8999563146879fb6f5_react@18.3.1/node_modules/kea/lib/index.cjs.js |
| 28.4 KiB | src/scenes/scenes.ts |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 301.8 KiB | ../node_modules/.pnpm/posthog-js@1.434.14_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 271.7 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 267.6 KiB | ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 153.7 KiB | ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 100.4 KiB | src/lib/api.ts |
| 98.5 KiB | ../packages/quill/packages/quill/dist/index.js |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
| 90.6 KiB | ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js |
| 88.4 KiB | src/products.tsx |
Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479
✅ Toolbar bundle — eager 2.38 MiB within budget
What the toolbar ships to customer pages, measured from the esbuild output (minified, post-tree-shake). The eager set is the entry plus everything statically imported from it — fetched before any feature runs; deferred chunks load lazily. The eager guardrail is 5.72 MiB. Each output file must also stay below 10 MB, where CloudFront stops compressing it. The module boundary is enforced separately by check-toolbar-graph.
| Metric | Size | Δ vs base | Budget |
|---|---|---|---|
| Eager (shipped) entry + static imports |
2.38 MiB · 19 files | 🔺 +60 B (+0.0%) | ████░░░░░░ 41.5% of 5.72 MiB |
| Deferred (lazy) | 2.10 MiB · 44 files | no change | n/a — loads on demand |
Loader dist/toolbar.js |
1.2 KiB | no change | █░░░░░░░░░ 6.0% of 19.5 KiB |
Largest eagerly-shipped chunks
| Size | File |
|---|---|
| 800.0 KiB | dist/toolbar/toolbar-app-6DYXIWRU.css |
| 651.4 KiB | dist/toolbar/chunk-chunk-JHHDMU36.js |
| 483.6 KiB | dist/toolbar/chunk-chunk-6JFSEK3E.js |
| 138.3 KiB | dist/toolbar/chunk-chunk-AQHCWZ4A.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-FDH2IBXT.js |
| 75.2 KiB | dist/toolbar/toolbar-app-UZMPV5BO.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-TSAL54PB.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-VCU3RQWC.js |
| 21.0 KiB | dist/toolbar/chunk-chunk-JTJ3ZEFX.js |
| 6.8 KiB | dist/toolbar/chunk-chunk-DV7IWQNF.js |
Posted automatically by check-toolbar-size · sizes are toolbar output bytes (shipped, post-tree-shake) from the esbuild metafile
✅ Dist folder size — 🔺 +7.1 KiB (+0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 946.23 MiB · 🔺 +7.1 KiB (+0.0%)
🦔 PostHog Review reviewed this pull requestFound 0 must fix, 5 should fix, 1 consider. Published 6 findings (view the review). Resolved comments: 1 left for you |
|
Took a look at the classify-with-Jev flow. The design holds up on the trust-boundary points you called out: the endpoint is authenticated only via Nothing jumped out as a vulnerability in this pass — didn't do a full line-by-line security review since that wasn't asked for here, just sanity-checked the auth/JWT scoping and the instruction/context separation since those are the parts most likely to hide an injection or auth-bypass issue in this kind of change. Happy to do a deeper pass (JWT claim validation edge cases, categories dict size bounds, rate limiting on the endpoint) if you want a full review. |
|
[High risk] Adds a new workflow action that calls an external LLM classification service. The behavioral issues are non-blocking, but the production-secret requirement must be satisfied before merging. Reviews (1) · Last reviewed commit: "feat(workflows): add a classify with jev..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| WORKFLOW_CLASSIFY_JWT_SECRETS = get_list( | ||
| get_from_env("WORKFLOW_CLASSIFY_JWT_SECRET", "local-dev-workflow-classify-jwt" if DEBUG or TEST else "") |
There was a problem hiding this comment.
Rule Used: When a new secret or environment variable is added (e.g. via get_from_env in settings files, or new entries in .env.example/env.default), warn that it must also be added to the posthog/charts repository and set via posthog/secrets to take effec... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: posthog/settings/data_stores.py
Line: 619-620
Comment:
**Production secret needs provisioning** This new setting defaults to empty in production. The repository requires new secrets to be added to posthog/charts and set through posthog/secrets; provision matching values for Django and the CDP worker before merging. Otherwise, the flagged step cannot run in production.
**Rule Used:** When a new secret or environment variable is added (e.g. via `get_from_env` in settings files, or new entries in `.env.example`/`env.default`), warn that it must also be added to the posthog/charts repository and set via posthog/secrets to take effec... ([source](https://app.greptile.com/posthog-org-19734/github/PostHog/posthog/-/custom-context?memory=c7932f06-eec7-4fbe-b35f-ac93bc7b7a80))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
There was a problem hiding this comment.
Escalated: a person must provision this secret, because it lives in the infra repos and not in this PR.
- Someone must add
WORKFLOW_CLASSIFY_JWT_SECRETto posthog/charts for web and the CDP worker, and set one matching value in posthog/secrets. - The empty production default is deliberate and fails closed. Django rejects all tokens, and the worker fails the step with an error that names the setting.
- The step is behind the
workflow-jev-classify-actionflag, so merging first does not break any existing workflows. - Please decide whether provisioning must happen before merge or can happen before the flag rollout.
How this was verified
No code change, so no tests or lint were run. I read the setting, the Django JWT purpose, and the worker's missing-key check to confirm the step fails closed while the secret is not set.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 📝 WalkthroughWalkthroughAdds a workflow classification API that authenticates scoped JWTs, validates requests, calls Jev, and returns category results. Adds a Node.js async function that calls the API. Adds a hidden template and a feature-gated classification action in the workflow editor. Strict workflow validation checks classification categories when saving. Priority: ➖ Normal Merge Risk: 🔵 Low · up to Some workflows can appear valid in a test run but fail classification when run live. Align the input checks and complete the required Python annotations; the remaining risk is bounded. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to The new action crosses a service-authentication boundary and sends workflow context to an AI gateway. Credential, input, and organization-approval controls are visible, but production key provisioning and rollout have not been verified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings
🧪 Generate unit tests (beta)
🛠️ Fix failing CI checks 💡
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (1)
products/workflows/backend/api/workflow_classifications.py-114-125 (1)
114-125: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winBound workflow classification requests by team or isolate them from web workers.
WorkflowClassificationViewSet.createcallsGatewaySystemOneClient.decidesynchronously. Each classification can hold a Django web worker while the gateway request is in progress. HTTPX 0.28.1 appliestimeout=5.0separately to connect, read, write, and pool operations; it is not an overall five-second deadline.The default DRF throttles do not limit this scoped-JWT caller, and the endpoint has no route-specific throttle or classification concurrency cap. Add a team-scoped burst/sustained throttle sized for the web-worker budget, or route classifications to a dedicated worker pool.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 60ca3968-5e21-40ab-ac29-e55aa239c8df
📒 Files selected for processing (17)
frontend/src/lib/constants.tsxnodejs/src/cdp/async-functions/classify.test.tsnodejs/src/cdp/async-functions/classify.tsnodejs/src/cdp/async-functions/index.tsnodejs/src/cdp/config.tsnodejs/src/cdp/templates/_destinations/posthog_workflows/posthog-jev-classify.template.tsnodejs/src/cdp/templates/index.tsnodejs/src/cdp/utils/jwt-utils.tsnodejs/tests/fixtures/hog-bytecode/875c549169e8670ceaf95a92833c00286e7e5b22a1a190f157cc0d5128097c09.jsonposthog/cdp/flag_gated_templates.pyposthog/jwt.pyposthog/settings/data_stores.pyproducts/workflows/backend/api/test/test_workflow_classifications_api.pyproducts/workflows/backend/api/workflow_classifications.pyproducts/workflows/backend/routes.pyproducts/workflows/backend/service_jwt.pyproducts/workflows/frontend/Workflows/hogflows/panel/HogFlowEditorPanelBuild.tsx
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.
|
|
|
PostHog Review alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏 |
![posthog[bot]](https://avatars.githubusercontent.com/in/1203049?s=60&v=4){kind=small}
| authentication_classes = [WorkflowClassifyJWTAuthentication] | ||
| permission_classes = [IsAuthenticated] | ||
| serializer_class = WorkflowClassificationRequestSerializer |
There was a problem hiding this comment.
Limit classification requests per team
Issue description
Each matching event can start a model request, but this service-JWT endpoint has no team rate or concurrency limit. The default PersonalApiKeyRateThrottle skips authenticated requests without a personal API key. One busy workflow can therefore fill Django workers and consume shared gateway capacity.
Why we think it's a valid issue
- Checked: Django throttling on this view.
WorkflowClassificationViewSet(products/workflows/backend/api/workflow_classifications.py:74-76) sets nothrottle_classes, so it gets the defaultsBurstRateThrottleandSustainedRateThrottle(posthog/settings/web.py:537-540). Both extendPersonalApiKeyRateThrottle. Itsallow_requestcalls_allow_request_internal(..., personal_api_key_only=True)(posthog/rate_limit.py:266-268), so it does not throttle a request that authenticates with a scoped service JWT. This endpoint has no Django limit. - Checked: Limits in the CDP worker.
hog-flow-invocation-pipeline.service.ts:118-126limits invocations per workflow id only. The defaults are a bucket of10_000and a refill of100per second (nodejs/src/cdp/config.ts:252-253). No limit covers the whole team, so a team with several workflows, or with several classify steps in one workflow, multiplies that rate. Each classify call runs inline and waits on Django. Django then waits on the gateway, for up to the 5 sTIMEOUT_SECONDSwhen the gateway is slow. - Found: Every other caller of
build_system_one_clientthat a request can trigger has an explicit throttle:EmojiSearchThrottle(posthog/api/emoji_search.py:61) andSearchIntentBurstThrottle/SearchIntentSustainedThrottle(posthog/api/taxonomic_search_intent.py:147,202). These callers share oneAI_GATEWAY_API_KEY(posthog/llm/system_one_client.py). - Found: The workflow step that sits next to this one,
workflow_tasks.py:223-311, enforces in-flight, per-workflow daily, and per-team daily caps for each AI call that an event starts. That code shows the team already protects per-event AI work this way. - Impact: A workflow that triggers on a high-volume event with a classify step can send up to 100 synchronous model requests per second, for each workflow, with no team cap. That load holds Django workers and uses the shared gateway key. When the gateway returns 429 or 529, Django returns 503 and the worker retries up to 3 times, which adds more load. Emoji search, taxonomic search intent, and replay rerank use the same gateway, so all teams can see them slow down or fail. The trigger (a high-volume trigger event) and the result (another team's burst degrades a shared resource) are both concrete.
- Impact: The step is behind the
workflow-jev-classify-actionflag, so no user sees this yet. Add the limit before the flag rolls out beyond dogfooding.
Suggested fix
Add a limit keyed by the authenticated team before client.decide. Bound concurrent requests as well as request rate, and return a clear overload response when the limit is reached.
Prompt to fix with AI (copy-paste)
## Context
@products/workflows/backend/api/workflow_classifications.py#L74-76
<issue_description>
Each matching event can start a model request, but this service-JWT endpoint has no team rate or concurrency limit. The default `PersonalApiKeyRateThrottle` skips authenticated requests without a personal API key. One busy workflow can therefore fill Django workers and consume shared gateway capacity.
</issue_description>
<issue_validation>
- **Checked:** Django throttling on this view. `WorkflowClassificationViewSet` (`products/workflows/backend/api/workflow_classifications.py:74-76`) sets no `throttle_classes`, so it gets the defaults `BurstRateThrottle` and `SustainedRateThrottle` (`posthog/settings/web.py:537-540`). Both extend `PersonalApiKeyRateThrottle`. Its `allow_request` calls `_allow_request_internal(..., personal_api_key_only=True)` (`posthog/rate_limit.py:266-268`), so it does not throttle a request that authenticates with a scoped service JWT. This endpoint has no Django limit.
- **Checked:** Limits in the CDP worker. `hog-flow-invocation-pipeline.service.ts:118-126` limits invocations per workflow id only. The defaults are a bucket of `10_000` and a refill of `100` per second (`nodejs/src/cdp/config.ts:252-253`). No limit covers the whole team, so a team with several workflows, or with several classify steps in one workflow, multiplies that rate. Each classify call runs inline and waits on Django. Django then waits on the gateway, for up to the 5 s `TIMEOUT_SECONDS` when the gateway is slow.
- **Found:** Every other caller of `build_system_one_client` that a request can trigger has an explicit throttle: `EmojiSearchThrottle` (`posthog/api/emoji_search.py:61`) and `SearchIntentBurstThrottle`/`SearchIntentSustainedThrottle` (`posthog/api/taxonomic_search_intent.py:147,202`). These callers share one `AI_GATEWAY_API_KEY` (`posthog/llm/system_one_client.py`).
- **Found:** The workflow step that sits next to this one, `workflow_tasks.py:223-311`, enforces in-flight, per-workflow daily, and per-team daily caps for each AI call that an event starts. That code shows the team already protects per-event AI work this way.
- **Impact:** A workflow that triggers on a high-volume event with a classify step can send up to 100 synchronous model requests per second, for each workflow, with no team cap. That load holds Django workers and uses the shared gateway key. When the gateway returns 429 or 529, Django returns 503 and the worker retries up to 3 times, which adds more load. Emoji search, taxonomic search intent, and replay rerank use the same gateway, so all teams can see them slow down or fail. The trigger (a high-volume trigger event) and the result (another team's burst degrades a shared resource) are both concrete.
- **Impact:** The step is behind the `workflow-jev-classify-action` flag, so no user sees this yet. Add the limit before the flag rolls out beyond dogfooding.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Add a limit keyed by the authenticated team before `client.decide`. Bound concurrent requests as well as request rate, and return a clear overload response when the limit is reached.
</potential_solution>
There was a problem hiding this comment.
Escalating. The endpoint does lack a team limit, but choosing the limit and what happens when a team hits it is a capacity and product decision.
- Rate: pick a per-team budget for the shared gateway key.
_TeamBucketRateThrottleinposthog/rate_limit.pygives one bucket per project. - Concurrency: an in-flight cap needs a new Redis counter. Decide if the rate limit alone is enough for the dogfood phase.
- Overload status: a DRF throttle returns 429. The worker retries 429 inline 3 times, which adds load, and then fails the step. A non-retried status, like the 409 the AI task step uses, fails fast.
- Configurable caps like
workflow_task_team_rate_limit_per_dayneed aTeamWorkflowsConfigfield and a migration. - Add the limit before the flag rolls out beyond dogfood.
How this was verified
No code change. Checked the default throttle path in posthog/rate_limit.py, the throttles on the other System One callers, the AI task step's caps in workflow_tasks.py, and the worker's retriable statuses in cdp-fetch.ts.
The worker used the default 3 second internal fetch budget, but Django gives the gateway 5 seconds and maps a gateway timeout to a 503 for the worker to retry. The worker aborted first, discarded late answers, and sent the same classification again. callInternalApi now accepts an optional per-attempt timeoutMs. The classify step passes 7 seconds. Other callers keep the default. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: 8cb2ba00-a783-4633-a0ce-424140c553eb
HostHog preview —
|
The AI usage report groups workflow AI spend by the team_id event property. The classify call sent only hog_flow_id, so any billable Jev generation grouped under team 0. The call now sends team_id with the other labels, as the Anthropic gateway client does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: 8cb2ba00-a783-4633-a0ce-424140c553eb
The classification endpoint refuses more than 16 categories or a description over 500 characters with a 400, so such a step failed on every run. A strict workflow save now runs the endpoint's own categories serializer on the step inputs. The mocked test run checks the same limits, so the test panel no longer reports success for inputs a live run rejects. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: 8cb2ba00-a783-4633-a0ce-424140c553eb
HostHog preview —
|
The classification endpoint mapped a gateway rejection to 502, which the worker retries, so a permanent rejection reached the gateway three times. A gateway 5xx also got the rejection message that blames the step inputs. A rejection now returns 422, which the worker does not retry, and a gateway 5xx returns 503 with the busy message. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: 8cb2ba00-a783-4633-a0ce-424140c553eb
The classification endpoint forwarded any context size to the gateway. It now rejects a context over 65,536 characters of JSON with a 400 that names the limit, the same state limit the ml_inference decide API uses for the same model. The worker checks the same limit, so a mocked test run fails on the same input as a live run. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: 8cb2ba00-a783-4633-a0ce-424140c553eb
There was a problem hiding this comment.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (3)
nodejs/src/cdp/async-functions/classify.ts-45-47 (1)
45-47: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winReject unsupported category-description values in the mock path.
parseClassifyPayloadmust match the Django serializer.CharFieldaccepts numeric values by coercing them to strings, but rejectsnull, arrays, objects, and booleans. The workflow template passesinputs.categoriesdirectly topostHogClassify, so these values can reach the mock.Suggested fix
- if ( - Object.values(categories).some( - (description) => typeof description === 'string' && [...description.trim()].length > MAX_DESCRIPTION_LENGTH - ) - ) { + const descriptions = Object.values(categories) + if (descriptions.some((description) => typeof description !== 'string' && typeof description !== 'number')) { + throw new Error('Category descriptions must be strings or numbers') + } + if (descriptions.some((description) => [...String(description).trim()].length > MAX_DESCRIPTION_LENGTH)) { throw new Error(`Keep each category description to ${MAX_DESCRIPTION_LENGTH} characters or fewer`) }products/workflows/backend/api/hog_flow.py-1508-1508 (1)
1508-1508: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winComplete the six Python signatures.
The repository guideline requires annotations on every signature and says to avoid
Any. Apply it to all six cited definitions:
- Use a typed input shape for
_validate_jev_classify_action.- Replace
Anyinvalidate_contextwith a JSON-compatible type. This DRF field-validation hook does not requireAny.- Parameterize the
bodydictionary.- Add
-> NonetosetUp.- Type
categoriesand the_post_flowresponse.- Annotate all parameters and the return value of the test method.
nodejs/src/cdp/async-functions/classify.ts-32-42 (1)
32-42: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winReject questions longer than 2,000 characters before execution.
parseClassifyPayloadaccepts any nonblank question, so mock validation and workflow save accept a question that the live API rejects on every invocation. Enforce the API limit in the shared parser so both mock and live execution fail before sending an invalid request.Suggested fix
const MAX_CATEGORIES = 16 +const MAX_QUESTION_LENGTH = 2000 const MAX_DESCRIPTION_LENGTH = 500 @@ if (typeof payload?.question !== 'string' || !payload.question.trim()) { throw new Error('Enter a question') } + if ([...payload.question].length > MAX_QUESTION_LENGTH) { + throw new Error(`Keep the question to ${MAX_QUESTION_LENGTH} characters or fewer`) + }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 4615871e-bc10-481a-abd5-d74ec5e8d482
📒 Files selected for processing (7)
nodejs/src/cdp/async-functions/classify.test.tsnodejs/src/cdp/async-functions/classify.tsnodejs/src/cdp/async-functions/internal-api-call.tsproducts/workflows/backend/api/hog_flow.pyproducts/workflows/backend/api/test/test_hog_flow.pyproducts/workflows/backend/api/test/test_workflow_classifications_api.pyproducts/workflows/backend/api/workflow_classifications.py
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 0 remain after this review.
Revives the "Classify with Jev" step from #107832 and #111710 on current master. The classification endpoint asks the model through the ml_inference facade with privacy mode on, and rejects answers it cannot branch on. The step and its endpoint stay behind the workflow-jev-classify-action flag. Generated-By: PostHog Desktop Task-Id: 8ed258e7-0201-436d-9625-5743d7c23174
Revives the "Classify with Jev" step from #107832 and #111710 on current master. The classification endpoint asks the model through the ml_inference facade with privacy mode on, and rejects answers it cannot branch on. The step and its endpoint stay behind the workflow-jev-classify-action flag. Generated-By: PostHog Desktop Task-Id: 8ed258e7-0201-436d-9625-5743d7c23174
Revives the "Classify with Jev" step from #107832 and #111710 on current master. The classification endpoint asks the model through the ml_inference facade with privacy mode on, and rejects answers it cannot branch on. The step and its endpoint stay behind the workflow-jev-classify-action flag. Generated-By: PostHog Desktop Task-Id: 8ed258e7-0201-436d-9625-5743d7c23174
Problem
Changes
workflow-jev-classify-actionflag.jev_categoryandjev_confidenceto workflow variables. A condition step branches on them.postHogClassifyasync function. That function calls a Django internal route with a scoped service JWT, as the "Create AI task" and "Run scout" steps do.build_system_one_clientwith no TypeSafe fallback, so the context never leaves PostHog.Note
WORKFLOW_CLASSIFY_JWT_SECREThas to be provisioned for web and the CDP worker before the step works outside local development. Until then the step fails with a message that names the setting.No screenshot yet. The only visible change is one more node in the editor's action list, behind the flag.
How did you test this code?
test_workflow_classifications_api.py: catches user text leaking into the instructions, a TypeSafe fallback being added, wrong status mapping for gateway failures, a missing AI approval check, bad category counts, and a token from another workflow action being accepted.classify.test.ts: catches the worker posting to the wrong route or with a token that is not scoped to the workflow, and checks that the mocked and live calls validate inputs the same way.Release status
Automatic notifications
Docs update
None. The step is behind a flag.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Agent: Claude Code, claude-opus-5-5
/improving-drf-endpoints,/reviewing-with-coderabbit,/writing-pr-descriptions.hogli build:openapiproduced no drift. JWT-only endpoints stay out of the generated clients, as the scout endpoint does.