P0: share gap-risk detection with the preview path; user-entered AI feedback - #43
Merged
Merged
Conversation
…eedback Completes the remaining P0 (Useful First) items. P0.1, P0.2 and P0.4 already landed on main via #41; this covers the rest. P0.3 — act-path safety The range-gap detector ("SUM skips an adjacent numeric cell") moves out of columnOps.ts into src/lib/formulaGapRisk.ts so the Apply/Reject *preview* path can share it, not just the execute path. The preview now surfaces the risk as a warning alongside the proposed changes, and Apply confirms those warnings via the existing confirmGaps override. columnOps.ts shrinks by ~110 lines as a result. P0.5 — AI quality loop Thumbs-down now opens an optional "what went wrong?" field. The rating is recorded immediately on click; submitting the note re-records it with the user's own words instead of the auto-derived fingerprint, which is what makes failover analysis actionable. Cmd/Ctrl+Enter submits, Escape skips. Parser question-veto format_as_table joins DESTRUCTIVE_TOOLS so "should I format this as a table?" no longer fires a bulk restyle. Targeted tools stay excluded — the polite-framing path deliberately treats "can you highlight X" as a command. Verified: lint:ci, typecheck, 1791 unit tests, 344 server tests, 12 realengine golden-set tests.
Reviewer's GuideThis PR completes the remaining usefulness-first P0 work by making formula gap-risk detection available to both execution and preview paths, surfacing risks before Apply, collecting actionable user-entered thumbs-down feedback, and vetoing question-framed bulk table formatting. Sequence diagram for formula gap-risk preview and applysequenceDiagram
participant User
participant Preview as buildActionPreview
participant Risk as detectFormulaRangeGapRisk
participant Apply as handleApplyFormula
participant Sheet
User->>Preview: Preview apply_formula
Preview->>Risk: detectFormulaRangeGapRisk
Risk->>Sheet: Inspect adjacent numeric cells
Sheet-->>Risk: Gap risk or null
Risk-->>Preview: warnings[] when risk exists
Preview-->>User: Proposed changes and warnings
User->>Apply: Apply with confirmGaps
Apply->>Risk: detectFormulaRangeGapRisk
Risk-->>Apply: Gap risk or null
Apply->>Sheet: Write formula when confirmed
Sequence diagram for user-entered thumbs-down feedbacksequenceDiagram
actor User
participant Toolbar as MessageToolbar
participant Panel as ChatPanel
participant Feedback as recordChatFeedback
User->>Toolbar: Click thumbs-down
Toolbar->>Panel: onFeedback(messageId, down)
Panel->>Feedback: recordChatFeedback with fingerprint
Toolbar-->>User: Show optional detail field
User->>Toolbar: Enter explanation
User->>Toolbar: Send feedback or Cmd/Ctrl+Enter
Toolbar->>Panel: onFeedback(messageId, down, userDetail)
Panel->>Feedback: recordChatFeedback with user words
Feedback-->>Panel: Deduplicate by messageId
Flow diagram for question-vetoed table formattingflowchart TD
A[User asks whether to format as a table] --> B{Parser resolves format_as_table}
B --> C{Question phrasing detected}
C -->|Yes| D[Veto destructive tool parse]
D --> E[Hand request to LLM]
C -->|No| F[Continue normal command handling]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: Comment |
This was referenced Sep 27, 2026
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've reviewed your changes and they look great!
Sourcery assessment
Needs a human reviewer. If the shared gap detector is wrong, it could either block valid formula applications or allow a formula with an omitted adjacent value to be written into the sheet. A bad formula persists in stored workbook data, but it is bounded and can be corrected or recomputed; reverting the change does not automatically repair formulas already written.
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.
Completes the remaining P0 (Useful First) items from
docs/strategy/2026-09-24-usefulness-first-strategy.md. P0.1, P0.2 and P0.4 already landed onmainvia #41; this covers the rest.8 files, +175 / −120.
P0.3 — act-path safety
The range-gap detector ("
SUMskips an adjacent numeric cell") moves out ofcolumnOps.tsintosrc/lib/formulaGapRisk.ts, so the Apply/Reject preview path can share it rather than only the execute path. The preview now reports the risk as awarnings[]entry alongside the proposed changes, and Apply confirms those warnings through the existingconfirmGapsoverride.columnOps.tsshrinks by ~110 lines as a side effect.P0.5 — AI quality loop
Thumbs-down now opens an optional "what went wrong?" field. The rating is recorded immediately on click; submitting the note re-records it with the user's own words instead of the auto-derived fingerprint — which is what makes failover analysis actionable rather than aggregate-only. Cmd/Ctrl+Enter submits, Escape skips.
Parser question-veto
format_as_tablejoinsDESTRUCTIVE_TOOLS, so "should I format this as a table?" no longer fires a bulk restyle. Targeted tools stay deliberately excluded — the polite-framing path treats "can you highlight X" as a real command, and there is a test pinning that.Verification
npm run lint:cinpm run typechecknpm run testnpm run test --prefix servernpm run test:realengineFollow-up
The P1 formatting sandbox is stacked on this branch — see #44. Merge this first.
🤖 Generated with Claude Code
Summary by Sourcery
Improve spreadsheet action safety and AI feedback quality by surfacing formula risks before application and capturing actionable user explanations for negative responses.
New Features:
Bug Fixes:
Enhancements:
Tests: