Skip to content

A base can opt into confirming well-grounded mapping definitions, and a person can take any of them back - #1332

Closed
Aschenbath wants to merge 1 commit into
deeplethe:devfrom
Aschenbath:feat/auto-confirm-mappings-504
Closed

Aschenbath wants to merge 1 commit into
deeplethe:devfrom
Aschenbath:feat/auto-confirm-mappings-504

Conversation

@Aschenbath

@Aschenbath Aschenbath commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Closes #504.

What this does

A base can opt into confirming exploration's mapping definitions without a person, and a person can take any of those confirmations back. The switch is knowledge_bases.auto_confirm_mappings, default off.

  • The gate. Exploration now keeps confidence and rationale on the proposal. With the switch on, a proposal confirms itself only when its score is at least 0.8, its rationale is not empty and it has an expression or SQL. Anything less waits for a person, as before.
  • The marker. concept_mappings.auto_confirmed_at is set while the current definition stands on an automatic confirmation. decided_by IS NULL cannot be the marker, because scripts/bench/ask.mjs:162 seeds confirmed truth that way.
  • The upsert. Exploration may change a row only when no person has decided it and it is either still proposed, or machine-confirmed with a new definition that also passes the gate. It never demotes anything. Weaker evidence or a switched-off base writes nothing and returns written = false. A refresh that changes a definition keeps the old one in concept_mapping_revisions with no actor. Rows a person confirmed, rejected or edited are never touched. That includes an edited proposal that is still pending, and the bench's seeded truth.
  • The revert. POST /kbs/{id}/mappings/{mapping_id}/revert (Editor) takes a machine-confirmed definition out of effect. It returns the row to pending under the person's name, so the next exploration does not confirm it again. A row that was not confirmed automatically gets a conflict. The row keeps its definition, because it holds only one version. An older version is in the history, and a person restores it by editing.
  • The record. Each automatic write commits with a mapping.auto_confirmed audit event and each revert with mapping.auto_confirm_reverted, so a failed audit writes nothing. The mappings page marks automatic definitions, shows their evidence and has a Revert button. Text is in English and Chinese.
  • Governance. Mappings are exempt from 0043 decision 1 and leave its cut 2. They are configuration outside the graph, so like auto_type_resolution they get their own switch and no agent_decisions row, precedent read or fuse. They keep the audit trail and the revert. The note is in 0043's new ## Revisions and in docs/design/governance.md. 0011 has a dated revision, and its status line now includes this PR.
  • What the gate does not do. 0036's dead ends name exploration-proposed definitions as what A mapping can be confirmed without a person #504 must never ship. A score cannot catch a sum(amt_pay) that runs, looks right and is off by a hundred. 0011's revision says so. What limits that risk is the switch being off by default, the audit trail and the revert. The threshold does not, and nothing here claims an accuracy improvement or argues for turning the switch on by default.
  • Migration 0124_a_base_can_opt_into_mapping_confirmation.sql. 0117 collided with A regression case can expect no binding while a person's binding remains authoritative #1334 and Govern conflicts in the typed graph and preserve human decisions #1131. The backup schema count is now 111.

Why this is over 500 lines

The diff adds 1410 lines: about 410 of product code, about 945 of tests and 55 of documentation. Most of the tests come from splitting each behaviour into its own test, as review asked: 18 store tests, 8 route tests and 4 web tests. The product code is one contract. Shipping the switch without the revert would confirm definitions with no way back. Shipping the gate without the upsert rules would let exploration demote or overwrite rows a person owns. So I could not find a smaller cut that is safe to merge on its own.

How it was checked

All on x86_64 Linux: a Docker container on WSL2 with Rust 1.99.0 and a throwaway pgvector/pgvector:pg16. The tree is commit 9a9d4709. The last amend only added four lines of text to 0011.

  • cargo fmt --all --check: passed.
  • cargo clippy --locked --workspace --all-targets -- -D warnings: passed.
  • cargo run -p utopia-store --example migrate on a fresh database: passed.
  • cargo test --locked --workspace --no-fail-fast with UTOPIA_TEST_REQUIRE_DB=1: 1777 passed, 6 ignored, and one target did not pass. In utopia-cli --test restore, 16 of 17 pass. real_postgres_backup_and_restore_require_force_and_preserve_files_on_sql_failure panics with "pg_dump is required", because my check image has no PostgreSQL client tools. CI installs postgresql-client-16, so CI runs that test and I did not.
  • The 18 new store tests in a_mapping_is_not_a_fact also passed when run on their own.
  • pnpm test: 50 files, 344 tests passed. pnpm build: passed.

🤖 Generated with Claude Code

@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.

This needs a design decision before the code review can finish, so I am putting it here rather than inline.

  1. Governance. 0043 decision 1 says every review queue but nods goes through the governor and names concept mappings as its cut 2; docs/design/governance.md requires an agent_decisions row, a revert and a fuse for every automatic decision. This PR confirms mappings automatically with none of those, and its ADR text declares itself independent of governance without revising 0043.
  2. Direction. 0036's dead ends say exploration-proposed SQL definitions are what automatic confirmation must never ship, and #556 retires this table's role. Automating the retiring path is 600 lines against two records. If the answer is that mappings are configuration and exempt (as #725 puts them on the data-source side), then 0043 and governance.md need the revision note, and an automatic confirm still needs a revert.

Code findings, whichever way that goes:

  • crates/utopia-store/src/mappings.rs, the upsert: status = EXCLUDED.status under decided_by IS NULL AND status IN ('proposed','confirmed') demotes an effective definition silently. An auto-confirmed GMV that the next exploration scores 0.7, or that is re-explored after the switch is turned off, flips back to proposed; chat stops using it and no ledger event records the change. The ADR text promises the opposite.
  • The same WHERE clause widens the refresh regardless of the switch: any confirmed row with NULL decided_by (scripts/bench/ask.mjs:162 seeds truth definitions that way) is now rewritten by the next exploration even with the switch off. Before, WHERE status='proposed' protected every confirmed row.
  • migrations/0117_* collides with #1334 and #1131.
  • The Chinese comment on propose() recording the DO UPDATE…WHERE / RETURNING trap was replaced by a terse English one; please restore the register, and comment the new struct fields.
  • automatic_definitions_refresh_until_a_person_decides bundles about nine behaviours in 127 lines; one test per behaviour, and the route test likewise.
  • Commit messages carry feat: / fix: / docs: prefixes; one English sentence stating the motivation.
  • 0011's status line is not updated and the description does not say.
  • setting_only in kbs.rs is an enumerated is_none() list; a field added to UpdateKbReq later becomes Editor-settable unless someone remembers to extend it. A comment there at least.

… a person can take any of them back.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Aschenbath <Aschenbath@users.noreply.github.com>
@Aschenbath
Aschenbath force-pushed the feat/auto-confirm-mappings-504 branch from f2bc765 to 9a9d470 Compare October 30, 2026 07:33
@Aschenbath Aschenbath changed the title feat: let bases opt into grounded mapping confirmation A base can opt into confirming well-grounded mapping definitions, and a person can take any of them back Oct 30, 2026
@Aschenbath

Copy link
Copy Markdown
Contributor Author

Thanks for this. I took the second branch of your design question: mappings are configuration and exempt from the governor. The branch is now one commit on current dev (9a9d4709) with a rewritten description.

Governance. 0043 now has a ## Revisions entry. Concept mappings are exempt from decision 1 and leave cut 2, citing 0011 and #725. docs/design/governance.md gets a "Mappings are outside the governor" paragraph, and its cut 2 line drops mappings. What the governor's rules still get here are the record and the way back. Every automatic write commits with mapping.auto_confirmed, and there is now a revert: POST /kbs/{id}/mappings/{mapping_id}/revert, Editor. It takes the definition out of effect and returns it to pending under the person's name, so exploration won't confirm it again. It also commits with mapping.auto_confirm_reverted. A row that wasn't confirmed automatically gets a conflict.

Direction. I don't have a good answer to 0036, and I've written that into 0011 rather than leave it implied. A score of 0.8 cannot catch sum(amt_pay). What limits that risk is the switch being off by default, the audit trail and the revert, not the threshold. If you'd rather this table's automation wait for #556 than land against the retiring path, I'll close this. The marker column, the upsert rules and the revert should carry over to whatever #556 renders.

On the code findings:

  • Silent demotion. The upsert never demotes now. A machine-confirmed row changes only when the new definition also passes the gate with the switch on. Weaker evidence or a switched-off base writes nothing and returns written = false. Tests: weaker_evidence_does_not_demote_an_automatic_definition, turning_the_switch_off_neither_revokes_nor_refreshes.
  • Seeded truth rewritten. I added a marker column, concept_mappings.auto_confirmed_at. Only rows that carry it are refreshable once confirmed, so ask.mjs's confirmed rows with NULL decided_by stay out of reach whether or not the switch is on (exploration_leaves_a_confirmed_row_it_did_not_confirm). A refresh that changes the definition writes the old one to concept_mapping_revisions, with no actor.
  • Migration number. It is now 0124_a_base_can_opt_into_mapping_confirmation.sql, and the backup schema count is 111.
  • Comments. The Chinese comment on propose() about the DO UPDATE … WHERE / RETURNING trap is restored, and the new fields are commented in the same register.
  • Bundled tests. These are split into 18 store tests (one per behaviour) and 8 route tests. The route tests share a small Fixture.
  • Commit messages. The branch is a single commit, one sentence, with no prefix.
  • 0011 status line. It now includes A base can opt into confirming well-grounded mapping definitions, and a person can take any of them back #1332, and the description says so.
  • setting_only. It now destructures UpdateKbReq exhaustively, so adding a field fails to compile until someone decides who may set it.

One limit I left in place: a revert keeps the current definition, because the row holds one version. If a refresh replaced a good definition, the good one is in the history, and a person gets it back by editing.

Checks are in the description. Every target passed except one test that needs pg_dump, which my check image lacks and CI has.

@WaylandYang

Copy link
Copy Markdown
Contributor

Thanks for working through all of this. The upsert rules, the marker column and the revert read well. I'd still like this to wait for #556: 0036 says `sum(amt_pay)` is what automatic confirmation must never ship, and an off-by-default switch doesn't answer that. As you offered, please close this for now and carry those parts into #556.

Things to fix when it comes back:

  • Pending hand-edits made before the migration have a NULL `decided_by`, so exploration could overwrite and confirm them.
  • A refresh never compares evidence, so a 0.80 definition can replace a 0.99 one, and a revert can't restore the old one.
  • The switch's note says refreshed definitions return to review, but with the switch off nothing refreshes.
  • The switch is Editor-level, while `auto_type_resolution` and `governance` need Admin.

@Aschenbath

Copy link
Copy Markdown
Contributor Author

Closing as agreed, to come back as part of #556. I'll carry over the upsert rules, the marker column and the revert, and fix the four points you listed there: pre-migration pending hand-edits with NULL decided_by, refreshes that don't compare evidence (and the revert that can't restore the old definition), the switch note that doesn't match off-by-default behaviour, and raising the switch to Admin like auto_type_resolution and governance. Thanks for the careful review.

@Aschenbath Aschenbath closed this Oct 31, 2026
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