Repository navigation
Grouped diagnostics and reusable value clustering - #120
Conversation
ee1859d to
f78eac3
Compare
There was a problem hiding this comment.
🟡 Changes recommended
There are compile-blocking uses of str::starts_with(char::is_uppercase) in crates/spellcheck/src/lib.rs that must be fixed before this can be safely merged.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR implements grouped diagnostics in the Problems panel using explicit producer-provided grouping metadata (while retaining atomic diagnostics as the source of truth), and extracts value-variant matching into a new reusable clustering crate, wired into per-column settings and fix providers.
Changes:
- Add diagnostic grouping metadata (
DiagnosticGroup) and richer column-validator output (ColumnFinding+ColumnValues) to support stable grouping and subdelimited-cell iteration without per-cell allocations. - Implement grouped rendering + expansion in
ProblemsPanel(including multi-select source filtering) while preserving navigation and one-cell fixes on expanded occurrences. - Introduce
crates/clustering(value-variant pairing + fixes) and add a per-columnvariant_reviewsetting with UI/config/docs updates.
File summaries
| File | Description |
|---|---|
| sample/column_config.csv | Adds Variant Review column to sample column config for enabling value-variant review per column. |
| PLAN.md | Adds a detailed implementation plan/spec for grouped diagnostics and clustering extraction (now marked as complete). |
| docs/diagnostics.md | Documents spelling/capitalization/value-variant checks, grouping behavior, scopes, counts, and OpenRefine credit. |
| docs/columns.md | Documents per-column spellcheck behavior and new value-variant review option. |
| crates/table/src/panel.rs | Adds a GPUI test validating variant-fix behavior (revalidate + undo semantics) and updates diagnostics literals for new group field. |
| crates/table/src/note.rs | Adds column context-menu toggle for “Review value variants” and updates diagnostics literals for new group field. |
| crates/table/src/file_links.rs | Adapts file-link findings pipeline to new ColumnFinding addressing. |
| crates/table/src/agent.rs | Ensures staged agent findings set group: None (bridge findings remain atomic/ungrouped). |
| crates/table/Cargo.toml | Adds clustering as a dev-dependency for table tests. |
| crates/spellcheck/src/lib.rs | Refactors spellcheck to load all installed dictionaries, adds capitalization validator + fixes, emits per-token findings with grouping metadata, and supports typographic apostrophes. |
| crates/spellcheck/src/catalogue.rs | Updates dictionary round-trip test to account for multi-dictionary storage. |
| crates/settings/src/columns.rs | Adds persisted per-column variant_review setting with defaults/tests. |
| crates/project-wizard/src/steps/review.rs | Applies variant_review from column config during project creation. |
| crates/project-wizard/src/data.rs | Extends column config parsing to accept Variant Review column. |
| crates/plugin-host/src/plugin.rs | Adapts plugin validators to ColumnValues input and ColumnFinding output. |
| crates/plugin-host/src/lib.rs | Passes column subdelimiter into ColumnValues for plugin validation runs. |
| crates/diagnostics/src/validator.rs | Introduces ColumnValues, CellValue, ColumnFinding, and begin_run; addresses findings with correct scope + group metadata and passes subdelimiter into snapshots. |
| crates/diagnostics/src/panel.rs | Implements grouped Problems panel projection + expansion state and replaces single-source dropdown with multi-select source filter. |
| crates/diagnostics/src/lib.rs | Adds Location::scope + constructors, DiagnosticGroup, Diagnostic.group, and friendly source labels. |
| crates/diagnostics/src/fixes.rs | Updates tests for new Diagnostic.group field. |
| crates/clustering/src/lib.rs | Adds ValueVariants column validator and fix provider gated by per-column variant_review, emitting grouped findings and stale-safe fixes. |
| crates/clustering/src/core.rs | Implements normalized tokenization, candidate blocking, pair evidence, and stable pair keys without transitive clustering. |
| crates/clustering/Cargo.toml | Defines new clustering crate dependencies (diagnostics/settings/gpui + caseless/unicode-normalization/strsim). |
| crates/checks/src/dates.rs | Updates date validator to validate per logical value via ColumnValues and return ColumnFinding. |
| crates/checks/src/authority/mod.rs | Adapts authority publish path to feed ColumnFinding into diagnostics::address. |
| crates/app/src/main.rs | Registers capitalization validator + fixes and registers the value-variant checker (validator + fixes) at app startup. |
| crates/app/src/app_settings/mod.rs | Updates spelling language setting text and adds Settings UI for selecting variant-reviewed columns. |
| crates/app/src/app_settings/config.rs | Adds Variant Review to the column config export schema and tests. |
| crates/app/Cargo.toml | Adds clustering dependency for app integration. |
| CLAUDE.md | Updates crate map description to include clustering and refreshes checks description. |
| Cargo.toml | Adds crates/clustering to the workspace members. |
| Cargo.lock | Locks new dependencies (clustering, caseless, strsim, unicode-normalization). |
| AGENTS.md | Clarifies that bridge findings remain ungrouped atomic diagnostics regardless of built-in grouping UI. |
Review details
Suppressed comments (2)
crates/spellcheck/src/lib.rs:470
str::starts_withdoes not acceptchar::is_uppercase; this will not compile. Use a first-character uppercase check instead.
!ignore_capitalized || !token.starts_with(char::is_uppercase)
crates/spellcheck/src/lib.rs:504
str::starts_withdoes not accept a predicate;word.starts_with(char::is_uppercase)will not compile. Usechars().next()to detect initial uppercase for the proper-noun rule.
None if ignore_capitalized && word.starts_with(char::is_uppercase) => {
WordOutcome::ProperNoun
}
- Files reviewed: 32/33 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Restores the merge resolution the rebase onto main dropped.
Adds a per-cell Problems submenu, a Show ignored toggle in the Problems panel, and a removable list of ignores in column settings.
Blurring a cell without editing it re-ran every validator over the whole sheet and rewrote the dataset. panel.rs also carries a timing log for the settings re-read.
The grid filter and the Problems source filter each had a private copy; settings pickers used the stock trailing check. Also drops the reviewed-columns picker's duplicate revalidation and logs picker and panel refresh timings.
Each rejected value is its own finding with group metadata and a subject, so repeats collapse in the Problems panel and fixes touch only that value. docs/diagnostics.md also carries the spelling language wording.
Spelling and capitalization re-checked every word twice per run (about 1.4s each on a 1893-row sheet); verdicts are now cached per value and shared. The language picker is multi-select, loads only ticked dictionaries, and reloads the checker without a restart.
c0e7e7d to
b5bb03b
Compare
Validators::run snapshots on the UI thread, checks on the background executor, and publishes back. A per-app generation drops superseded runs and runs outpaced by a removal; a lock keeps passes from interleaving begin_run state.
Ignoring shows a toast with Undo that reverses the ignore; a newer ignore replaces the older toast. The main window now renders the notification layer.
Values like "Satwinder B." and "Dr. Satwinder Bains" now pair with "Bains, Satwinder": word order is ignored, an initial matches a word it starts, and one extra word is allowed.
Capitalization slips come from the spell source, so they get the same suggestions, Add to dictionary, and ignore options. Group fixes also offer Add to dictionary.
An expanded group with several observed forms lists each form as a collapsible row over its occurrences. Group menus get a direct Ignore all item.
Row, column, and message sit in fixed-width cells, groups show a chevron and count, and nested rows get guide lines and a tint. Main groups and observed forms share one projection, so a form seen once is a plain row. Column and message cells show their full text on hover.
The settings description is plain text, so the bold markers showed literally.
SourceActions offers menu items about the producer itself, on single findings and groups alike.
Authorities offer Try again after a refusal and Refresh otherwise. In-flight terms are tracked so overlapping runs never repeat a request, a cancelled run keeps its answers, failed terms wait before retrying, and passes continue until every term is answered.
Purpose
Group repeated diagnostics by stable producer keys while preserving every exact location. Move value-variant matching from
spellcheckinto a reusableclusteringcrate.Scope
crates/clustering.Safety
Grouping changes presentation only. Atomic diagnostics remain the source of truth, and similarity never changes data automatically.
Follow-up
The full cluster-review workspace is tracked in #125.
Plan
See
PLAN.mdfor contracts, crate boundaries, implementation order, tests, and the OpenRefine license boundary.Status
Planning complete. Implementation has not started for the grouped diagnostic UI.
Notion: The source task still needs to be created and cross-linked.