feat(signals): expose PUT API for suggested-reviewer artefacts#59357
Merged
Conversation
Generated-By: PostHog Code Task-Id: 4b273054-e446-41d6-b4c0-bac3bd4cb309
Contributor
|
🎭 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 Most PRs don't need this. Real regressions still get caught on master and fix-forward. |
Contributor
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
products/signals/backend/test/test_signal_report_artefact_api.py:258-260
Dead method reference — `other_org` is assigned `self._create_org_member` (the bound method object itself, not a call result). The variable is never used, and the `# noqa: F841` confirms the linter already flagged it. As written, the test exercises a random UUID that doesn't belong to any org, which is fine, but the misleading name + dead assignment violates "no superfluous parts." Either call the helper to actually create an out-of-org user (which would require a second organization) or drop the dead line entirely and rename the test to match what it actually verifies.
```suggestion
def test_put_user_uuid_not_in_org_returns_400(self):
# Random UUID not tied to anyone in this org.
```
Reviews (1): Last reviewed commit: "feat(signals): expose PUT API for sugges..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
|
Size Change: +12.4 kB (+0.01%) Total Size: 116 MB 📦 View Changed
ℹ️ View Unchanged
|
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a project-nested DRF viewset to list/retrieve signal report artefacts and to allow PUT full-replacement edits of only the suggested_reviewers artefact type (keeping other artefact types immutable to preserve the agentic pipeline contract). It also regenerates the OpenAPI-derived frontend/MCP clients and introduces a comprehensive backend test suite for the new endpoint behavior.
Changes:
- Added
SignalReportArtefactViewSetunder/api/projects/<team>/signals/reports/<report_id>/artefacts/with GET list/retrieve and PUT update (type-guarded tosuggested_reviewersonly). - Introduced write serializers for suggested reviewer entries + PUT body, and enriched OpenAPI help text for generated clients.
- Added a new backend test module covering replacement semantics, identity resolution, dedupe/idempotency, scoping, and method restrictions; regenerated TS/Zod/MCP API types.
Reviewed changes
Copilot reviewed 5 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| services/mcp/src/api/generated.ts | Regenerated MCP API types to include artefact schemas and list params. |
| products/signals/frontend/generated/api.zod.ts | Added Zod schemas for the new PUT body. |
| products/signals/frontend/generated/api.ts | Added generated client functions for artefacts list/retrieve/update. |
| products/signals/frontend/generated/api.schemas.ts | Added generated TS types for artefacts + paginated list response + write payload. |
| products/signals/backend/views.py | Replaced the old report artefacts action with a nested artefact viewset and implemented PUT update logic. |
| products/signals/backend/test/test_signal_report_artefact_api.py | New unit tests validating new endpoint behavior and invariants. |
| products/signals/backend/serializers.py | Added help_text to read serializer + new write serializers for PUT. |
| posthog/api/init.py | Registered the new nested artefacts viewset under signal reports routing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
MCP UI Apps size report
|
Match the surrounding signals viewset pattern: exclude=True per-method rather than per-method schema overrides. Drop the redundant retrieve() override and get_serializer_class() — the inherited mixin handles retrieve, and update() instantiates the write serializer locally. Regenerated frontend/MCP types accordingly. Generated-By: PostHog Code Task-Id: 4b273054-e446-41d6-b4c0-bac3bd4cb309
The bot added these when the viewset's update/list/retrieve were in the OpenAPI spec. After the simplification commit they no longer are, so the references are dangling. Removing them keeps the yaml honest. Generated-By: PostHog Code Task-Id: 4b273054-e446-41d6-b4c0-bac3bd4cb309
PR overviewSuggested-reviewer artefact API addedThis PR moves report artefacts onto a nested viewset and adds a PUT path for replacing suggested-reviewer content. I checked the new route registration, queryset scoping, API-scope enforcement, deleted-report handling, and user/GitHub identity resolution; the endpoint stays team-scoped and limits writes to the intended artefact type. Security review
Risk: 2/10 |
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
- Exclude artefacts whose parent report is deleted: SignalReportViewSet filters DELETED reports from its queryset, so the old @action-based artefacts endpoint 404'd on deleted reports. The new nested viewset has to mirror that, otherwise a known UUID would bypass deletion. - Preserve the distinction between "github_name omitted" and "github_name explicitly empty" in PUT, so clients can clear a previously stored name. Previously `or None` collapsed both cases and silently carried over the prior value. - Drop dead `other_org` test variable and rename the test to match what it actually verifies (random UUID not in any org). Generated-By: PostHog Code Task-Id: 4b273054-e446-41d6-b4c0-bac3bd4cb309
Twixes
approved these changes
May 22, 2026
Member
Twixes
left a comment
Twixes
left a comment
There was a problem hiding this comment.
Did not take a hard look at the tests… but PUT mechanism and API implementation all rock solid.
Where's the Code PR?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Signal report artefacts are produced server-side only — the agentic Temporal pipeline writes
suggested_reviewers,priority_judgment,dismissal, and friends, and the artefact serializer was read-only. There was no way for a client to fix a mis-resolved reviewer, drop the wrong person, or add a teammate the agent missed.This PR exposes a narrow API to modify the suggested reviewers for a report. Mutation is locked to that single artefact type so other types (which are part of the agentic pipeline contract) cannot be hand-edited.
Changes
SignalReportArtefactViewSetat/api/projects/<team>/signals/reports/<report_id>/artefacts/supporting GET list, GET retrieve, and PUT update. Replaces the previous read-only@action(artefacts)onSignalReportViewSet.{ "content": [{ "github_login"?: str, "user_uuid"?: uuid, "github_name"?: str }, ...] }. Entries identify reviewers bygithub_login,user_uuid, or both — the server canonicalizes to a lowercasegithub_loginvia the existing org-member resolution helpers used by the?suggested_reviewers=filter.suggested_reviewers(covered by a parameterized test across every other type).relevant_commitsis preserved across the replace for reviewers that survive; new entries get[].task:writescope. Team / report scoping enforced viaTeamAndOrgViewSetMixin.help_texton artefact serializer fields and newSignalReportArtefactWriteSerializerso the OpenAPI spec → TypeScript / Zod / MCP pipeline picks the endpoint up cleanly.products/signals/frontend/generated/*andservices/mcp/src/api/generated.ts.Out of scope for this PR:
suggested_reviewersartefact from scratch (PUT requires an existing one).How did you test this code?
I am an agent (Claude / PostHog Code). I did not perform manual UI testing.
products/signals/backend/test/test_signal_report_artefact_api.pycovering full-replace, identity resolution (login / uuid / both),relevant_commitspreservation, dedupe, idempotency, empty-list clear, validation errors, 400 on every non-suggested-reviewers artefact type, team scoping, method restrictions, and a round-trip against the existing?suggested_reviewers=filter.ruff check+ruff formatclean across changed files.uv run ty checkclean.pytest --collect-onlydiscovers all 31 tests.bin/hogli build:openapiruns end-to-end and emits the expectedSignalReportArtefactWriteApi/SuggestedReviewerEntryWriteApitypes into the signals generated client and MCP generated types.Behaviour note worth flagging in review: the artefacts list endpoint is now standard DRF-paginated (
{count, next, previous, results}) rather than the bespoke{count, results}envelope the old@actionreturned. There are no existing consumers of the generated client for this URL.Publish to changelog?
do not publish to changelog
Docs update
skip-inkeep-docs
🤖 Agent context
SignalReportTaskViewSet), and the?suggested_reviewers=filter's UUID-to-GitHub-login resolution path. Reused those helpers (get_org_member_github_logins_by_user_uuid,resolve_org_github_login_to_users,normalized_github_logins_from_suggested_reviewer_artefacts,enrich_reviewer_dicts_with_org_members) rather than introducing a new resolution code path.suggested_reviewers-specific URL — leaves headroom to whitelist additional artefact types later without restructuring routes, while still failing closed today.github_loginoruser_uuidper entry, withuser_uuidwinning when both are supplied — mirrors how the existing list-filter already maps PostHog users to artefact content.is_suggested_reviewerannotation and?suggested_reviewers=jsonb-containment filter keep working without modification.@action(artefacts)with the new viewset'slist/retrieveto avoid URL collision; this incidentally surfaces the read endpoints in the OpenAPI spec for the first time. List response shape is now standard DRF pagination, which matches the generated client types.