Skip to content

Normalize response shapes - #8

Merged
anderdc merged 1 commit into
entrius:testfrom
seroperson:response-cleanup
May 10, 2026
Merged

Normalize response shapes#8
anderdc merged 1 commit into
entrius:testfrom
seroperson:response-cleanup

Conversation

@seroperson

@seroperson seroperson commented May 6, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR implements transformation logic which previously was hold in client.

After this PR lands, we can safely simplify entrius/gittensor#914 to make client contain only plain response models.

Related Issues

entrius/gittensor#915

Type of Change

  • Bug fix
  • New feature
  • Refactor
  • Documentation
  • Other (describe below)

Checklist

  • I have read the Contributing Guide
  • Code builds without errors
  • New and existing tests pass (if applicable)
  • Documentation updated (if applicable)
  • No unnecessary dependencies added

@anderdc anderdc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This PR normalizes response shapes but stops short of gating the MERGED-without-merged_at corruption shape. Consumers (gittensor #925, oss_contributions/mirror/scoring.py:192) are adding parallel defensive gates downstream that belong here instead.

Three additions:

  • miners.service.ts solving_pr subquery (~line 200): extend AND sp.author_github_id IS NOT NULL with AND NOT (sp.state = 'MERGED' AND sp.merged_at IS NULL). Symmetric to the null-author gate already in this PR.
  • packages/db/02_pull_requests.sql: add CHECK (state != 'MERGED' OR merged_at IS NOT NULL) so future writes can't persist the corrupted shape regardless of upstream payload.
  • packages/das/src/webhook/handlers/pull-request.handler.ts:36-39: gate state: 'MERGED' on pr.merged && pr.merged_at together so the handler can't trip the new CHECK constraint on a partial GitHub payload.

@seroperson

seroperson commented May 6, 2026

Copy link
Copy Markdown
Contributor Author

This PR normalizes response shapes but stops short of gating the MERGED-without-merged_at corruption shape. Consumers (gittensor #925, oss_contributions/mirror/scoring.py:192) are adding parallel defensive gates downstream that belong here instead.

Three additions:

  • miners.service.ts solving_pr subquery (~line 200): extend AND sp.author_github_id IS NOT NULL with AND NOT (sp.state = 'MERGED' AND sp.merged_at IS NULL). Symmetric to the null-author gate already in this PR.
  • packages/db/02_pull_requests.sql: add CHECK (state != 'MERGED' OR merged_at IS NOT NULL) so future writes can't persist the corrupted shape regardless of upstream payload.
  • packages/das/src/webhook/handlers/pull-request.handler.ts:36-39: gate state: 'MERGED' on pr.merged && pr.merged_at together so the handler can't trip the new CHECK constraint on a partial GitHub payload.

Done. I'm unsure about editing 02_pull_requests.sql, but I see it's not exactly migration, so I guess that's fine.

P.S. May we afterwards re-open the underlying issue in gittensor? entrius/gittensor#915

@anderdc

anderdc commented May 8, 2026

Copy link
Copy Markdown
Collaborator

PRs should target test branch

@seroperson
seroperson changed the base branch from main to test May 8, 2026 19:40
@seroperson

Copy link
Copy Markdown
Contributor Author

@anderdc Rebased + squashed

@anderdc

anderdc commented May 10, 2026

Copy link
Copy Markdown
Collaborator

Thank you for the response hardening, but I still think 915 doesn't need to be opened, I'm perfectly fine with the current boilerplate/implementation instead of the switch

@anderdc
anderdc merged commit 433d2b1 into entrius:test May 10, 2026
2 checks passed
@anderdc anderdc mentioned this pull request May 12, 2026
4 tasks
anderdc pushed a commit that referenced this pull request May 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants