Reconsider negative bindings when existing definitions change - #775
Conversation
Signed-off-by: dada-yan <BinjunYann@gmail.com>
WaylandYang
left a comment
There was a problem hiding this comment.
The gap is real and it is mine: I wrote the negative side of that predicate against created_at, so
a base only reconsidered a none when something new appeared, never when the thing already there
was given the definition that would have matched. Thank you for the issue and the fix in one piece.
Verified here against an isolated database. On the branch:
test result: ok. 6 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
With only the two source files reverted to dev and your tests kept:
test class_edit_reconsiders_none ... FAILED
test class_edit_reconsiders_undecided ... FAILED
test property_edit_reconsiders_none ... FAILED
test property_edit_reconsiders_undecided ... FAILED
test stale_person_decisions_still_reject_agent_writes ... FAILED
test new_candidates_and_bound_edits_still_invalidate ... ok
test result: FAILED. 1 passed; 5 failed
Which is exactly the split your description claims. new_candidates_and_bound_edits_still_invalidate
passing in both directions is the control that matters most to me: the change is a superset, since
updated_at is never null and never earlier than created_at, so nothing that was stale before
stops being stale.
The cost is worth writing down somewhere
One edit anywhere in a base now re-opens every negative binding decided before it. In the bases on
hand that is 151 signatures in one and 131 in another, and each one costs the aligner two votes.
That was already true for an added property, so the blast radius is not new, but an edit is a much
more common gesture than an addition, and a typo fixed in a description will now buy a few hundred
model calls.
The debounce in #757 coalesces the runs but not the work inside a run, so this is real. I am not
asking you to narrow it in this pull request — comparing against the definitions that could actually
match is the harder fix your description already names, and this is the right small one. I would
like one sentence in ontology.md saying what an edit costs, next to where
it now says an edit reconsiders, so that the next person reading the page can see the bill before
they discover it.
Two smaller things
stale_person_decisions_still_reject_agent_writesfailing without the fix is the nicest test here:
it shows the widened predicate reaches the person-decision guard too, and that the guard still
holds. A person's call stays a person's call.- The fixture ordering timestamps instead of sleeping is the right choice, and it is why this suite
runs in 1.5 seconds.
The scope you propose is the scope I would take. In-flight model calls still race an edit, and that
is input-version tracking, which is its own change.
Signed-off-by: dada-yan <BinjunYann@gmail.com>
|
Thank you for checking this and for the guidance. I've added the cost note to |
Editing an existing class or property definition leaves automatic
none/undecidedbindings out of the alignment work list. Both negative stale predicates now compare against the newestupdated_atin the same KB, so an edited definition can be considered again. Bound results still depend only on their selected definition, and the existing person-decision protections are unchanged.Fixes #773.
The database regression covers both negative statuses for classes and properties, new candidates, bound-definition edits, unchanged inputs, KB isolation, fresh negative decisions returning to cache, and person decisions rejecting automatic writes. Fixture timestamps establish ordering without sleeps; signatures and edits use the real store APIs. This tests the store lifecycle, not a complete model-backed worker run.
Validation: Linux x86_64, Rust 1.98.1, PostgreSQL 16.15 / pgvector 0.8.6, with
UTOPIA_TEST_REQUIRE_DB=1and an isolated database. Final regression: 6 passed (18 lifecycle cases); the old predicates fail the four automatic-edit regressions and the newly-stale person-decision control. Full workspace: 875 passed, 1 existing live-HTTPS test ignored, 0 failed. Also passed:cargo fmt --all --check,cargo clippy --locked --workspace --all-targets -- -D warnings,cargo test --locked --workspacewith DB and PDF requirements enabled,cargo build --locked --workspace, and the frozen-lockfile frontend install, 63 frontend tests and production build.This is deliberately the small timestamp-based fix. Cosmetic edits also update these timestamps and can cause extra reevaluation. Changes during an in-flight model call still need input-version tracking; this PR does not change that concurrency contract or introduce migrations. The review feedback on reevaluation cost is addressed in the design page: debouncing reduces runs, not the number of items reconsidered; two votes per eligible item are made through batched requests.
Tested commit
c33e20c3a4ebb62ccbcee85ca2f746af6190fe64on top ofdev@966b4028f6630117ec9b623f17d6e9576880ac68(includes #772). Linux source hashes match every tracked file in this commit.Documentation-only follow-up
c0983b2addresses the review request;git diff --checkpassed. The implementation and regression tests are unchanged from the Linux-tested commit above.