Finish Phase 22: the half of Petal that works with the tunnel down
Grammar lite, the false-friend list, the daily invitation and the offline miscollocations — the four remaining §5–§6 items, all client-side and all alive on a box that cannot reach the model. The offline collocations forced a schema change. `type` had been doubling as the answer to "which engine found this" — `mechanics` meant offline — and that stops being true the moment an offline rule proposes a collocation. Migration 0013 adds `source` (llm | local) and every pass now scopes its DELETE by engine; without it the coach silently wiped every offline chunk on the page. Existing rows backfill by type, so a pre-0013 collocation row is claimed as the coach's, which it was: the offline list did not exist yet. The rule pack is hand-curated rather than mined, and the entries left out are the point — `married with` is wrong until "married with children", `arrive to` wants at or in depending on the noun. A pack running on every keystroke must not correct correct writing. Claude-Session: https://claude.ai/code/session_016y6gyuHkQXPiEuW8RGQyua
This commit is contained in:
@@ -90,6 +90,22 @@ type mechanicsFinding struct {
|
||||
Original string `json:"original"`
|
||||
Replacement string `json:"replacement"`
|
||||
Explanation string `json:"explanation"`
|
||||
// Which family this offline finding belongs to. Empty (the historical shape)
|
||||
// means mechanics; the miscollocation rules send 'collocation' so a chunk the
|
||||
// rule pack caught is indistinguishable from one the coach caught — same
|
||||
// family, same rail, and the same planting into the garden on accept.
|
||||
Type string `json:"type"`
|
||||
}
|
||||
|
||||
// localType maps a client-supplied family onto the two an offline rule may claim.
|
||||
// Anything else — including the empty string older clients send — is mechanics,
|
||||
// so a stray label can never smuggle a row into an LLM family and survive that
|
||||
// pass's DELETE.
|
||||
func localType(t string) string {
|
||||
if strings.ToLower(strings.TrimSpace(t)) == db.SuggestionTypeCollocation {
|
||||
return db.SuggestionTypeCollocation
|
||||
}
|
||||
return db.SuggestionTypeMechanics
|
||||
}
|
||||
|
||||
// maxMechanicsFindings caps a single submission so a runaway client can't flood
|
||||
@@ -144,10 +160,16 @@ func (h *Handler) mechanics(w http.ResponseWriter, r *http.Request) {
|
||||
httputil.WriteJSON(w, http.StatusOK, out)
|
||||
}
|
||||
|
||||
// replaceMechanics swaps the document's pending mechanics rows for the supplied
|
||||
// replaceMechanics swaps the document's pending offline rows for the supplied
|
||||
// findings in one transaction, leaving the LLM families and actioned rows
|
||||
// untouched. Findings the user already accepted or dismissed are suppressed (the
|
||||
// detector has no memory between runs), and malformed spans are skipped.
|
||||
//
|
||||
// The DELETE is scoped by *source*, not by type: the rule pack owns both the
|
||||
// mechanics family and its share of the collocation family, and every run is a
|
||||
// full recompute of the document, so everything it wrote last time goes. Scoping
|
||||
// by type instead would strand offline collocations the current text no longer
|
||||
// warrants — the one row nobody would ever replace.
|
||||
func (h *Handler) replaceMechanics(docID string, findings []mechanicsFinding) error {
|
||||
tx, err := h.DB.Begin()
|
||||
if err != nil {
|
||||
@@ -156,8 +178,8 @@ func (h *Handler) replaceMechanics(docID string, findings []mechanicsFinding) er
|
||||
defer tx.Rollback()
|
||||
|
||||
if _, err := tx.Exec(
|
||||
`DELETE FROM suggestions WHERE doc_id = ? AND status = ? AND type = ?`,
|
||||
docID, db.SuggestionStatusPending, db.SuggestionTypeMechanics,
|
||||
`DELETE FROM suggestions WHERE doc_id = ? AND status = ? AND source = ?`,
|
||||
docID, db.SuggestionStatusPending, db.SuggestionSourceLocal,
|
||||
); err != nil {
|
||||
return err
|
||||
}
|
||||
@@ -175,9 +197,10 @@ func (h *Handler) replaceMechanics(docID string, findings []mechanicsFinding) er
|
||||
continue
|
||||
}
|
||||
if _, err := tx.Exec(
|
||||
`INSERT INTO suggestions (doc_id, from_pos, to_pos, original, replacement, explanation, type)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?)`,
|
||||
docID, f.From, f.To, f.Original, f.Replacement, f.Explanation, db.SuggestionTypeMechanics,
|
||||
`INSERT INTO suggestions (doc_id, from_pos, to_pos, original, replacement, explanation, type, source)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?)`,
|
||||
docID, f.From, f.To, f.Original, f.Replacement, f.Explanation,
|
||||
localType(f.Type), db.SuggestionSourceLocal,
|
||||
); err != nil {
|
||||
return err
|
||||
}
|
||||
@@ -289,17 +312,22 @@ type pendingScope struct {
|
||||
forceType string // if set, every inserted row gets this type; else normalizeType
|
||||
}
|
||||
|
||||
// Every scope below is confined to source='llm'. The offline rule pack replaces
|
||||
// its own rows wholesale on each edit (see replaceMechanics) and its findings
|
||||
// must survive all three model passes — including the collocation coach, which
|
||||
// now shares the collocation family with it.
|
||||
var (
|
||||
// grammarScope owns the grammar/phrasing/idiom/clarity flags — everything but
|
||||
// the other self-owned families (voice, collocation, mechanics), which run on
|
||||
// their own cadence/pass and must survive a grammar checkpoint. Notably the
|
||||
// deterministic mechanics pass writes its rows in the same /check request just
|
||||
// before this DELETE runs, so excluding it here is what keeps them alive.
|
||||
grammarScope = pendingScope{deleteWhere: "type NOT IN ('voice','collocation','mechanics')", forceType: ""}
|
||||
// voiceScope owns the voice flags only.
|
||||
voiceScope = pendingScope{deleteWhere: "type = 'voice'", forceType: db.SuggestionTypeVoice}
|
||||
// collocationScope owns the collocation flags only.
|
||||
collocationScope = pendingScope{deleteWhere: "type = 'collocation'", forceType: db.SuggestionTypeCollocation}
|
||||
// the other self-owned families (voice, collocation), which run on their own
|
||||
// cadence/pass and must survive a grammar checkpoint. Notably the offline pass
|
||||
// writes its rows in the same /check request just before this DELETE runs, so
|
||||
// the source clause is also what keeps them alive.
|
||||
grammarScope = pendingScope{deleteWhere: "source = 'llm' AND type NOT IN ('voice','collocation')", forceType: ""}
|
||||
// voiceScope owns the model's voice flags only.
|
||||
voiceScope = pendingScope{deleteWhere: "source = 'llm' AND type = 'voice'", forceType: db.SuggestionTypeVoice}
|
||||
// collocationScope owns the model's collocation flags only — the rule pack's
|
||||
// share of the same family is left standing.
|
||||
collocationScope = pendingScope{deleteWhere: "source = 'llm' AND type = 'collocation'", forceType: db.SuggestionTypeCollocation}
|
||||
)
|
||||
|
||||
// replacePending swaps a document's pending suggestions within one family for a
|
||||
@@ -341,9 +369,9 @@ func (h *Handler) replacePending(docID, contentText string, raw []llm.RawSuggest
|
||||
}
|
||||
from, to := locate(contentText, s.Original)
|
||||
if _, err := tx.Exec(
|
||||
`INSERT INTO suggestions (doc_id, from_pos, to_pos, original, replacement, explanation, type)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?)`,
|
||||
docID, from, to, s.Original, s.Replacement, s.Explanation, typ,
|
||||
`INSERT INTO suggestions (doc_id, from_pos, to_pos, original, replacement, explanation, type, source)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?)`,
|
||||
docID, from, to, s.Original, s.Replacement, s.Explanation, typ, db.SuggestionSourceLLM,
|
||||
); err != nil {
|
||||
return err
|
||||
}
|
||||
@@ -497,7 +525,7 @@ func (h *Handler) listForDoc(w http.ResponseWriter, r *http.Request) {
|
||||
func (h *Handler) fetchPending(userID, docID string) ([]db.Suggestion, error) {
|
||||
rows, err := h.DB.Query(
|
||||
`SELECT s.id, s.doc_id, s.from_pos, s.to_pos, s.original, s.replacement,
|
||||
s.explanation, s.type, s.status, s.created_at
|
||||
s.explanation, s.type, s.status, s.source, s.created_at
|
||||
FROM suggestions s
|
||||
JOIN documents d ON d.id = s.doc_id
|
||||
WHERE s.doc_id = ? AND d.user_id = ? AND s.status = ?
|
||||
@@ -514,7 +542,7 @@ func (h *Handler) fetchPending(userID, docID string) ([]db.Suggestion, error) {
|
||||
var s db.Suggestion
|
||||
if err := rows.Scan(
|
||||
&s.ID, &s.DocID, &s.FromPos, &s.ToPos, &s.Original, &s.Replacement,
|
||||
&s.Explanation, &s.Type, &s.Status, &s.CreatedAt,
|
||||
&s.Explanation, &s.Type, &s.Status, &s.Source, &s.CreatedAt,
|
||||
); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
@@ -526,11 +554,14 @@ func (h *Handler) fetchPending(userID, docID string) ([]db.Suggestion, error) {
|
||||
return dedupeSpans(out), nil
|
||||
}
|
||||
|
||||
// dedupeSpans resolves collisions between the deterministic mechanics family and
|
||||
// the LLM families: when a mechanics finding and an LLM suggestion fight over the
|
||||
// same characters, mechanics wins and the LLM card is dropped. Its span is exact
|
||||
// (the detector matched it), whereas the LLM positions are only advisory
|
||||
// (re-anchored by string at render), so the precise fix should own the span.
|
||||
// dedupeSpans resolves collisions between the offline rule pack and the model:
|
||||
// when a local finding and an LLM suggestion fight over the same characters, the
|
||||
// local one wins and the LLM card is dropped. Its span is exact (the detector
|
||||
// matched it), whereas the LLM positions are only advisory (re-anchored by string
|
||||
// at render), so the precise fix should own the span. This is why the split is by
|
||||
// source rather than by type — an offline miscollocation is as exact as an
|
||||
// offline comma, and the coach's fuzzy version of the same chunk shouldn't
|
||||
// double up next to it.
|
||||
//
|
||||
// This deliberately does NOT dedupe LLM-vs-LLM overlaps: voice (awareness-only,
|
||||
// no replacement) and collocation legitimately co-occupy the same span, and that
|
||||
@@ -540,7 +571,7 @@ func dedupeSpans(in []db.Suggestion) []db.Suggestion {
|
||||
type span struct{ from, to int }
|
||||
var claimed []span
|
||||
for _, s := range in {
|
||||
if s.Type == db.SuggestionTypeMechanics && s.FromPos >= 0 {
|
||||
if s.Source == db.SuggestionSourceLocal && s.FromPos >= 0 {
|
||||
claimed = append(claimed, span{s.FromPos, s.ToPos})
|
||||
}
|
||||
}
|
||||
@@ -550,7 +581,7 @@ func dedupeSpans(in []db.Suggestion) []db.Suggestion {
|
||||
|
||||
out := make([]db.Suggestion, 0, len(in))
|
||||
for _, s := range in {
|
||||
if s.Type != db.SuggestionTypeMechanics && s.FromPos >= 0 {
|
||||
if s.Source != db.SuggestionSourceLocal && s.FromPos >= 0 {
|
||||
overlaps := false
|
||||
for _, sp := range claimed {
|
||||
if s.FromPos < sp.to && sp.from < s.ToPos {
|
||||
@@ -559,7 +590,7 @@ func dedupeSpans(in []db.Suggestion) []db.Suggestion {
|
||||
}
|
||||
}
|
||||
if overlaps {
|
||||
continue // an exact mechanics fix owns these characters
|
||||
continue // an exact offline fix owns these characters
|
||||
}
|
||||
}
|
||||
out = append(out, s)
|
||||
|
||||
@@ -0,0 +1,203 @@
|
||||
package suggestions
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"testing"
|
||||
|
||||
"gitea.parodia.dev/drwily/petal/internal/db"
|
||||
)
|
||||
|
||||
// The offline rule pack and the LLM now share the collocation family, which is
|
||||
// the point: the writer sees one rail and is never told which engine spoke. What
|
||||
// makes that safe is `source` — each pass replaces only its own rows. These tests
|
||||
// pin the two ways that could go wrong, both of which the old type-scoped DELETEs
|
||||
// would have hit.
|
||||
|
||||
// pendingOfType counts the pending rows of one family in a response body.
|
||||
func pendingOfType(got []db.Suggestion, typ string) []db.Suggestion {
|
||||
var out []db.Suggestion
|
||||
for _, s := range got {
|
||||
if s.Type == typ {
|
||||
out = append(out, s)
|
||||
}
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// TestOfflineCollocationFilesAsCollocation proves a miscollocation the rule pack
|
||||
// found is stored in the collocation family (so accepting it plants a garden
|
||||
// card, exactly as the coach's would) while still being marked as locally found.
|
||||
func TestOfflineCollocationFilesAsCollocation(t *testing.T) {
|
||||
srv, docID, _ := newTestServer(t, &stubClient{response: `{"suggestions":[]}`})
|
||||
|
||||
got := postMechanics(t, srv, docID, `[
|
||||
{"from":0,"to":13,"original":"do a decision","replacement":"make a decision","explanation":"pairing","type":"collocation"},
|
||||
{"from":20,"to":27,"original":"the the","replacement":"the","explanation":"doubled word","type":"mechanics"}
|
||||
]`)
|
||||
if len(got) != 2 {
|
||||
t.Fatalf("want both findings, got %+v", got)
|
||||
}
|
||||
coll := pendingOfType(got, db.SuggestionTypeCollocation)
|
||||
if len(coll) != 1 {
|
||||
t.Fatalf("want 1 collocation, got %+v", got)
|
||||
}
|
||||
if coll[0].Source != db.SuggestionSourceLocal {
|
||||
t.Errorf("offline finding should be source=local, got %q", coll[0].Source)
|
||||
}
|
||||
if mech := pendingOfType(got, db.SuggestionTypeMechanics); len(mech) != 1 {
|
||||
t.Fatalf("want 1 mechanics finding, got %+v", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestUnknownLocalTypeFallsBackToMechanics: a family the offline pass isn't
|
||||
// allowed to claim (or an older client sending none at all) must land in
|
||||
// mechanics. Otherwise a stray label would smuggle a row into an LLM family,
|
||||
// where nothing would ever replace it.
|
||||
func TestUnknownLocalTypeFallsBackToMechanics(t *testing.T) {
|
||||
srv, docID, _ := newTestServer(t, &stubClient{response: `{"suggestions":[]}`})
|
||||
|
||||
got := postMechanics(t, srv, docID, `[
|
||||
{"from":0,"to":5,"original":"aaaaa","replacement":"bbbbb","explanation":"x","type":"voice"},
|
||||
{"from":6,"to":11,"original":"ccccc","replacement":"ddddd","explanation":"y"}
|
||||
]`)
|
||||
if len(got) != 2 {
|
||||
t.Fatalf("want 2 findings, got %+v", got)
|
||||
}
|
||||
for _, s := range got {
|
||||
if s.Type != db.SuggestionTypeMechanics {
|
||||
t.Errorf("offline finding claimed family %q; only mechanics/collocation are allowed", s.Type)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestCoachDoesNotWipeOfflineCollocations is the collision the source column
|
||||
// exists for: the LLM collocation pass replaces the collocation family, and the
|
||||
// rule pack's share of that family has to survive it. Before `source`, running
|
||||
// the coach silently deleted every offline chunk on the page.
|
||||
func TestCoachDoesNotWipeOfflineCollocations(t *testing.T) {
|
||||
client := &stubClient{response: `{"suggestions":[
|
||||
{"original":"apple","replacement":"an apple","explanation":"article","type":"collocation"}
|
||||
]}`}
|
||||
srv, docID, _ := newTestServer(t, client)
|
||||
|
||||
// The seeded doc is "I has two apple." — the coach's flag anchors on "apple"
|
||||
// at [10,15], so the offline finding is given a span well clear of it. Two
|
||||
// findings fighting over the same characters is a different rule (see
|
||||
// TestOfflineCardWinsSpanCollision); this test is about the DELETE.
|
||||
postMechanics(t, srv, docID, `[
|
||||
{"from":0,"to":5,"original":"do a decision","replacement":"make a decision","explanation":"pairing","type":"collocation"}
|
||||
]`)
|
||||
|
||||
rec := do(t, srv, http.MethodPost, "/docs/"+docID+"/collocation", "")
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("collocation pass: code=%d body=%s", rec.Code, rec.Body)
|
||||
}
|
||||
var got []db.Suggestion
|
||||
if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil {
|
||||
t.Fatalf("decode: %v", err)
|
||||
}
|
||||
|
||||
var local, llm int
|
||||
for _, s := range pendingOfType(got, db.SuggestionTypeCollocation) {
|
||||
if s.Source == db.SuggestionSourceLocal {
|
||||
local++
|
||||
} else {
|
||||
llm++
|
||||
}
|
||||
}
|
||||
if local != 1 {
|
||||
t.Errorf("the coach wiped the offline collocation: local=%d, got %+v", local, got)
|
||||
}
|
||||
if llm != 1 {
|
||||
t.Errorf("want the coach's own flag alongside it: llm=%d, got %+v", llm, got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestOfflinePassReplacesItsOwnCollocations is the mirror: the rule pack
|
||||
// recomputes the whole document every run, so a chunk the current text no longer
|
||||
// warrants must go — and the coach's flags must stay. Scoping the offline DELETE
|
||||
// by type instead of source would have stranded the first row forever.
|
||||
func TestOfflinePassReplacesItsOwnCollocations(t *testing.T) {
|
||||
client := &stubClient{response: `{"suggestions":[
|
||||
{"original":"apple","replacement":"an apple","explanation":"article","type":"collocation"}
|
||||
]}`}
|
||||
srv, docID, _ := newTestServer(t, client)
|
||||
|
||||
// A coach flag, then an offline chunk, then a rerun that no longer finds it.
|
||||
do(t, srv, http.MethodPost, "/docs/"+docID+"/collocation", "")
|
||||
postMechanics(t, srv, docID, `[
|
||||
{"from":0,"to":13,"original":"do a decision","replacement":"make a decision","explanation":"pairing","type":"collocation"}
|
||||
]`)
|
||||
got := postMechanics(t, srv, docID, `[]`)
|
||||
|
||||
for _, s := range got {
|
||||
if s.Source == db.SuggestionSourceLocal {
|
||||
t.Errorf("stale offline finding survived a recompute: %+v", s)
|
||||
}
|
||||
}
|
||||
if len(pendingOfType(got, db.SuggestionTypeCollocation)) != 1 {
|
||||
t.Fatalf("the coach's own flag should be untouched, got %+v", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestOfflineCollocationPlantsOnAccept closes the loop the family split was for:
|
||||
// a chunk the rule pack found, accepted, becomes a vocabulary-garden card — with
|
||||
// no model involved anywhere in the path.
|
||||
func TestOfflineCollocationPlantsOnAccept(t *testing.T) {
|
||||
srv, docID, h := newTestServer(t, &stubClient{response: `{"suggestions":[]}`})
|
||||
if _, err := h.DB.Exec(
|
||||
`UPDATE documents SET content_text = ? WHERE id = ?`,
|
||||
"I had to do a decision about the job.", docID,
|
||||
); err != nil {
|
||||
t.Fatalf("set content: %v", err)
|
||||
}
|
||||
|
||||
got := postMechanics(t, srv, docID, `[
|
||||
{"from":9,"to":22,"original":"do a decision","replacement":"make a decision","explanation":"pairing","type":"collocation"}
|
||||
]`)
|
||||
if len(got) != 1 {
|
||||
t.Fatalf("want the offline chunk, got %+v", got)
|
||||
}
|
||||
if rec := do(t, srv, http.MethodPost, "/suggestions/"+got[0].ID+"/accept", ""); rec.Code != http.StatusNoContent {
|
||||
t.Fatalf("accept: code=%d body=%s", rec.Code, rec.Body)
|
||||
}
|
||||
|
||||
cards := gardenCards(t, h)
|
||||
if len(cards) != 1 || cards[0].word != "make a decision" {
|
||||
t.Fatalf("want a planted phrase card, got %+v", cards)
|
||||
}
|
||||
// The example is the corrected sentence — the phrasing she kept, not the one
|
||||
// she just left behind.
|
||||
if cards[0].example != "I had to make a decision about the job." {
|
||||
t.Errorf("example should be the corrected sentence, got %q", cards[0].example)
|
||||
}
|
||||
}
|
||||
|
||||
// TestOfflineCardWinsSpanCollision: the tiebreak is by engine, not by family. An
|
||||
// offline miscollocation has an exact span; the coach's overlapping flag is only
|
||||
// advisory, so it is the one that goes.
|
||||
func TestOfflineCardWinsSpanCollision(t *testing.T) {
|
||||
client := &stubClient{response: `{"suggestions":[
|
||||
{"original":"do a decision about","replacement":"decide about","explanation":"wordy","type":"collocation"}
|
||||
]}`}
|
||||
srv, docID, h := newTestServer(t, client)
|
||||
if _, err := h.DB.Exec(
|
||||
`UPDATE documents SET content_text = ? WHERE id = ?`,
|
||||
"I had to do a decision about the job.", docID,
|
||||
); err != nil {
|
||||
t.Fatalf("set content: %v", err)
|
||||
}
|
||||
|
||||
do(t, srv, http.MethodPost, "/docs/"+docID+"/collocation", "")
|
||||
got := postMechanics(t, srv, docID, `[
|
||||
{"from":9,"to":22,"original":"do a decision","replacement":"make a decision","explanation":"pairing","type":"collocation"}
|
||||
]`)
|
||||
|
||||
if len(got) != 1 {
|
||||
t.Fatalf("want the overlapping coach flag dropped, got %+v", got)
|
||||
}
|
||||
if got[0].Source != db.SuggestionSourceLocal {
|
||||
t.Errorf("the exact offline card should own the span, got %+v", got[0])
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user