Admin: evidence-based DocumentDb detection, and soft delete it can act on - #9
Open
aritchie wants to merge 1 commit into
Open
Admin: evidence-based DocumentDb detection, and soft delete it can act on#9aritchie wants to merge 1 commit into
aritchie wants to merge 1 commit into
Conversation
…t on
Replaces the admin tool's table classifier. It ran unanchored name-substring
rules before gathering any evidence, so a business table called audit_history
or geo_spatial_index was reported as a DocumentDb sidecar, a documents table
called orders_history was hidden as one, and everything else cost a
deliberately-failing SELECT per table.
Phase 1 - classification from evidence
- IDatabaseProvider gains BuildListColumnsSql() (ANSI default; SQLite, Oracle
and MySQL override), so one read describes every table with its column types
instead of one failing probe per table.
- IDatabaseProvider gains SpatialTableName() and OwnedTableNames(table, type):
every name the provider would have created around a documents table, with
SQLite adding its R*Tree/FTS5/vec0 shadows and DuckDB its full-text source
table. Sidecars are now found by computing a name and finding it.
- A table is a candidate on the envelope and Confirmed only on corroboration
(JSON-shaped Data, the (Id, TypeName) key, idx_{table}_typename, JSON
property indexes, or an opt-in sampled row). Envelope-only is Probable -
browsable, and badged with why. A candidate that is another candidate's
computed sidecar is demoted, which is what the blob table needs: it carries
the whole envelope plus BlobKey.
- New DatabaseIdentity verdict (participates, confidence, counts, features,
reasons) computed from the same read; TestConnection now formats from it.
- TableRole splits Sidecar into Spatial/Vector/FullText; TableInfo gains Owner,
Confidence and Feature. IsBrowsable is unchanged, so existing consumers are.
Phase 2 - surfacing, and the soft-delete hazard
Soft delete writes no DDL, so nothing in a database records it - and the tool
was hard-deleting documents the application would only have flagged. It is now
declared per connection (ConnectionProfile.SoftDeleteFlags, or
Shiny:DocumentDb:{name}:SoftDelete:{Type} for a host-provided connection):
- DeleteDocuments/ClearType refuse a declared type unless the caller says
permanent; SoftDeleteDocuments/RestoreDocuments write the flag the way
SoftDeleteMapping would (false for a bool, null for a timestamp).
- Browse gains a live/deleted/all filter over the declared path; flagged rows
are badged in both front ends.
- InferSchema reports a soft-delete *candidate* only when the field name is in
the deleted family AND its sampled values match one of the two accepted
shapes. A candidate changes no role, no browsability, no feature and no
button - the Structure tab offers a one-click declare.
- Blazor overview gets the verdict banner plus an internals/foreign split with
an Owner column; the TUI gets the verdict, an Owner column and a foreign
toggle; HideForeignTables is a profile setting.
- The assistant gains a database_identity tool, reports role/owner/confidence,
and scopes a declared type's browse to live documents and says so.
Tests: 26 new in ShinyDocDbMyAdmin.Tests (classification against real sidecars
plus decoys, the orders_history regression, probable/sampling, the verdict, and
soft delete declared and undeclared - including that the library's own query
filter agrees with the flag this tool writes) and 2 in the TUI widget tests.
BREAKING: TableRole.Sidecar is gone; DeleteDocuments/ClearType take a
`permanent` flag ahead of the CancellationToken.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NrYw1ykmqwQkW9f4A9GhJg
There was a problem hiding this comment.
Pull request overview
This PR implements Phases 1 and 2 of plans/admin-database-identification.md in ShinyDocDbMyAdmin: it replaces name-substring table classification with evidence-based detection (including a unified DatabaseIdentity verdict) and introduces an explicit soft-delete declaration path so the admin UI can safely “delete” (flag) documents in a way that matches the application.
Changes:
- Add evidence-based table classification using provider-supplied catalog reads (
BuildListColumnsSql) plus computed owned-table names (OwnedTableNames), and surface a unifiedDatabaseIdentityverdict across UI/TUI/assistant. - Add “declared soft delete” support (profile/config-driven) with safe delete semantics (flag vs permanent), browse partitioning (live/deleted/all), restore, and candidate suggestions from schema inference.
- Update UI/TUI surfaces, assistant tool surface, docs/skill/readme, and add new test coverage for classification + soft delete behavior.
Reviewed changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| version.json | Bumps version to 13.5-beta.{height}. |
| tests/ShinyDocDbMyAdmin.Tui.Tests/WidgetTests.cs | Adds TUI grid tests for flagged/deleted row labeling. |
| tests/ShinyDocDbMyAdmin.Tests/SoftDeleteAdminTests.cs | New tests covering declared vs undeclared soft-delete behavior and safety guards. |
| tests/ShinyDocDbMyAdmin.Tests/DatabaseIdentityTests.cs | New tests validating evidence-based classification and verdict output. |
| src/ShinyDocDbMyAdmin/wwwroot/app.css | Adds styling for dimming deleted rows and formatting verdict “Why” reasons. |
| src/ShinyDocDbMyAdmin/Components/Panels/StructureTab.razor | Surfaces soft-delete candidates and adds one-click “Declare” for saved profiles. |
| src/ShinyDocDbMyAdmin/Components/Panels/BrowseTab.razor | Adds live/deleted/all filtering for declared types; delete now flags by default with explicit permanent delete. |
| src/ShinyDocDbMyAdmin/Components/Pages/DatabaseOverview.razor | Adds verdict banner + internals/foreign tables panels. |
| src/ShinyDocDbMyAdmin/Components/Pages/ConnectionEdit.razor | Adds HideForeignTables and SoftDeleteFlags editing UI. |
| src/ShinyDocDbMyAdmin.Tui/Widgets/DocumentGrid.cs | Adds soft-delete awareness to TUI grid row rendering. |
| src/ShinyDocDbMyAdmin.Tui/Screens/TableOverviewScreen.cs | Updates “Empty type” flow to account for declared soft-delete types. |
| src/ShinyDocDbMyAdmin.Tui/Screens/DatabaseOverviewScreen.cs | Adds verdict display, owner column, and toggling visibility of foreign tables. |
| src/ShinyDocDbMyAdmin.Tui/Panels/BrowsePanel.cs | Adds declared soft-delete partitioning, restore, and “flag deleted” behavior. |
| src/ShinyDocDbMyAdmin.Core/Services/ProvidedConnections.cs | Adds host-config soft-delete declarations parsing for provided connections. |
| src/ShinyDocDbMyAdmin.Core/Services/ProfileStore.cs | Adds SaveSoftDeleteFlags for updating declarations without re-entering secrets. |
| src/ShinyDocDbMyAdmin.Core/Services/DocumentAdminService.SoftDelete.cs | New soft-delete declaration read/write/restore + predicate + row-flag detection. |
| src/ShinyDocDbMyAdmin.Core/Services/DocumentAdminService.Schema.cs | Adds schema-based “soft delete candidate” inference (suggestion only). |
| src/ShinyDocDbMyAdmin.Core/Services/DocumentAdminService.Identity.cs | New catalog snapshot + evidence scoring + owned-table detection + database verdict. |
| src/ShinyDocDbMyAdmin.Core/Services/DocumentAdminService.Geometry.cs | Uses provider SpatialTableName convention. |
| src/ShinyDocDbMyAdmin.Core/Services/DocumentAdminService.cs | Replaces per-table cache with catalog snapshot cache; TestConnection now reports verdict. |
| src/ShinyDocDbMyAdmin.Core/Services/DocumentAdminService.Crud.cs | Adds “permanent required” guard for declared soft-delete types; updates spatial naming usage. |
| src/ShinyDocDbMyAdmin.Core/Services/DocumentAdminService.Browse.cs | Adds declared soft-delete partitioning via BrowseQuery.Deleted. |
| src/ShinyDocDbMyAdmin.Core/Services/AiToolSurface.cs | Adds database_identity tool; enriches table listings; browse now communicates soft-delete scope. |
| src/ShinyDocDbMyAdmin.Core/Services/AdminConnection.cs | Includes soft-delete declarations in connection fingerprint to invalidate cache on change. |
| src/ShinyDocDbMyAdmin.Core/Models/ConnectionProfile.cs | Adds SoftDeleteFlag(s) and HideForeignTables. |
| src/ShinyDocDbMyAdmin.Core/Models/AdminModels.cs | Expands TableInfo (Owner/Confidence/Feature/IsOwned) and adds identity/confidence enums. |
| src/Shiny.DocumentDb/IDatabaseProvider.cs | Adds BuildListColumnsSql, OwnedTableNames, and SpatialTableName defaults. |
| src/Shiny.DocumentDb.Sqlite/SqliteDatabaseProvider.cs | Implements BuildListColumnsSql and extends owned table naming for SQLite shadows. |
| src/Shiny.DocumentDb.Oracle/OracleDatabaseProvider.cs | Implements BuildListColumnsSql via user_tab_columns. |
| src/Shiny.DocumentDb.MySql/MySqlDatabaseProvider.cs | Implements BuildListColumnsSql scoped to DATABASE(). |
| src/Shiny.DocumentDb.DuckDb/DuckDbDatabaseProvider.cs | Extends owned table naming for DuckDB’s full-text source table. |
| skills/shiny-documentdb/SKILL.md | Updates skill triggers and guidance for new provider members. |
| readme.md | Documents the new verdict and soft-delete declaration behavior; updates assistant tool count. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+125
to
+137
| var target = body; | ||
| for (var i = 0; i < segments.Length - 1; i++) | ||
| { | ||
| if (target[segments[i]] is JsonObject nested) | ||
| { | ||
| target = nested; | ||
| continue; | ||
| } | ||
|
|
||
| var created = new JsonObject(); | ||
| target[segments[i]] = created; | ||
| target = created; | ||
| } |
Comment on lines
+67
to
+71
| var softDelete = string.Join( | ||
| ";", | ||
| resolved.Profile.SoftDeleteFlags.Select(f => $"{f.TypeName}:{f.PropertyPath}:{f.FlagKind}")); | ||
|
|
||
| var fingerprint = $"{resolved.Provider}|{resolved.ConnectionString}|{resolved.Password}|{softDelete}"; |
Comment on lines
+151
to
+158
| yield return new SoftDeleteFlag | ||
| { | ||
| TypeName = entry.Key, | ||
| PropertyPath = path.Trim(), | ||
| FlagKind = string.IsNullOrWhiteSpace(kindName) | ||
| ? SoftDeleteFlagKind.Boolean | ||
| : Enum.Parse<SoftDeleteFlagKind>(kindName, ignoreCase: true) | ||
| }; |
| @@ -509,6 +560,15 @@ string BuildHistoryPruneByCountSql(string tableName) | |||
|
|
|||
| // Spatial (optional — only SQLite implements these) | |||
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.
Description of Change
Implements
plans/admin-database-identification.md, Phases 1 and 2. Phase 3 (an engine-side__shinydocdbregistry) is deliberately not built — the plan calls it optional and says to decide after Phase 1 ships.version.jsonmoves to13.5-beta.{height}.Phase 1 — classification from evidence. The classifier ran unanchored name-substring rules before gathering any evidence. A business table called
audit_history,customer_blobsorgeo_spatial_indexwas reported as a DocumentDb sidecar; a documents table calledorders_historywas hidden as one; everything else cost a deliberately-failingSELECT … WHERE 1 = 0per table.IDatabaseProvider.BuildListColumnsSql()— ANSI default, overridden for SQLite (sqlite_master⨝pragma_table_info), Oracle (user_tab_columns) and MySQL (scoped toDATABASE()). One read describes every table with its column types, so a 300-table shared schema no longer costs 300 failing statements.IDatabaseProvider.SpatialTableName()andOwnedTableNames(table, typeName)— every name the provider would have created around a documents table. SQLite adds its R*Tree, FTS5 andvec0shadows; DuckDB its full-text source table. Sidecars are found by computing a name and finding it, so name matching became confirmation rather than a guess.Datacolumn, the(Id, TypeName)key,idx_{table}_typename,idx_json_*, or an opt-in sampled row that really is a document). Envelope-and-nothing-else is Probable — still browsable, and badged with why.BlobKey, so by columns alone it reads as a documents table.DatabaseIdentityverdict (participates, confidence, counts, features, reasons) from the same read.TestConnectionnow formats from it, so the test button and the overview cannot disagree.Phase 2 — surfacing, and the soft-delete hazard.
AddSoftDeletewrites no column, table or index, so nothing in a database records it — and the admin's delete was a realDELETEagainst a type the application would only have flagged, silently. It is now declared per connection (ConnectionProfile.SoftDeleteFlags, orShiny:DocumentDb:{name}:SoftDelete:{Type}for a host-provided connection, which is the only place one can be declared).DeleteDocuments/ClearTyperefuse a declared type unless the caller sayspermanent.SoftDeleteDocuments/RestoreDocumentswrite whatSoftDeleteMappingwould:true/falsefor a bool, now/nullfor a timestamp.InferSchemareports a soft-delete candidate only when the field name is in the deleted family and its sampled values match one of the two shapesSoftDeleteMapping.Buildaccepts. A candidate changes no role, no browsability, no feature and no button — the Structure tab offers a one-click declare.HideForeignTablesis a profile setting.database_identitytool, role/owner/confidence onlist_tables, and a declared type's browse scoped to live documents with ascopesentence saying so.Docs site (
~/Desktop/dev/documentation) is not reachable from this session's container, so the admin pages and the## 13.5 TBDrelease note still need writing there.Issues Resolved
None — implements
plans/admin-database-identification.md.API Changes
Shiny.DocumentDb— all three are default interface members, so no provider is obliged to implement any of them:IDatabaseProvider.BuildListColumnsSql()— new; ANSI default.IDatabaseProvider.SpatialTableName(string tableName)— new;{table}_spatialdefault.IDatabaseProvider.OwnedTableNames(string tableName, string? typeName)— new, withstatic IDatabaseProvider.DefaultOwnedTableNames(provider, table, type)for overrides to build on.ShinyDocDbMyAdmin.Core(breaking for anything embedding the admin layer):TableRole.Sidecarremoved, replaced bySpatial/Vector/FullText.TableInfogainsOwner,Confidence,Feature,IsOwned;IsBrowsableis unchanged.DocumentAdminService.DeleteDocuments/ClearTypetake abool permanentparameter before theCancellationToken, so positionalctcall sites need updating.DatabaseIdentity,IdentityConfidence,TableConfidence,DeletedFilter,SoftDeleteFlag,SoftDeleteFlagKind,GetIdentity,GetSoftDeleteFlag,SoftDeleteDocuments,RestoreDocuments,IsFlagged,ProfileStore.SaveSoftDeleteFlags,BrowseQuery.Deleted,InferredField.SoftDeleteCandidate,ConnectionProfile.SoftDeleteFlags/HideForeignTables.AiToolSurface.ToolNamesgainsdatabase_identity;TableSummaryandDocumentResultsgain fields.Behavioral Changes
TestConnection's sentence changed to the verdict summary.Testing Procedure
dotnet test tests/ShinyDocDbMyAdmin.Tests— 404 passed. The 17 failures are environmental and pre-existing on this branch's base: 14 need the sqlite-vec native binary (committed for osx-arm64 only) and 3 need a PostgreSQL container.dotnet test tests/ShinyDocDbMyAdmin.Tui.Tests— 58 passed, 0 failed.dotnet test tests/Shiny.DocumentDb.Tests— 2028 passed / 4261 failed, byte-identical to the same run on the unmodified tree (verified by stashing). Every failure isDockerUnavailableException; Docker is not available in this container, so the non-SQLite provider suites cannot run here and someone with Docker should re-run them before merge.audit_history/customer_blobs/geo_spatial_index/invoice_vec_lines/search_fts_cache/ a partial-envelopeorders; theorders_historyregression; probable-vs-confirmed and the sampling upgrade; the foreign-only verdict; and soft delete declared and undeclared — including that the library's own query filter agrees with the flag this tool writes.PR Checklist
Generated by Claude Code