Skip to content

feat: add resolved_round to comments (mirror crit timeline)#175

Merged
tomasz-tomczyk merged 2 commits intomainfrom
feat/173-resolved-round-on-comments
May 5, 2026
Merged

feat: add resolved_round to comments (mirror crit timeline)#175
tomasz-tomczyk merged 2 commits intomainfrom
feat/173-resolved-round-on-comments

Conversation

@tomasz-tomczyk
Copy link
Copy Markdown
Owner

@tomasz-tomczyk tomasz-tomczyk commented May 5, 2026

Summary

  • Adds resolved_round (nullable integer) to the comments table to mirror crit's round-faithful timeline data model
  • On resolve (false → true), stamps the review's current review_round; on unresolve (true → false), clears to nil
  • Threads the field through CLI share imports (replace_comments, create_changeset) and surfaces it in API + export payloads
  • null semantics: absent = never/currently unresolved; integer = resolved at round N

Review

  • Code review: passed
  • Parity audit: matches crit's stamping semantics (session.go:909, comment_cli.go:78)
  • Integration tests: TestShareSyncResolvedRoundMapping (crit) + extended api_controller_test (crit-web)

Test plan

Closes #173

🤖 Generated with Claude Code

Stamp resolved_round with the review's current review_round on
false → true transitions; clear to nil on true → false. Mirrors
crit's round-faithful timeline so /api/reviews/:token consumers can
filter resolution state per round.

The field is nullable (NULL = never/currently unresolved). Threaded
through CLI share imports and surfaced in API + export payloads.

Closes #173
Adds a test-only POST /api/reviews/:token/seed-resolve/:comment_id
endpoint so the crit ↔ crit-web share integration suite can exercise
the resolve/unresolve flow end-to-end and assert the resolved_round
contract surfaced in the public API. Extends api_controller_test
to verify resolved_round round-trips through POST /reviews and the
export endpoint.
@codecov
Copy link
Copy Markdown

codecov Bot commented May 5, 2026

Codecov Report

❌ Patch coverage is 35.29412% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.14%. Comparing base (b55f907) to head (6007738).

Files with missing lines Patch % Lines
lib/crit_web/controllers/api_controller.ex 0.00% 9 Missing ⚠️
lib/crit/reviews.ex 83.33% 1 Missing ⚠️
lib/crit_web/router.ex 0.00% 1 Missing ⚠️

❌ Your patch check has failed because the patch coverage (35.29%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #175      +/-   ##
==========================================
- Coverage   79.56%   79.14%   -0.43%     
==========================================
  Files          56       56              
  Lines        1801     1817      +16     
==========================================
+ Hits         1433     1438       +5     
- Misses        368      379      +11     
Flag Coverage Δ
unit 79.14% <35.29%> (-0.43%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@tomasz-tomczyk tomasz-tomczyk merged commit 952db51 into main May 5, 2026
3 of 4 checks passed
@tomasz-tomczyk tomasz-tomczyk deleted the feat/173-resolved-round-on-comments branch May 5, 2026 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add resolved_round to comments (mirror crit timeline)

1 participant