Add proposal conflict detector for review UI (#1022)#1040
Add proposal conflict detector for review UI (#1022)#1040Chris0Jeky wants to merge 20 commits intomainfrom
Conversation
Part of #1022: tone-classified conflict rows for proposal review. ConflictTone has Warn/Info/Ok values mapping to frontend color semantics. ConflictRow is an immutable value object with Tone, Key, and Value.
Part of #1022: IProposalConflictDetector interface and implementation. Detects warn/info/ok conditions for proposals including stale data, WIP limits, duplicate proposals, high risk, webhooks, comments, and multi-op conflicts. Includes ConflictRowDto for API serialization.
Part of #1022: wire ProposalConflictDetector into API layer. New endpoint delegates to conflict detector with auth from JWT claims. DI registration added in ApplicationServiceRegistration.
Part of #1022: 12 tests covering construction validation, equality, hashing, toString, and invalid tone rejection.
Part of #1022: 31 tests covering all conflict detection scenarios. Tests: authorization (5), no-conflicts ok (1), stale data (2), WIP limit (2), duplicate proposals (2), high risk (4), webhooks (3), comments (2), multi-op (2), positive signals (2), sorting (1), combinations (1), expired proposals (1), edge cases (3).
Adversarial self-review fix: the original implementation tried to detect already-flagged cards by searching for card IDs in warning message text. This failed because stale-data messages contain card titles, not IDs. Now uses explicit HashSet<Guid> tracking for flagged card and column IDs.
Adversarial Self-ReviewIssues Found and Fixed1. Positive signal tracking bug (FIXED in f1b407d)
2. Test timing false positives (FIXED in 923e21c)
3. Moq setup order override (FIXED in 923e21c)
Authorization Review
Graceful Degradation
Remaining Acceptable Risks
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a conflict detection system for automation proposals, featuring a new API endpoint and a service that analyzes proposals for risks like stale data and WIP limit violations. Feedback identifies a logic error in target column extraction where operations lacking parameters are skipped and suggests caching entities to eliminate redundant database lookups within the detection process.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1b407db8c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Adversarial Review: PR #1040 — All 5 bot findings addressedReviewed all changed files and addressed every bot comment. Fixes pushed. Findings and fixes applied1. HIGH (Gemini): Column detection skipped for parameterless ops (line 343) 2. MEDIUM (Gemini): Redundant entity fetching in AddPositiveSignalsAsync (line 277) 3. P1 (Codex): False warnings for create-card operations (line 327) 4. P2 (Codex): Only checks latest proposal for duplicates (line 177) 5. P2 (Codex): Unsafe JSON parsing (line 360) Verification
|
- Move TargetType check before Parameters guard in GetTargetColumnIds so column-targeted ops without parameters are detected - Add entity caches (cardCache, columnCache) to avoid redundant DB lookups across CheckStaleData, CheckWipLimit, and AddPositiveSignals - Exclude create actions from GetDistinctCardTargetIds to prevent false stale/missing warnings for not-yet-created cards - Add GetPendingByOperationTargetAsync to find ALL pending proposals for a target card, not just the latest - Check ValueKind before GetString() on JSON properties to avoid InvalidOperationException on non-string columnId values
New method returns all pending-review proposals targeting a given entity, replacing the single-result GetLatestByOperationTargetAsync for duplicate-proposal detection.
…itory Queries for all PendingReview proposals that have operations targeting a given entity type and ID. Filters by status at the DB level.
- Switch duplicate-proposal mocks to GetPendingByOperationTargetAsync returning lists instead of single entities - Add test for create-card operations not producing false warnings - Add test for column-targeted ops with no parameters being detected - Add test for non-string columnId in JSON not throwing
All IAutomationProposalRepository concrete fakes in Api.Tests now implement the new interface method (throws NotSupportedException).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d76ce47f1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Fresh Adversarial Review (Round 3)IMPORTANT1. Fix: Add 2. Authorization duplicated between controller and service (Confidence: 83) Fix: Use 3. WIP limit uses current state, not projected post-execution state (Confidence: 80) Verified Non-Issues
|
ProposalConflictDetector.CheckActiveCommentsAsync was calling GetByCardIdAsync which includes AuthorUser + Mentions + MentionedUser navigation properties, but only used .Count() on the result. For N cards this materialized N full entity graphs unnecessarily. Add CountByCardIdAsync(cardId) to ICardCommentRepository that executes SELECT COUNT(*) without includes, and use it in the conflict detector.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 390b966490
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Update STATUS.md, TESTING_GUIDE.md, IMPLEMENTATION_MASTERPLAN.md, MANUAL_TEST_CHECKLIST.md, GOLDEN_PRINCIPLES.md, and ISSUE_EXECUTION_GUIDE.md to reflect round 3 adversarial review findings and fixes across all 10 Paper backend gap PRs (#1031-#1040): 3 critical bugs fixed (seal-day race, tomorrow-note phantom data, card-history case mismatch), 8 important issues resolved, test counts updated (~460→~465), manual test checklist expanded with Today dossier and Review deep-dive API spot checks.
Resolve conflicts from #1032 (streak), #1033 (side-effects), #1038 (similar-past) merges: - AutomationProposalsController: keep conflicts endpoint alongside side-effects, diff, and similar-past endpoints; constructor accepts all services - ApplicationServiceRegistration: register both ProposalConflictDetector and SideEffectAnalyzer - WorkerResilienceTests: fake repo implements both GetPendingByOperationTargetAsync and GetTerminalByActionTypeAsync - smoke.spec.ts: take main's more robust addColumn helper (waits for API response and dialog close)
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12d09c9594
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Separate mangled GetProposalConflicts and GetProposalHistory methods into distinct endpoints.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e55fc6b38b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d97861d1b8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 40dba4f847
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62f202ce13
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
2 participants
Summary
ConflictToneenum (Warn,Info,Ok) andConflictRowvalue object to Domain layerIProposalConflictDetectorinterface andProposalConflictDetectorservice that analyzes proposals and produces tone-classified conflict rowsGET /api/automation/proposals/{id}/conflictsendpoint with authorizationApplicationServiceRegistrationConflict detection rules
Warn scenarios:
Info scenarios:
Ok scenarios (positive signals):
Tests
ConflictRow(construction, validation, equality, hashing)ProposalConflictDetectorcovering all warn/info/ok scenarios, authorization, sorting, combinations, edge cases (invalid GUIDs, null targets, malformed JSON), and expired proposal handlingArchiveConflictDetectortests continue to passAdversarial self-review findings (fixed)
HashSet<Guid>tracking for flagged card and column IDs.CreatedAt/UpdatedAttoDateTimeOffset.UtcNow, so creating a Card entity after a Proposal entity in test code would always makecard.UpdatedAt > proposal.CreatedAt, falsely triggering stale-data warnings. Fixed by restructuring tests to create card entities before proposals.SetupEmptySecondaryChecksusedIt.IsAny<string>()forGetLatestByOperationTargetAsync, which overrode specific card-ID setups made earlier. Fixed by inlining secondary check setup in the duplicate-proposal test.Remaining risks (acceptable)
card.UpdatedAt > proposal.CreatedAtcomparison usesDateTimeOffsetwith tick-level precision. In production, proposal creation and card fetching happen at clearly different times, so sub-tick races are not a concern.Closes #1022
Test plan
dotnet build backend/Taskdeck.sln -c Releasepassesdotnet test --filter "FullyQualifiedName~Conflict"-- all 147 tests pass (37 domain + 83 application + 27 api)