Skip to content

Adopting a proposal commits the element, its follow-up work and the decision together - #1257

Merged
WaylandYang merged 3 commits into
deeplethe:devfrom
Maya-Kid:fix/adopting-a-proposal-is-one-commit
Oct 16, 2026
Merged

WaylandYang merged 3 commits into
deeplethe:devfrom
Maya-Kid:fix/adopting-a-proposal-is-one-commit

Conversation

@Maya-Kid

Copy link
Copy Markdown
Contributor

First of three PRs from my adoption plan on #725, where the 2026-10-04 comment says adoption runs "in separate steps, with no check that the proposal still matches what the person saw. A failure in the middle leaves an element without an adopted proposal." This PR makes adoption one commit; the next keeps what a person decided after the agent read a shape or kind word; the versioned PR adds the check, conflict codes and replay once the replay question is answered.

Problem

On dev 2293ec9, an attribute proposal whose example statement has an entity object commits the attribute and queues align_phrases, then add_on refuses the example with 422 bad_property_shape. The attribute stays with no adopted proposal; every retry answers 409 because the key is taken.

Three more gaps:

  • create_entity_type and create_relation_type commit the row, then write its parents, or its domains and ranges, on a second pooled connection, so they cannot join a caller's transaction.
  • The status-only POST /ontology/proposals accepts status: "adopted": it marks the proposal adopted and creates nothing.
  • Adopting a new element calls the embedding model inside the request (ontology_index::refresh).

Change

  • One transaction. adopt locks the open agent proposal FOR UPDATE, writes the element (for a reuse, the person's bindings), its regression cases, the adopted status with the edited flag and the follow-up jobs in one commit. Adopting a row left by the retired Suggest answers 404; the workbench hides those.
  • Writers. create_entity_type and create_relation_type take impl sqlx::Acquire and write on the caller's connection, exactly as Explore table alignments and adopt their ontology together #1179 does. The two PRs also edit the status-only handler; the second to land merges that hunk, keeping Explore table alignments and adopt their ontology together #1179's table guard ahead of the reject.
  • Locks. The classes an adoption names and a reused property are locked against deletion with FOR KEY SHARE, so one deleted meanwhile answers 422 instead of a foreign-key 500. FOR SHARE would deadlock with set_type_embeddings' batch updates.
  • Jobs. A new element queues embed_ontology and its alignment job, due 5 s later (jobs::enqueue_with_max_attempts_after_tx), in its commit, unconditionally, since a queued copy may already be claimed (0051); adopting makes no model call.
  • Reuse. A property reuse writes each shape as the person's binding (decide_on) and queues one materialize_typed per commit, not per shape; that job commits with every binding it covers, so 0051's delivery argument holds per commit. A class reuse writes each kind word through the new type_bindings::decide_and_apply_human_tx and always queues its own align_phrases.
  • Busy. A lock timeout (55P03, only in a class reuse) and a deadlock anywhere in the transaction (40P01) answer the retryable 409 alignment_busy.
  • Status-only endpoint. It answers 422 adoption_endpoint for adopted and always rejects through reject_proposal; the store's now-unused decide_proposal and mark_proposal_edited are removed.
  • Docs. A dated revision note under 0061's decision 3; docs/design/ontology.md says adoption queues embed_ontology like the editor.

The PR adds 674 lines of code and tests. It stays one PR because without any one of the writers, the transaction, the reuse branch, the jobs or the status-only path, some path still commits part of an adoption.

Verification

Each new test fails on dev:

  • a_failed_adoption_leaves_nothing_behind: dev leaves the attribute and its alignment job.
  • the_status_only_endpoint_does_not_adopt: dev answers 200 and marks the proposal adopted.
  • a_proposal_the_retired_suggest_left_is_not_adopted: dev adopts it.
  • adoption_queues_vectors_before_alignment (a relation, then a class): dev queues only the alignment job and embeds in the request.
  • a_reuse_queues_one_projection_for_its_shapes: dev writes both bindings but queues two materialize_typed.
  • a_class_reuse_queues_its_own_phrase_alignment: with align_phrases already queued, dev adds none.
  • a_class_deleted_during_adoption_answers_unknown_class: dev answers 500 from the foreign key.

Linux x86_64 container, Rust 1.98.1, PostgreSQL 16 + pgvector 0.8.6, Poppler 25.03, DB required (UTOPIA_TEST_REQUIRE_DB=1), commit bd83b2b on dev 2293ec9: cargo fmt --all --check and clippy clean; cargo test --locked --workspace 1,569 passed, 0 failed, 6 ignored.

Limits

  • No version check yet: a proposal refreshed after the person read it is adopted as it now stands.
  • A reuse still overwrites a binding a person decided after the agent read it, as on dev; the next PR keeps and lists it.
  • A proposal, new or a reuse, whose example statement does not fit the property's kind (an entity object for an attribute, a value for a relation) can only be rejected.

…ecision together

Signed-off-by: dada-yan <BinjunYann@gmail.com>
Signed-off-by: dada-yan <BinjunYann@gmail.com>

@WaylandYang WaylandYang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, this is the right shape for adoption. The proposal is locked FOR UPDATE first, and the element, regression cases, decided status and follow-up jobs commit together, so a second concurrent adoption waits, re-checks status='open', and answers 404 without writing anything; before this, two reuses of the same proposal could both commit bindings and jobs. One materialize_typed per commit keeps 0051's delivery argument intact, because every decision in that commit is visible to the job that commits with it, and queuing unconditionally is the right call since enqueue_unless_queued_tx cannot stop a queued copy being claimed before our commit. FOR KEY SHARE rather than FOR SHARE is well argued, and the deleted-class test exercises a real blocked DELETE.

Three small things for the next PR in the series, not blockers here:

  • The class-reuse branch queues align_phrases even when no kind word changed. Gating on the bool from decide_and_apply_human_tx, as the property branch does, avoids a pointless model run.
  • api.decideProposal still accepts "adopted" in its TypeScript type. Narrowing it to "rejected" makes the new 422 unreachable from the client.
  • The workbench's onError shows the raw server message, so alignment_busy reaches Chinese-language users in English. Mapping the code there, as reviewErrors.ts does on the review page, would fix that.

Whichever of this and #1179 lands second needs to keep #1179's table guard ahead of the adopted rejection in decide_proposal. Merging, thanks.

@WaylandYang
WaylandYang merged commit 1cc0bfb into deeplethe:dev Oct 16, 2026
6 checks passed
@Maya-Kid
Maya-Kid deleted the fix/adopting-a-proposal-is-one-commit branch October 16, 2026 11:46
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.

2 participants