Feat/merge review staleness - #259
Open
AlexAxthelm wants to merge 3 commits into
Open
Conversation
When an overlapping merge candidate is approved, a member of another pending
candidate gets merged away (repointed). Previously that candidate's Approve was
guaranteed to fail with a message interpolating repr() on ORM objects
("Repointed: [<...object at 0x...>]") — no id, no pointer to where the resource
went — and the only exit was Deny, recording a review judgment no one made.
Surface the move instead: resolve repointed member ids to their terminal
resource and show the curator where each one went. Completes AC 3 of STIT-418
Backend:
- ResourceModel._parent_tree_cte now emits (origin_id, id) so a batch keeps
each input mapped to its own root instead of returning an unkeyed set of
roots; add root_id_by_resource_id(), a batched dict-returning resolver.
get_root/_root_select are unchanged (documented single-id).
- Add RepointedResourceView and a repointed_resources list on MergeCandidateView
(plain field, not computed_field, so the detail view's model_dump rebuild
keeps it). Populated only for PENDING candidates via _resolve_candidates,
which does a single root lookup over the union of member ids (the queue is
unpaginated, so per-candidate resolution would be an N+1).
- Replace the repr() merge-guard message with a shared repointed_merge_error()
that names each id and its terminal target ("resource 102 is now resource
301"), wired into both the merge-candidate and write-path guards.
Frontend:
- New utils/mergeCandidateStaleness.js: the single reader of repointed_resources,
degrading to "not stale" when the field is absent.
- Merge review pane shows a warning banner naming each move (both ids linked),
hides the decision controls when stale (Approve would fail; Deny would record
a false judgment), and annotates a moved source resource as "(now 301)".
Tests:
- Backend: origin-column batch mapping, the N+1 batch guard, the PENDING-only
rule, the cross-candidate stale case end-to-end (queue + detail report the
move; re-approve 400s naming both ids with no "object at 0x", stays PENDING),
and fill the previously-empty apply_resource_merge guard stub.
- Frontend: stale banner + hidden controls, the facts annotation, a compat guard
(payload without the field renders as before), and a pure-function suite for
the staleness util.
The retire/requeue/SUPERSEDED-status flow is intentionally deferred (see PLAN.md)
until the stale-candidate dead end actually blocks a reviewer, keeping this PR
purely additive and rollback-safe.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
AlexAxthelm
force-pushed
the
feat/merge-review-staleness
branch
from
August 28, 2026 16:20
0abc86e to
3a61211
Compare
CD summary
|
| service | url | fqdn |
|---|---|---|
| api | open | pr-0259-api.purplegrass-c07d0a94.westus2.azurecontainerapps.io |
| entity-linkage | open | pr-0259-el.purplegrass-c07d0a94.westus2.azurecontainerapps.io |
| frontend | https://witty-mushroom-017a3dc1e-259.westus2.1.azurestaticapps.net | |
| stitch-llm | open | pr-0259-llm.purplegrass-c07d0a94.westus2.azurecontainerapps.io |
Database (1)
| db_name | postgres_host | postgres_port | postgres_db |
|---|---|---|---|
| pr_0259 | stitch-dev.postgres.database.azure.com |
5432 |
pr_0259 |
Jobs (2)
| job | image | postgres_db | api_url | auth_mode |
|---|---|---|---|---|
| db-migrations | ghcr.io/rmi/stitch-api:pr-0259@sha256:6d9b4dc7ec697ec57a548f3ddd3785d3c8498e3b2b830fb8015757143d3e61a0 |
pr_0259 |
||
| seed | ghcr.io/rmi/stitch-seed:pr-0259@sha256:8132cd8b82c8eb988cbc9abec651f74940391fa52a7eb37efce3450b80df67c9 |
https://pr-0259-api.purplegrass-c07d0a94.westus2.azurecontainerapps.io/api/v1 |
stitch-client-bearer-token |
Images (4)
| build_time | commit_time | git_sha | image | image_digest |
|---|---|---|---|---|
| 2026-08-28T16:21:11Z | 2026-08-28T16:20:54Z | f7f223f | ghcr.io/rmi/stitch-api:pr-0259 |
ghcr.io/rmi/stitch-api:pr-0259@sha256:6d9b4dc7ec697ec57a548f3ddd3785d3c8498e3b2b830fb8015757143d3e61a0 |
| 2026-08-28T16:21:11Z | 2026-08-28T16:20:54Z | f7f223f | ghcr.io/rmi/stitch-entity-linkage:pr-0259 |
ghcr.io/rmi/stitch-entity-linkage:pr-0259@sha256:84e4db2403c9d16e701df08e14ce3bc7a1bc24b06d2da8aacba6c08f7ad6b18a |
| 2026-08-28T16:21:11Z | 2026-08-28T16:20:54Z | f7f223f | ghcr.io/rmi/stitch-seed:pr-0259 |
ghcr.io/rmi/stitch-seed:pr-0259@sha256:8132cd8b82c8eb988cbc9abec651f74940391fa52a7eb37efce3450b80df67c9 |
| 2026-08-28T16:21:11Z | 2026-08-28T16:20:54Z | f7f223f | ghcr.io/rmi/stitch-stitch-llm:pr-0259 |
ghcr.io/rmi/stitch-stitch-llm:pr-0259@sha256:f79d040427b9b0af22516f2c48b95aa61ca64b00986c24f513479b1d1e3fe9f7 |
…(STIT-418) - get_resolved loads the resource row once and, for the common non-repointed case, builds the entity from it directly instead of going through resolve_root_id + get() (which re-fetched the same id). A repointed id still resolves via resolve_root_id (which maps a broken chain to 404) and loads the terminal resource. - create_merge_candidate returns the view with an empty staleness resolution instead of running the root-lookup CTE: its members are validated as unrepointed immediately above, so the lookup could only ever return empty. (approve/deny short-circuit on their terminal status.) Co-Authored-By: Claude Opus 4.8 <[email protected]>
AlexAxthelm
requested review from
jdhoffa and
mbarlow12
and removed request for
mbarlow12
August 28, 2026 16:55
AlexAxthelm
marked this pull request as ready for review
August 28, 2026 16:56
CD summary
|
| service | url | fqdn |
|---|---|---|
| api | open | pr-0259-api.purplegrass-c07d0a94.westus2.azurecontainerapps.io |
| entity-linkage | open | pr-0259-el.purplegrass-c07d0a94.westus2.azurecontainerapps.io |
| frontend | https://witty-mushroom-017a3dc1e-259.westus2.1.azurestaticapps.net | |
| stitch-llm | open | pr-0259-llm.purplegrass-c07d0a94.westus2.azurecontainerapps.io |
Database (1)
| db_name | postgres_host | postgres_port | postgres_db |
|---|---|---|---|
| pr_0259 | stitch-dev.postgres.database.azure.com |
5432 |
pr_0259 |
Jobs (1)
| job | image | postgres_db |
|---|---|---|
| db-migrations | ghcr.io/rmi/stitch-api:pr-0259@sha256:cfa9e21f3f572cbb4c2726f226abdc53ed9a0d13e4879762c29efde8f59c96a7 |
pr_0259 |
Images (4)
| build_time | commit_time | git_sha | image | image_digest |
|---|---|---|---|---|
| 2026-08-28T17:01:20Z | 2026-08-28T17:01:00Z | 388fa91 | ghcr.io/rmi/stitch-api:pr-0259 |
ghcr.io/rmi/stitch-api:pr-0259@sha256:cfa9e21f3f572cbb4c2726f226abdc53ed9a0d13e4879762c29efde8f59c96a7 |
| 2026-08-28T17:01:19Z | 2026-08-28T17:01:00Z | 388fa91 | ghcr.io/rmi/stitch-entity-linkage:pr-0259 |
ghcr.io/rmi/stitch-entity-linkage:pr-0259@sha256:af706928bf511c0f0437fec2afcd36cbe3f1d0f3a0b9a0ab768f5c51781874dd |
| 2026-08-28T17:01:19Z | 2026-08-28T17:01:00Z | 388fa91 | ghcr.io/rmi/stitch-seed:pr-0259 |
ghcr.io/rmi/stitch-seed:pr-0259@sha256:85bb8793994aee34409b738bcab02b65f107239866e332cb4044c6a71268b371 |
| 2026-08-28T17:01:23Z | 2026-08-28T17:01:00Z | 388fa91 | ghcr.io/rmi/stitch-stitch-llm:pr-0259 |
ghcr.io/rmi/stitch-stitch-llm:pr-0259@sha256:ee03a011a1403b4a1a97c1dfc33eca2169a5b44ba810d02d0e08b04e96308546 |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
get_resolved still re-queries the same resource on the repointed path via resolve_root_id, which is an avoidable extra round-trip and undermines the intended perf improvement.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR extends the merge-review workflow to detect and surface “stale” merge candidates caused by overlapping candidates where one approval repoints a resource that another pending candidate still references. It adds an explicit repointed_resources signal to merge-candidate API views, blocks re-approval with clearer id-based error messages, and updates the Merge Review UI to warn reviewers and hide actions when a candidate has become invalid.
Changes:
- Backend: add
repointed_resourcesto merge-candidate views (computed in a batched root lookup for PENDING candidates) and improve merge guards to return id-based “resource X is now resource Y” errors. - Frontend: add a staleness utility and update the Merge Review pane to show a warning banner, annotate moved members, and hide Approve/Deny controls for stale candidates.
- Tests: add/extend unit + integration tests covering batch resolution, PENDING-only behavior, overlapping-candidate staleness, and merge guard messaging.
File summaries
| File | Description |
|---|---|
| deployments/stitch-frontend/src/utils/mergeCandidateStaleness.js | Adds a single utility to interpret repointed_resources into {isStale, moves}. |
| deployments/stitch-frontend/src/utils/mergeCandidateStaleness.test.js | Adds a pure-function test suite for the staleness utility. |
| deployments/stitch-frontend/src/pages/MergeCandidateReviewPage.jsx | Renders stale-candidate banner, annotates moved members, and hides decision controls when stale. |
| deployments/stitch-frontend/src/pages/MergeCandidateReviewPage.test.jsx | Adds UI tests for stale banner, hidden controls, and facts annotations. |
| deployments/api/src/stitch/api/entities.py | Introduces RepointedResourceView and adds repointed_resources to MergeCandidateView. |
| deployments/api/src/stitch/api/db/og_field_resource_actions.py | Improves repointed-merge error messaging and optimizes get_resolved read path. |
| deployments/api/src/stitch/api/db/model/resource.py | Adds batched root_id_by_resource_id and augments the recursive CTE to preserve origin-to-root mapping. |
| deployments/api/src/stitch/api/db/merge_candidate_actions.py | Computes and propagates repointed_resources for candidates (batched for list endpoint; resolved for detail/actions). |
| deployments/api/tests/test_merge_candidate_actions.py | Updates unit tests and adds tests for batched resolution and terminal-status skip logic. |
| deployments/api/tests/routers/test_merge_candidates_integration.py | Adds end-to-end integration test for overlapping-approval staleness and id-based 400 messaging. |
| deployments/api/tests/db/test_resource_actions.py | Adds integration tests for root_id_by_resource_id mapping correctness and empty-input behavior. |
| deployments/api/tests/db/actions/test_merge.py | Adds guard test ensuring apply-merge rejects repointed inputs with id-based message (no repr() leakage). |
Review details
- Files reviewed: 12/12 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+196
to
+198
| if model.repointed_id is not None: | ||
| root_id = await resolve_root_id(session, id) | ||
| return await get(session, root_id, licensed_sources=licensed_sources) |
CD summary
|
| service | url | fqdn |
|---|---|---|
| api | open | pr-0259-api.purplegrass-c07d0a94.westus2.azurecontainerapps.io |
| entity-linkage | open | pr-0259-el.purplegrass-c07d0a94.westus2.azurecontainerapps.io |
| frontend | https://witty-mushroom-017a3dc1e-259.westus2.1.azurestaticapps.net | |
| stitch-llm | open | pr-0259-llm.purplegrass-c07d0a94.westus2.azurecontainerapps.io |
Database (1)
| db_name | postgres_host | postgres_port | postgres_db |
|---|---|---|---|
| pr_0259 | stitch-dev.postgres.database.azure.com |
5432 |
pr_0259 |
Jobs (1)
| job | image | postgres_db |
|---|---|---|
| db-migrations | ghcr.io/rmi/stitch-api:pr-0259@sha256:4e84bbebc59552192978814d3b27c0a0d624161bd705680d12160d1dc256d94c |
pr_0259 |
Images (4)
| build_time | commit_time | git_sha | image | image_digest |
|---|---|---|---|---|
| 2026-09-04T13:17:33Z | 2026-09-04T13:17:13Z | bdde2c3 | ghcr.io/rmi/stitch-api:pr-0259 |
ghcr.io/rmi/stitch-api:pr-0259@sha256:4e84bbebc59552192978814d3b27c0a0d624161bd705680d12160d1dc256d94c |
| 2026-09-04T13:17:31Z | 2026-09-04T13:17:13Z | bdde2c3 | ghcr.io/rmi/stitch-entity-linkage:pr-0259 |
ghcr.io/rmi/stitch-entity-linkage:pr-0259@sha256:34bc444baa61d180271843bd4cfa2fac766784ec93f05bf8bf287bcb058c948e |
| 2026-09-04T13:17:34Z | 2026-09-04T13:17:13Z | bdde2c3 | ghcr.io/rmi/stitch-seed:pr-0259 |
ghcr.io/rmi/stitch-seed:pr-0259@sha256:7576c09da4861ffe7feb11e8c18c37680f96fdf40b804f413967a6ae80e9a1ec |
| 2026-09-04T13:17:30Z | 2026-09-04T13:17:13Z | bdde2c3 | ghcr.io/rmi/stitch-stitch-llm:pr-0259 |
ghcr.io/rmi/stitch-stitch-llm:pr-0259@sha256:6bf247b1041f1047502a58f198faa2706cbb02e9ac92ac14e25514b3dd003cb0 |
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
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.
Extends #258 to address the merge-review dead end: when two pending merge
candidates share a source resource, approving one merges that resource away
(its original becomes an all-null shell), leaving the other candidate
functionally invalid. This surfaces that state — hiding the approve/deny
controls and telling the reviewer which resource was merged into which.
Added as a separate PR (stack) primarily because this one has some choices around the ergonomics of merge review that we may want to discuss more without blocking #258
The primary problem I have with the current flow is around the null-shell behavior. I think the best option if we have candidates:
Is that A+C is marked as invalid/outdated, and the merge candidate is taken out of the review queue (like "denied", terminal state, can't be suggested again, but is a 3rd state), and we run the linker on
Din hopes that it picks up a new candidate forC(this should probably be part of the approval, or even the resource creation, process - putting the new resource in the queue to look for matches), so that we would get something like:STIT-418 (part 2 of 2) — resolve repointed resources in the Merge Review pane
Stacked on #258 (base branch
feat/redirect-detail); review/merge part 1first, then this retargets to
main.When an overlapping candidate is approved, a member of another pending candidate
gets merged away. Previously that candidate's Approve was guaranteed to 400 with
a message interpolating
repr()on ORM objects (Repointed: [<...object at 0x...>]), and the only exit was Deny — recording a judgment no one made.repointed_resources(which members movedand their terminal resource), computed for PENDING candidates via a single
batched root lookup. The approve-time guard now names the terminal resource
("resource 102 is now resource 301") on both the merge-candidate and
write-path guards.
hides Approve/Deny when a member has moved, and annotates moved members with
"(now 301)".
Also includes a small perf commit dropping redundant queries on the resolve
paths (
get_resolvedbuilds the entity from the row it already loaded;createskips an always-empty staleness lookup).
The retire/requeue/
SUPERSEDED-status flow is intentionally deferred until thestale-candidate dead end actually blocks a reviewer.
Tests
Backend: origin-column batch mapping, N+1 batch guard, PENDING-only rule, the
cross-candidate case end-to-end (approve 400s naming both ids, no
object at 0x, stays PENDING), and the apply_resource_merge guard case. Frontend: stalebanner + hidden controls, facts annotation, compat guard, and a pure-function
suite for the staleness util.
AI assistance
Implemented with Claude Code. Verified: API pytest + frontend vitest green.