[verified] Support explicit review-only provider counters
This commit is contained in:
@@ -58,6 +58,17 @@ Shape example only, **not a claim about this deployment**:
|
|||||||
}
|
}
|
||||||
```
|
```
|
||||||
|
|
||||||
|
For `inline.components`, the explicit projection modes are `["inline"]`,
|
||||||
|
`["reviews"]`, and `["reviews", "inline"]`. The review-only mode projects
|
||||||
|
only the number of reviews into `review_comments`; it does not add per-review
|
||||||
|
inline comments. The inline-only mode projects collected per-review inline
|
||||||
|
comments; the combined mode projects both. These are caller-supplied policies:
|
||||||
|
select one only from independently verified deployed-provider semantics, never
|
||||||
|
automatically detect a mode or choose one to fit observed counters. Both
|
||||||
|
`per_review` and `per_review_or_pr_count` retain the same explicit fallback
|
||||||
|
policy; per-review counters, complete collection pagination, duplicate IDs and
|
||||||
|
record cardinality remain checked.
|
||||||
|
|
||||||
`per_review` requires every exact-review comment endpoint. Only the explicit
|
`per_review` requires every exact-review comment endpoint. Only the explicit
|
||||||
`per_review_or_pr_count` capability permits endpoint-unavailable 404/405 fallback.
|
`per_review_or_pr_count` capability permits endpoint-unavailable 404/405 fallback.
|
||||||
Permission errors, redirects, malformed JSON and conflicting metadata never
|
Permission errors, redirects, malformed JSON and conflicting metadata never
|
||||||
@@ -72,7 +83,11 @@ and `html_url` as HTML URLs, and PR-Issue `pull_request.html_url` without a nati
|
|||||||
`pull_request.url`. Require the actual native relation URL; do not invent a
|
`pull_request.url`. Require the actual native relation URL; do not invent a
|
||||||
missing field. This source pin is fixture/protocol evidence, not a claim about
|
missing field. This source pin is fixture/protocol evidence, not a claim about
|
||||||
the deployment version. The caller's independent provider contract still binds
|
the deployment version. The caller's independent provider contract still binds
|
||||||
that version. In `per_review` mode only, an omitted `review_comments` is accepted
|
that version. In Gitea `v1.27.3`, `services/convert/pull.go` delegates the PR
|
||||||
|
review-comment count to `models/issues/pull.go`'s `GetReviewCommentsCount`,
|
||||||
|
which filters `CommentTypeReview`; this is source evidence for that version's
|
||||||
|
review-only projection, not a blanket claim for all versions or a live-gate
|
||||||
|
success. In `per_review` mode only, an omitted `review_comments` is accepted
|
||||||
as zero only when complete per-review records and their counters prove a zero
|
as zero only when complete per-review records and their counters prove a zero
|
||||||
projection. Explicit null, nonzero projection or counts-only fallback with a
|
projection. Explicit null, nonzero projection or counts-only fallback with a
|
||||||
missing scalar is rejected. Raw pagination Link evidence is preserved and
|
missing scalar is rejected. Raw pagination Link evidence is preserved and
|
||||||
|
|||||||
@@ -147,7 +147,7 @@ class ReworkReadbackAdapter:
|
|||||||
if (set(inline) != {'mode','field','components'}
|
if (set(inline) != {'mode','field','components'}
|
||||||
or inline.get('mode') not in ('per_review','per_review_or_pr_count')
|
or inline.get('mode') not in ('per_review','per_review_or_pr_count')
|
||||||
or inline.get('field') != 'review_comments'
|
or inline.get('field') != 'review_comments'
|
||||||
or inline.get('components') not in (['inline'], ['reviews','inline'])):
|
or inline.get('components') not in (['inline'], ['reviews'], ['reviews','inline'])):
|
||||||
raise ReadbackError('invalid inline count declaration')
|
raise ReadbackError('invalid inline count declaration')
|
||||||
return copy.deepcopy(contract)
|
return copy.deepcopy(contract)
|
||||||
|
|
||||||
@@ -347,7 +347,9 @@ class ReworkReadbackAdapter:
|
|||||||
for row in collected['rows']:
|
for row in collected['rows']:
|
||||||
if row['id'] in inline_ids: raise ReadbackError('duplicate cross-review inline ID')
|
if row['id'] in inline_ids: raise ReadbackError('duplicate cross-review inline ID')
|
||||||
inline_ids.add(row['id'])
|
inline_ids.add(row['id'])
|
||||||
computed = inline + (len(raw['reviews']['rows']) if 'reviews' in self.provider['inline']['components'] else 0)
|
parts = self.provider['inline']['components']
|
||||||
|
computed = ((inline if 'inline' in parts else 0)
|
||||||
|
+ (len(raw['reviews']['rows']) if 'reviews' in parts else 0))
|
||||||
field = self.provider['inline']['field']
|
field = self.provider['inline']['field']
|
||||||
if field not in raw['pr']:
|
if field not in raw['pr']:
|
||||||
if (self.provider['inline']['mode'] != 'per_review' or fallback or computed != 0
|
if (self.provider['inline']['mode'] != 'per_review' or fallback or computed != 0
|
||||||
|
|||||||
@@ -105,6 +105,74 @@ class ReworkReadbackTests(unittest.TestCase):
|
|||||||
self.underlying['comments'] += 1
|
self.underlying['comments'] += 1
|
||||||
self.pr['updated_at'] = self.underlying['updated_at'] = T1
|
self.pr['updated_at'] = self.underlying['updated_at'] = T1
|
||||||
|
|
||||||
|
def test_review_only_projection_preserves_inline_records_without_counting_them(self):
|
||||||
|
module = self.load('rework_readback')
|
||||||
|
contract = copy.deepcopy(self.contract)
|
||||||
|
contract['inline']['components'] = ['reviews']
|
||||||
|
adapter = module.ReworkReadbackAdapter(self.request, origin=ORIGIN+'/api/v1',
|
||||||
|
credential_source='$HERMES_HOME/.env:GITEA_TOKEN', actor_id=42, provider_contract=contract)
|
||||||
|
self.pr['review_comments'] = 1
|
||||||
|
self.reviews[0]['comments_count'] = 0
|
||||||
|
snapshot = adapter.snapshot(self.expected)
|
||||||
|
self.assertEqual(snapshot['normalized_counts']['inline'], 0)
|
||||||
|
self.reviews[0]['comments_count'] = 1
|
||||||
|
self.inline = [{'id':801,'body':'inline','created_at':T0,'updated_at':T0,
|
||||||
|
'user':{'id':33,'login':'gitea-review-agent'}}]
|
||||||
|
self.pr['review_comments'] = 1
|
||||||
|
self.pr['comments'] = self.underlying['comments'] = 2
|
||||||
|
snapshot = adapter.snapshot(self.expected)
|
||||||
|
self.assertEqual(snapshot['normalized_counts']['inline'], 1)
|
||||||
|
|
||||||
|
def test_review_only_projection_rejects_counter_mismatch(self):
|
||||||
|
module = self.load('rework_readback')
|
||||||
|
contract = copy.deepcopy(self.contract)
|
||||||
|
contract['inline']['components'] = ['reviews']
|
||||||
|
adapter = module.ReworkReadbackAdapter(self.request, origin=ORIGIN+'/api/v1',
|
||||||
|
credential_source='$HERMES_HOME/.env:GITEA_TOKEN', actor_id=42, provider_contract=contract)
|
||||||
|
self.reviews[0]['comments_count'] = 1
|
||||||
|
self.inline = [{'id':801,'body':'inline','created_at':T0,'updated_at':T0,
|
||||||
|
'user':{'id':33,'login':'gitea-review-agent'}}]
|
||||||
|
self.pr['review_comments'] = 0
|
||||||
|
with self.assertRaisesRegex(ValueError, 'inline provider counter crosscheck failed'):
|
||||||
|
adapter.snapshot(self.expected)
|
||||||
|
|
||||||
|
def test_inline_component_contract_validation_remains_strict(self):
|
||||||
|
module = self.load('rework_readback')
|
||||||
|
for parts in ([], None, 'reviews', ['inline','inline'], ['reviews','reviews'],
|
||||||
|
['unknown'], ['reviews','unknown'], ['reviews','inline','reviews']):
|
||||||
|
contract = copy.deepcopy(self.contract)
|
||||||
|
contract['inline']['components'] = parts
|
||||||
|
with self.subTest(components=parts), self.assertRaises(ValueError):
|
||||||
|
module.ReworkReadbackAdapter(self.request, origin=ORIGIN+'/api/v1',
|
||||||
|
credential_source='$HERMES_HOME/.env:GITEA_TOKEN', actor_id=42, provider_contract=contract)
|
||||||
|
|
||||||
|
def test_review_only_projection_retains_inline_evidence_rejections(self):
|
||||||
|
module = self.load('rework_readback')
|
||||||
|
contract = copy.deepcopy(self.contract)
|
||||||
|
contract['inline']['components'] = ['reviews']
|
||||||
|
adapter = module.ReworkReadbackAdapter(self.request, origin=ORIGIN+'/api/v1',
|
||||||
|
credential_source='$HERMES_HOME/.env:GITEA_TOKEN', actor_id=42, provider_contract=contract)
|
||||||
|
self.reviews[0]['comments_count'] = 1
|
||||||
|
self.inline = [{'id':801,'body':'inline','created_at':T0,'updated_at':T0,
|
||||||
|
'user':{'id':33,'login':'gitea-review-agent'}}]
|
||||||
|
self.pr['review_comments'] = 2
|
||||||
|
with self.assertRaises(ValueError): adapter.snapshot(self.expected)
|
||||||
|
self.pr['review_comments'] = 1
|
||||||
|
self.inline = []
|
||||||
|
with self.assertRaisesRegex(ValueError, 'per-review inline count mismatch'):
|
||||||
|
adapter.snapshot(self.expected)
|
||||||
|
self.inline = [{'id':801,'body':'inline','created_at':T0,'updated_at':T0,
|
||||||
|
'user':{'id':33,'login':'gitea-review-agent'}},
|
||||||
|
{'id':801,'body':'duplicate','created_at':T0,'updated_at':T0,
|
||||||
|
'user':{'id':33,'login':'gitea-review-agent'}}]
|
||||||
|
with self.assertRaisesRegex(ValueError, 'duplicate'):
|
||||||
|
adapter.snapshot(self.expected)
|
||||||
|
self.inline = [{'id':801,'body':'inline','created_at':T0,'updated_at':T0,
|
||||||
|
'user':{'id':33,'login':'gitea-review-agent'}}]
|
||||||
|
self.overrides = {'/repos/acme/demo/pulls/9/reviews/301/comments?page=1&limit=50':
|
||||||
|
(self.inline, {'X-Total-Count':'2'}, 200)}
|
||||||
|
with self.assertRaises(ValueError): adapter.snapshot(self.expected)
|
||||||
|
|
||||||
def test_snapshot_rejects_wrong_provider_bindings(self):
|
def test_snapshot_rejects_wrong_provider_bindings(self):
|
||||||
cases = [('repo',['id'],True), ('repo',['full_name'],'elsewhere/demo'),
|
cases = [('repo',['id'],True), ('repo',['full_name'],'elsewhere/demo'),
|
||||||
('repo',['html_url'],'https://foreign.invalid/acme/demo'),
|
('repo',['html_url'],'https://foreign.invalid/acme/demo'),
|
||||||
|
|||||||
Reference in New Issue
Block a user