drwily/petal
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Suppress fickle re-edits of sentences the user already settled

Browse Source 
Suppression keyed on the exact original->replacement pair, which the
model routinely sidestepped: it reverses an accepted edit (reverse
pair), re-polishes its own accepted output (new original == accepted
replacement), and the editor's smart-quote churn ("..." -> '...')
defeated even a byte-exact match. Result: a few sentences got nudged
back and forth pass after pass.

Replace the exact-pair actionedKeys with a suppressor that compares
under a normalization folding all quote variants and collapsing
whitespace, and drops a fresh suggestion when it re-touches an
already-settled span: same edit re-proposed, an original the user
already accepted/dismissed, the model re-touching its own accepted
output, or a multi-word sub-clause contained in an accepted span.

Tradeoff: once a sentence is accepted/dismissed it won't be re-flagged
until its text changes — stability over marginal improvement, the right
call for the calm ESL persona.

Claude-Session: https://claude.ai/code/session_016Yr6jELuRc7hyzYLccQKZd
This commit is contained in:
prosolis
2026-06-27 00:18:23 -07:00
parent 08b801e752
commit 9d2501a625
2 changed files with 168 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files

159
internal/suggestions/handlers.go
View File

@@ -155,7 +155,7 @@ func (h *Handler) replaceMechanics(docID string, findings []mechanicsFinding) er
return err
}
actioned, err := actionedKeys(tx, docID)
sup, err := buildSuppressor(tx, docID)
if err != nil {
return err
}
@@ -164,7 +164,7 @@ func (h *Handler) replaceMechanics(docID string, findings []mechanicsFinding) er
if f.From < 0 || f.To <= f.From || strings.TrimSpace(f.Original) == "" {
continue // malformed span — the client re-anchors by string anyway
}
if _, seen := actioned[suggestionKey(f.Original, f.Replacement)]; seen {
if sup.suppressed(f.Original, f.Replacement) {
continue
}
if _, err := tx.Exec(
@@ -289,13 +289,12 @@ var (
// fresh batch in a single transaction. Accepted/rejected suggestions and the
// other family's pending rows are left untouched.
//
// Suggestions the user already accepted or dismissed are suppressed from the
// fresh batch: the model has no memory between passes, so without this it would
// re-propose the identical edit on the very next checkpoint — re-nagging a
// sentence the user already resolved. This matters most when an accept silently
// no-ops (the `original` text couldn't be anchored in the editor, so the doc
// never changed): the sentence is unaltered, yet the user shouldn't see the same
// card again.
// Suggestions touching a sentence the user already settled are suppressed from
// the fresh batch (see suppressor): not just the identical edit re-proposed, but
// reversals and re-polishing of the model's own just-accepted output — the
// "fickle, keeps going back and forth on a few sentences" behavior. The model has
// no memory between passes, so without this it re-opens resolved sentences every
// checkpoint.
func (h *Handler) replacePending(docID, contentText string, raw []llm.RawSuggestion, scope pendingScope) error {
tx, err := h.DB.Begin()
if err != nil {
@@ -310,13 +309,13 @@ func (h *Handler) replacePending(docID, contentText string, raw []llm.RawSuggest
return err
}
actioned, err := actionedKeys(tx, docID)
sup, err := buildSuppressor(tx, docID)
if err != nil {
return err
}
for _, s := range raw {
if _, seen := actioned[suggestionKey(s.Original, s.Replacement)]; seen {
if sup.suppressed(s.Original, s.Replacement) {
continue
}
typ := scope.forceType
@@ -336,37 +335,131 @@ func (h *Handler) replacePending(docID, contentText string, raw []llm.RawSuggest
return tx.Commit()
}
// actionedKeys returns the set of original→replacement keys the user has already
// accepted or rejected for this document, so a fresh checkpoint can skip
// re-proposing them. Keyed on the trimmed original+replacement pair, matching the
// normalization ParseCheckpoint applies, so a genuinely different edit on the same
// sentence is not suppressed.
func actionedKeys(tx *sql.Tx, docID string) (map[string]struct{}, error) {
// dedupQuoteReplacer folds every straight/curly single- and double-quote variant
// (and backtick/acute accent) onto one canonical character. The editor and the
// model both rewrite quotes between passes — a sentence accepted with "…" comes
// back flagged with '…' — so without folding, byte-identical text reads as a
// different edit and the suppression below misses it. (This normalization is for
// dedup ONLY; the frontend still anchors on the verbatim `original`.)
var dedupQuoteReplacer = strings.NewReplacer(
"‘", "'", "’", "'", "‚", "'", "‛", "'", // single curly
"“", "'", "”", "'", "„", "'", "″", "'", // double curly
"\"", "'", "`", "'", "´", "'", // straight double, backtick, acute
)
// normalizeForDedup canonicalizes a string for suppression comparisons: quotes
// folded (above) and runs of whitespace collapsed to single spaces (so a reflowed
// paragraph still matches). Used only to decide what to suppress, never to alter
// stored or rendered text.
func normalizeForDedup(s string) string {
return strings.Join(strings.Fields(dedupQuoteReplacer.Replace(s)), " ")
}
// suppressor decides which fresh suggestions to drop because the user has already
// settled the sentence they touch. The model has no memory between passes, so on
// every checkpoint it re-examines the current text and proposes edits — including
// ones that re-open a sentence the user already resolved. Three families of those
// are suppressed (all compared under normalizeForDedup):
//
// - pairs: the identical edit, re-proposed verbatim (the original "accept it,
// then it nags again" case; also covers a silent no-op accept that left the
// text unchanged).
// - actionedOrig: any edit whose original is a span the user already accepted or
// dismissed an edit on — "you already decided about this exact sentence."
// - acceptedRepl: any edit whose original is text the user accepted AS a
// replacement — i.e. the model re-touching its own just-accepted output, which
// is how the reversals and endless re-polishing arise (accept "due to the
// rain", next pass proposes changing "due to the rain" back). Guarded to
// multi-word spans so word-level fixes aren't swept up as collateral.
// - acceptedReplList holds the same accepted replacements for a containment
// check: the model evades the exact acceptedRepl match by re-flagging a
// *sub-clause* of an accepted sentence (flag "she was…due to the rain" instead
// of the whole sentence). When one of the new original / an accepted
// replacement contains the other and the shorter side is substantial
// (>= minContainWords words), it's the same settled span and is dropped.
type suppressor struct {
pairs map[string]struct{}
actionedOrig map[string]struct{}
acceptedRepl map[string]struct{}
acceptedReplList []string
}
// minContainWords is the floor for the containment check: the shorter of the two
// spans must be at least this many words before a substring relationship counts
// as "the same settled text." High enough that an incidental common phrase ("the
// rain") can't suppress an unrelated sentence, low enough to catch a re-flagged
// clause.
const minContainWords = 4
// suppressed reports whether a fresh suggestion should be dropped as already
// settled. An empty original is never suppressed here (it can't anchor anyway and
// is dropped upstream).
func (s suppressor) suppressed(original, replacement string) bool {
o := normalizeForDedup(original)
if o == "" {
return false
}
if _, ok := s.pairs[o+"\x00"+normalizeForDedup(replacement)]; ok {
return true
}
if _, ok := s.actionedOrig[o]; ok {
return true
}
if strings.ContainsRune(o, ' ') {
if _, ok := s.acceptedRepl[o]; ok {
return true
}
}
// Containment: the model re-flagged a sub-clause of (or a window around) an
// accepted span. Suppress when one contains the other and the shorter side is
// a substantial multi-word run.
for _, r := range s.acceptedReplList {
shorter, longer := o, r
if len(r) < len(o) {
shorter, longer = r, o
}
if len(strings.Fields(shorter)) >= minContainWords && strings.Contains(longer, shorter) {
return true
}
}
return false
}
// buildSuppressor loads the document's accepted/rejected edits and indexes them
// into the three suppression families described on suppressor.
func buildSuppressor(tx *sql.Tx, docID string) (suppressor, error) {
rows, err := tx.Query(
`SELECT original, replacement FROM suggestions
`SELECT original, replacement, status FROM suggestions
WHERE doc_id = ? AND status IN (?, ?)`,
docID, db.SuggestionStatusAccepted, db.SuggestionStatusRejected,
)
if err != nil {
return nil, err
return suppressor{}, err
}
defer rows.Close()
keys := make(map[string]struct{})
for rows.Next() {
var original, replacement string
if err := rows.Scan(&original, &replacement); err != nil {
return nil, err
}
keys[suggestionKey(original, replacement)] = struct{}{}
s := suppressor{
pairs: make(map[string]struct{}),
actionedOrig: make(map[string]struct{}),
acceptedRepl: make(map[string]struct{}),
}
return keys, rows.Err()
}
// suggestionKey is the dedup identity for a suggestion: its trimmed original and
// replacement text. Two suggestions with the same key are "the same edit."
func suggestionKey(original, replacement string) string {
return strings.TrimSpace(original) + "\x00" + strings.TrimSpace(replacement)
for rows.Next() {
var original, replacement, status string
if err := rows.Scan(&original, &replacement, &status); err != nil {
return suppressor{}, err
}
o := normalizeForDedup(original)
r := normalizeForDedup(replacement)
s.pairs[o+"\x00"+r] = struct{}{}
if o != "" {
s.actionedOrig[o] = struct{}{}
}
if status == db.SuggestionStatusAccepted && r != "" {
s.acceptedRepl[r] = struct{}{}
s.acceptedReplList = append(s.acceptedReplList, r)
}
}
return s, rows.Err()
}
// listForDoc returns the document's current pending suggestions (used when the

42
internal/suggestions/handlers_test.go
View File

@@ -165,6 +165,48 @@ func TestResolvedSuggestionsNotReproposed(t *testing.T) {
}
}
// TestFickleEditsSuppressed proves the suppressor kills the "keeps going back and
// forth on a few sentences" behavior seen live on the Missing Key doc: a later
// pass that (a) reverses an edit the user just accepted — even with the editor's
// double→single quote churn that defeats a byte-exact match — or (b) re-polishes
// the model's own accepted output. A genuinely new edit on a fresh sentence still
// survives.
func TestFickleEditsSuppressed(t *testing.T) {
client := &stubClient{response: `{"suggestions":[
{"original":"He left \"early,\" because of the rain.","replacement":"He left \"early,\" due to the rain.","explanation":"smoother","type":"phrasing"},
{"original":"The cat always have a calm face.","replacement":"The cat always has a calm face.","explanation":"agreement","type":"grammar"}
]}`}
srv, docID, h := newTestServer(t, client)
h.Limit = llm.NewRateLimiter(0)
rec := do(t, srv, http.MethodPost, "/docs/"+docID+"/check", "")
var got []db.Suggestion
_ = json.Unmarshal(rec.Body.Bytes(), &got)
if len(got) != 2 {
t.Fatalf("first pass: want 2, got %d", len(got))
}
for _, s := range got {
do(t, srv, http.MethodPost, "/suggestions/"+s.ID+"/accept", "")
}
// Reversal of the first accept (note the " → ' quote churn) and a re-polish of
// the second accept must both be dropped; only the unrelated edit survives.
client.response = `{"suggestions":[
{"original":"He left 'early,' due to the rain.","replacement":"He left 'early,' because of the rain.","explanation":"reverts the accepted edit","type":"phrasing"},
{"original":"The cat always has a calm face.","replacement":"The cat always seems to have a calm face.","explanation":"re-polishes accepted output","type":"phrasing"},
{"original":"due to the rain","replacement":"because of the rain","explanation":"sub-clause reversal of the accepted span","type":"phrasing"},
{"original":"She drived home.","replacement":"She drove home.","explanation":"past tense","type":"grammar"}
]}`
rec = do(t, srv, http.MethodPost, "/docs/"+docID+"/check", "")
var again []db.Suggestion
if err := json.Unmarshal(rec.Body.Bytes(), &again); err != nil {
t.Fatalf("decode: %v", err)
}
if len(again) != 1 || again[0].Original != "She drived home." {
t.Fatalf("want only the new edit to survive, got %d: %+v", len(again), again)
}
}
// TestVoicePassCoexists proves the voice pass and the grammar checkpoint own
// disjoint pending families: running one never wipes the other's flags, and
// each endpoint returns the unified pending set.