Skip to content

feat: let bases opt into grounded mapping confirmation - #1332

Open
Aschenbath wants to merge 4 commits into
deeplethe:devfrom
Aschenbath:feat/auto-confirm-mappings-504
Open

Aschenbath wants to merge 4 commits into
deeplethe:devfrom
Aschenbath:feat/auto-confirm-mappings-504

Conversation

@Aschenbath

Copy link
Copy Markdown
Contributor

What this does

Implements the opt-in auto-confirmation contract for exploration mapping proposals (ADR 0011 revision, migration 0117):

  • Adds knowledge_bases.auto_confirm_mappings (boolean, default off) beside governance in base settings.
  • Exploration proposals keep the model's confidence and rationale; at confidence >= 0.8 a proposal takes effect without a person confirming it. Below the threshold it still waits for a person.
  • An automatically confirmed row is recognisable by decided_by IS NULL. propose's conflict handling refreshes those rows too, so a later exploration can still improve a definition no person has touched; once a person revises or decides it, it locks.
  • Each automatic write commits together with a mapping.auto_confirmed system audit event. The mappings page shows automatic origin, confidence and rationale, and permits a person to confirm or reject an automatic definition. English and Chinese setting text included.
  • ADR 0011 revised; migration 0117 added (next number after dev's 0116); backup schema count advanced to 105.

Turning the setting off leaves existing definitions effective until refreshed. The 0.8 gate is opt-in policy, not a measured correctness guarantee; no live-model accuracy claim or default-on change is made.

This exceeds 500 added lines because the setting, persisted evidence, transactional decision/audit, human ownership rules, UI and database-backed regressions form one complete opt-in contract; splitting them would expose an incomplete automation path.

How it was checked

Checked on Linux x86_64 with Rust 1.99.1, PostgreSQL 16.15 + pgvector 0.6.0 and Poppler, final commit 176b756726326e40b14d3fed0d3ab0f59b8766fe:

  • cargo fmt --all --check: passed.
  • cargo clippy --workspace --all-targets -- -D warnings: passed, zero warnings.
  • cargo test --workspace: 1649 passed, 0 failed, 6 pre-existing ignored (with database/PDF coverage enabled).
  • Migration 0117: applied on a fresh database and replayed successfully; stored checksum matches the SQL file.
  • pnpm test: 45 files, 308 tests passed; pnpm build: passed (style guard, TypeScript and Vite; existing bundle-size advisory only).
  • node --test scripts/bench/lib.test.mjs: 15 passed.

The earlier MCP failures were a local migration-checksum mismatch, not a mapping behavior regression: the previous log reports migration 118 was previously applied but has been modified for the MCP fixtures. Compared with unmodified dev at f2e2a47a2b6d0508aab88dc1b3689a6fee44c606 in a separate worktree and its own fresh database: cargo test -p utopia-server api::mcp -- --nocapture passed all 34 tests. The same 34 MCP tests pass on this branch with the final migration and a fresh database. No baseline MCP failure remains.

The first full run also found two store tests failing because the local PostgreSQL template used SQL_ASCII: a_time_mention_is_words_not_a_date::a_time_mention_resolves_inside_the_chunk_and_sets_no_date (invalid byte sequence for encoding "UTF8": 0xba) and phrase_signature_evidence::multiple_evidence_does_not_multiply_statements_or_examples (0xe7). Both errors were reproduced unchanged on dev with the same SQL_ASCII environment. Final validation uses a fresh database explicitly created with --template=template0 --encoding=UTF8, matching the UTF-8 deployment/test requirement. Both tests and the entire workspace pass in the UTF-8 environment.

Before review

  • Every commit is signed off.
  • Rust formatting, strict workspace Clippy and workspace tests pass.
  • pnpm build and pnpm test pass (45 files, 308 tests).
  • Store SQL tested with a required PostgreSQL connection.
  • Migration uses the next free dev number; backup schema count matches migration file count.
  • UI strings are in both English and Chinese.
  • Data model/API changes are recorded in ADR 0011.

Closes #504

…cisions

Signed-off-by: Aschenbath <Aschenbath@users.noreply.github.com>
…ng human decisions

Signed-off-by: Aschenbath <Aschenbath@users.noreply.github.com>
Signed-off-by: Aschenbath <Aschenbath@users.noreply.github.com>
Signed-off-by: Aschenbath <Aschenbath@users.noreply.github.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.

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.

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