feat(issue-discovery): precise post-merge edit detection upgrade#434
Conversation
7a98b6a to
4adaf12
Compare
| authorAssociation | ||
| userContentEdits(last: 1) { | ||
| nodes { editedAt } | ||
| } |
There was a problem hiding this comment.
To get last body edit, first:1 will return newest edit time, proved by manual testing
| nodes { | ||
| ... on RenamedTitleEvent { createdAt } | ||
| } | ||
| } |
There was a problem hiding this comment.
To get last title edit time of the issue.
5ac412c to
a4af442
Compare
|
@anderdc Would you review this PR? |
anderdc
left a comment
There was a problem hiding this comment.
A couple things to address:
Delete the spec file
issue_discovery/issue-discovery-rewards.md is a planning doc that was used to guide implementation. It shouldn't live in the repo long-term. Please delete it in this PR or a follow-up.
Clean up the tests
The tests cover the right scenarios but need some cleanup:
_make_issuehasdatetime = Nonedefaults — these should beOptional[datetime] = None- Lazy import of
_DiscovererDatainside_run()— no reason for this in a test file, just import at the top _merged_at()is a function returning a constant — just make it a module-level constant- Make sure the tests are actually exercising the new code path — right now they test
_collect_issues_from_prs(a private function) directly. That's fine for now, but double check the assertions are meaningful and not just passing because of default values or skipped branches
|
failing ci |
01614b9 to
f5c2e94
Compare
|
@anderdc Updated the PR. Would you review again? Thanks |
Replace the noisy updated_at proxy with timeline-based detection for body and title edits only. updated_at fires on any activity (bot comments, labels, reactions), silently demoting legitimate solved issues to closed_count in active repos. - GraphQL: fetch userContentEdits + RenamedTitleEvent alongside existing closingIssuesReferences (no extra round trip) - Issue.body_or_title_edited_at: max(last body edit, last rename) - scoring.py: anti-gaming check now uses body_or_title_edited_at - Tests: benign updated_at bump ignored, real body edit demotes, pre-merge edits ignored fix fix fix fix
userContentEdits is reverse-chronological on GitHub, so last:1 returned the oldest body edit and the check never fired on subsequent edits. first:1 returns the newest. timelineItems is chronological, so last:1 stays correct for title renames.
- Optional[datetime] on _make_issue defaults (fixes pyright) - Hoist _DiscovererData / defaultdict imports to module level - Replace _merged_at() function with MERGED_AT constant - Delete issue_discovery/issue-discovery-rewards.md planning doc
bd162f0 to
f297b9d
Compare
anderdc
left a comment
There was a problem hiding this comment.
Solid upgrade. The updated_at proxy was a known false-positive source (bot comments, labels, reactions demoting legitimate solved issues). The timeline-based approach is precise and well-tested.
- GraphQL pagination is correct:
first:1for reverse-chronologicaluserContentEdits,last:1for chronologicaltimelineItems - Null handling is robust throughout the parsing chain
- Tests cover the three key cases: benign activity ignored, real post-merge edit caught, pre-merge edit ignored
- CI green: pyright, ruff, tests all pass
Closes #402
Summary
Replaces the
issue.updated_atanti-gaming logic in issue discovery scoring with precise timeline-based detection for body and title edits only. The old check false-positived on any activity (bot comments, label changes, reactions, stale-bots), silently demoting legitimate solved issues toclosed_countand unfairly cutting miner credibility in active repos.Body edit and title edit both should be checked.
This is because title and body are connected.
If title change allowed, it's possible to confuse the reviewer by changing title.
From the purpose of this feature from doc: https://docs.gittensor.io/issue-discovery.html, title is important to define scope, it's possible to inflate by just changing title.
Related Issues
Closes #402
Type of Change
Testing
Added:
tests/validator/test_issue_discovery_post_merge_edit.py— 3 cases:updated_atbump after merge (bot activity) → issue stayssolved.closed, no score.solved.Results: 3/3 new tests pass. Broader existing suite (66 validator tests) still green. Pre-existing
tree_sitterimport errors in unrelated test modules are not affected by this change.Manual testing
Before adding comment

After adding commment

After changing title

After chaing body

Checklist
body_or_title_edited_atis not persisted to DB — fine today since scoring re-fetches from GraphQL each run, but worth revisiting if any code path reconstructsIssuefrom storage)updated_atcolumn preserved)issue_discovery/issue-discovery-rewards.mdstill describes theupdated_atproxy as current behavior; should be updated in a follow-up doc PR or this one if preferred