Refactor opr file stucture and code#2345
Draft
jh-RLI wants to merge 41 commits into
Draft
Conversation
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Move parse_keys, sort_in_category and get_all_field_descriptions out of TablePeerReviewView as pure, unit-testable functions. Drops the unused 'table' arg from sort_in_category. Behavior-preserving. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Moves the reviewer/contributor POST branching out of the views 1:1 (behavior-preserving). Raises ContributorNotFoundError for the no-table-holder case. Duplicate-PeerReviewManager bug intentionally left for S3/S4. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
TablePeerReviewView/TablePeerRreviewContributorView POST handlers now parse the request and delegate to ReviewService; metadata shaping uses the extracted serializer. Removes the now-unused inline methods and imports. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pins current behavior of merge_field_reviews, recursive_update and process_review_data (17 SimpleTestCase tests, no DB). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pins set_next_reviewer/whos_turn, save vs update paths, the contributor==reviewer guard, load and days_open (14 TestCase tests). Documents the duplicate-manager bug. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
11 SimpleTestCase tests for parse_keys, sort_in_category and get_all_field_descriptions (previously untested). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two OEDB-free tests covering contributor submit (merge + turn toggle) and save (SAVED, no toggle). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds the ReviewRound model (append-only per-turn log; opr FK, sequence, role, actor, action, field_reviews, sets_finished) that PeerReview.review will be projected from. Also fixes PeerReview.save() to use get_or_create for the PeerReviewManager so a review keeps exactly one manager (was creating a new one on every call). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pure functions: project_review/build_reviews_from_rounds rebuild the review JSON from rounds with fieldReview always a list; compute_round_delta keeps only new contributions vs prior rounds; reconstruct_rounds_from_review rebuilds rounds from a legacy review blob (for backfill). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
submit/finish now compute the per-turn delta, append a ReviewRound, and rebuild PeerReview.review via project_review. Drops merge_field_reviews and the payload-extend hack. _apply_finished merges the projected values into the live metadata + snapshot. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reviewer-only: all fields accepted -> offer Finish; any suggestion/deny -> keep Submit enabled (ping-pong). Stops disabling submit and showing the badge UI prematurely. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor option buttons now reveal the inputs, the response is recorded into current_review (was lost before), the field is colored/stated, the description resolves, and prior in-session responses restore. Mirrors the reviewer; dedup is a Phase 3 task. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fixes the long-standing double-R typo in the contributor view class name. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
delete_peer_review_simple_view now delegates to the single delete_peer_review implementation, with an English docstring; drops the now-unused PeerReview import. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Badge policy is isolated behind a BadgeStrategy seam (DEFAULT_BADGE_STRATEGY / strategy= arg) so the calculation and which-fields-matter can change without touching callers. Default cumulative_tier_strategy derives tiers from the schema badge attributes; reviewer choice overrides the auto-suggestion; normalize_badge unifies platin/Platinum/PLATINUM. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
_apply_finished now resolves the final badge (reviewer choice else auto-suggest) from the merged metadata and persists it to metadata['review']['badge'] on both the live table and the snapshot. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Framework-free observable store: the future single source of truth (config, role, field inventory, review datamodel, fieldState, selection). Additive; existing modules unchanged until incrementally adopted. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pure derivations over store state: progress, reviewer completeness, reviewer-has-changes, and contributor targets (the reviewer-flagged fields). One place for the two distinct actor behaviors. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The dataview 'Latest review completed' card reads PeerReview.review['badge']; _apply_finished now records the badge there (not only in the table metadata), so a finished review shows its badge instead of 'None'. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Summary of the discussion
During development for the Open Peer Review feature we found that the current architecture is to complex and hard to maintain. This PR streamlines the full OPR code in a full refactor while keeping the functionality.
Type of change (CHANGELOG.md)
Changes
(#)
Documentation updates
(#)
Workflow checklist
Automation
Closes #
PR-Assignee
CONTRIBUTING.md
CHANGELOG.md
mkdocs
Reviewer
Reviewer Guidelines