From 9552cfc2058aa09c6c8b5d4b02d8700bc9fbce17 Mon Sep 17 00:00:00 2001 From: Sam Scholten Date: Thu, 15 Oct 2026 07:45:05 +1000 Subject: fix read/star label issue --- cmds/export.go | 83 ++++++++++++++++++++++++++++-------- cmds/export_test.go | 120 +++++++++++++++++++++++++++++++++++++++++++++++++--- cmds/relabel.go | 77 +++++++++++++++++++++++++++++++++ core/labels.go | 5 ++- justfile | 5 +++ 5 files changed, 265 insertions(+), 25 deletions(-) create mode 100644 cmds/relabel.go diff --git a/cmds/export.go b/cmds/export.go index 203a7ef..9b153ee 100644 --- a/cmds/export.go +++ b/cmds/export.go @@ -2,9 +2,13 @@ // labels.jsonl, the canonical label store. // // Positives: starred entries, incremental via a persisted changed_after -// cursor, deduped by ID then normalized URL. Negatives: seeded random -// sample of recent read+unstarred entries, topped up to a fixed ratio of -// the positive pool. Also carries --dump-raw fixture capture. +// cursor, deduped by ID then normalized URL. A star outranks the read state +// that put an entry in the negative pool: it flips the stored negative row +// (see relabel.go), or — for a fluxrec pick whose original was never +// exported — is recorded as its own positive. +// Negatives: seeded random sample of recent read+unstarred entries, topped +// up to a fixed ratio of the positive pool. Also carries --dump-raw fixture +// capture. package cmds import ( @@ -48,6 +52,12 @@ No single feed may hold more than --neg-feed-cap-pct percent of the negative target (floored at 50 rows), so a few firehoses can't own the negative class. Each run also writes a JSON summary to --run-out. +A star always wins over the read state that produced a negative: a starred +entry already labeled negative is relabeled to positive in place (identity +kept, fresh captured_at, original copied to LABELS.bak), and a star on a +rec-feed item whose original was never exported is added as its own +positive (URL is the original's; title/feed identity are the rec copy's). + With --dump-raw, instead fetch ~25 starred and ~100 read unstarred entries and write the raw API responses to DIR for offline development fixtures. `) @@ -108,7 +118,7 @@ func saveExportState(path string, st exportState) error { // exportCounts is what the run reports to stdout. type exportCounts struct { - seen, appended, idDup, urlDup, recHit, recUnresolved int + seen, appended, idDup, urlDup, recHit, recAdded, relabeled int } func (c *ExportCommand) runExport(stdout io.Writer) error { @@ -143,6 +153,10 @@ func (c *ExportCommand) runExport(stdout io.Writer) error { var counts exportCounts var newRows []core.LabelRow + // Every star seen this run, so rows already in the store can be flipped + // to positive after the walk (relabelStars). + starIDs := make(map[int64]bool) + starURLs := make(map[string]bool) maxChanged := state.ChangedAfter now := time.Now().UTC() @@ -159,16 +173,30 @@ func (c *ExportCommand) runExport(stdout io.Writer) error { if ts := e.ChangedAt.Unix(); ts > maxChanged { maxChanged = ts } - - // Stars on the recommendations feed are not original entries; - // resolve by URL against existing labels. If the original was - // never exported, there is nothing to attach the star to. + starIDs[e.ID] = true + starURLs[core.NormalizeURL(e.URL)] = true + + // Stars on the recommendations feed are not original entries. + // A star outranks whatever the original's read state said, so + // resolve by URL against existing labels when possible (the + // relabel pass below flips a stored negative), and otherwise + // record the star as its own positive: the URL is the original + // article's, only the title/feed identity come from the rec copy. if c.RecFeedID != 0 && e.Feed.ID == c.RecFeedID { if seenURLs[core.NormalizeURL(e.URL)] { counts.recHit++ - } else { - counts.recUnresolved++ + continue } + nu := core.NormalizeURL(e.URL) + newRows = append(newRows, core.LabelRow{ + EntryID: e.ID, URL: e.URL, Title: e.Title, + FeedID: e.Feed.ID, FeedTitle: e.Feed.Title, + Label: core.LabelPos, CapturedAt: now, + }) + seenIDs[e.ID] = true + seenURLs[nu] = true + counts.appended++ + counts.recAdded++ continue } @@ -196,6 +224,18 @@ func (c *ExportCommand) runExport(stdout io.Writer) error { } } + // A star on an entry already in the store as a negative flips that row: + // same identity, label positive, captured_at refreshed so the star reads + // as recent evidence. Rewritten in place, backup first — the rows the + // dedup counters above skipped as duplicates are exactly the ones this + // rescues. + if flipped := relabelStars(existing, starIDs, starURLs, now); flipped > 0 { + if err := rewriteLabels(c.Labels, c.Labels+".bak", existing); err != nil { + return err + } + counts.relabeled = flipped + } + if err := core.AppendLabels(c.Labels, newRows); err != nil { return err } @@ -219,11 +259,16 @@ func (c *ExportCommand) runExport(stdout io.Writer) error { } posTotal += counts.appended - fmt.Fprintf(stdout, "export: seen=%d appended=%d (id-dup=%d url-dup=%d rec-hit=%d rec-unresolved=%d) pos-total=%d\n", - counts.seen, counts.appended, counts.idDup, counts.urlDup, - counts.recHit, counts.recUnresolved, posTotal) - if counts.recUnresolved > 0 { - fmt.Fprintln(stdout, "note: rec-feed stars without a matching labeled original were skipped") + fmt.Fprintf(stdout, "export: seen=%d appended=%d relabeled=%d (id-dup=%d url-dup=%d rec-hit=%d rec-added=%d) pos-total=%d\n", + counts.seen, counts.appended, counts.relabeled, counts.idDup, counts.urlDup, + counts.recHit, counts.recAdded, posTotal) + if counts.relabeled > 0 { + fmt.Fprintf(stdout, "note: relabeled %d starred entries that were stored as negatives (backup: %s)\n", + counts.relabeled, c.Labels+".bak") + } + if counts.recAdded > 0 { + fmt.Fprintf(stdout, "note: %d rec-feed stars had no labeled original and were added as positives\n", + counts.recAdded) } nc, err := c.exportNegatives(client, seenIDs, seenURLs, negPerFeed, posTotal, negTotal, now, stdout) @@ -235,8 +280,9 @@ func (c *ExportCommand) runExport(stdout io.Writer) error { PositivesSeen: counts.seen, PositivesNew: counts.appended, PosTotal: posTotal, NegCandidates: nc.candidates, NegNew: nc.appended, NegTotal: negTotal + nc.appended, NegCapped: nc.capped, - IDDup: counts.idDup, URLDup: counts.urlDup, - RecHit: counts.recHit, RecUnresolved: counts.recUnresolved, + Relabeled: counts.relabeled, + IDDup: counts.idDup, URLDup: counts.urlDup, + RecHit: counts.recHit, RecAdded: counts.recAdded, }) } @@ -359,10 +405,11 @@ type runSummary struct { NegNew int `json:"neg_new"` NegTotal int `json:"neg_total"` NegCapped int `json:"neg_capped"` + Relabeled int `json:"relabeled"` IDDup int `json:"id_dup"` URLDup int `json:"url_dup"` RecHit int `json:"rec_hit"` - RecUnresolved int `json:"rec_unresolved"` + RecAdded int `json:"rec_added"` } func writeRunSummary(path string, s runSummary) error { diff --git a/cmds/export_test.go b/cmds/export_test.go index 25eadbb..0be1ac3 100644 --- a/cmds/export_test.go +++ b/cmds/export_test.go @@ -218,6 +218,105 @@ func TestExportURLDedup(t *testing.T) { } } +// Read-then-starred: an entry sampled as a negative, starred later, must flip +// to positive (identity kept, captured_at refreshed) rather than be dropped as +// a duplicate — the star is the signal, and a stale negative row is worse than +// no row at all. +func TestExportRelabelsStarredNegative(t *testing.T) { + p := &pagedServer{starred: makeEntries(1, 1000, true, 7)} + dir := t.TempDir() + labelsPath := filepath.Join(dir, "labels.jsonl") + + old := time.Now().Add(-48 * time.Hour).UTC() + pre := core.LabelRow{ + EntryID: 1000, URL: "https://example.com/post/1000", Title: "Title 1000", + FeedID: 7, FeedTitle: "Fixture Feed", Label: core.LabelNeg, + CapturedAt: old, + } + if err := core.AppendLabels(labelsPath, []core.LabelRow{pre}); err != nil { + t.Fatal(err) + } + + out := runExportAgainst(t, p, dir) + if !strings.Contains(out, "relabeled=1") || !strings.Contains(out, "appended=0") { + t.Errorf("expected relabeled=1 appended=0, got:\n%s", out) + } + + labels, err := core.ReadLabels(labelsPath) + if err != nil { + t.Fatal(err) + } + if len(labels) != 1 { + t.Fatalf("relabel must rewrite in place, got %d rows", len(labels)) + } + got := labels[0] + if got.Label != core.LabelPos { + t.Errorf("label = %q, want pos", got.Label) + } + if got.EntryID != 1000 || got.Title != "Title 1000" || got.FeedID != 7 { + t.Errorf("relabel must keep the row's identity, got %+v", got) + } + if !got.CapturedAt.After(old) { + t.Errorf("captured_at = %v, want refreshed past %v", got.CapturedAt, old) + } + + // The pre-rewrite file is preserved as a backup. + bak, err := core.ReadLabels(labelsPath + ".bak") + if err != nil { + t.Fatalf("read backup: %v", err) + } + if len(bak) != 1 || bak[0].Label != core.LabelNeg { + t.Errorf("backup should hold the pre-flip negative row, got %+v", bak) + } + + body, err := os.ReadFile(filepath.Join(dir, "export_run.json")) + if err != nil { + t.Fatal(err) + } + var rs runSummary + if err := json.Unmarshal(body, &rs); err != nil { + t.Fatal(err) + } + if rs.Relabeled != 1 || rs.PosTotal != 1 || rs.NegTotal != 0 { + t.Errorf("run summary should reflect the flip: %+v", rs) + } +} + +// The same flip has to work through rec-feed resolution: the starred row is +// the rec feed's copy, so only the URL ties it to the labeled original. +func TestExportRelabelsRecFeedStar(t *testing.T) { + rec := makeEntries(1, 3000, true, 99) + rec[0].URL = "https://example.com/post/known" + p := &pagedServer{starred: rec} + dir := t.TempDir() + labelsPath := filepath.Join(dir, "labels.jsonl") + + pre := core.LabelRow{ + EntryID: 1234, URL: "https://example.com/post/known", Title: "Known", + FeedID: 7, FeedTitle: "Orig", Label: core.LabelNeg, + CapturedAt: time.Now().Add(-48 * time.Hour).UTC(), + } + if err := core.AppendLabels(labelsPath, []core.LabelRow{pre}); err != nil { + t.Fatal(err) + } + + out := runExportAgainst(t, p, dir, "--rec-feed-id", "99") + if !strings.Contains(out, "relabeled=1") || !strings.Contains(out, "rec-hit=1") { + t.Errorf("expected relabeled=1 rec-hit=1, got:\n%s", out) + } + + labels, err := core.ReadLabels(labelsPath) + if err != nil { + t.Fatal(err) + } + if len(labels) != 1 || labels[0].Label != core.LabelPos { + t.Fatalf("original row should be flipped to pos, got %+v", labels) + } + if labels[0].EntryID != 1234 { + t.Errorf("relabel must keep the original entry id, got %d", labels[0].EntryID) + } +} + func TestExportRecFeedResolution(t *testing.T) { // Two starred entries ON the recommendations feed (feed id 99). rec := makeEntries(2, 3000, true, 99) @@ -237,18 +336,27 @@ func TestExportRecFeedResolution(t *testing.T) { } out := runExportAgainst(t, p, dir, "--rec-feed-id", "99") - if !strings.Contains(out, "rec-hit=1") || !strings.Contains(out, "rec-unresolved=1") { - t.Errorf("expected rec-hit=1 rec-unresolved=1, got:\n%s", out) + if !strings.Contains(out, "rec-hit=1") || !strings.Contains(out, "rec-added=1") { + t.Errorf("expected rec-hit=1 rec-added=1, got:\n%s", out) } - if !strings.Contains(out, "appended=0") { - t.Errorf("rec-feed entries must never append rows, got:\n%s", out) + if !strings.Contains(out, "appended=1") { + t.Errorf("an unresolved rec-feed star should append its own positive, got:\n%s", out) } labels, err := core.ReadLabels(filepath.Join(dir, "labels.jsonl")) if err != nil { t.Fatal(err) } - if len(labels) != 1 { - t.Errorf("labels file should be untouched, has %d rows", len(labels)) + if len(labels) != 2 { + t.Fatalf("expected the seeded row plus one added star, got %d rows", len(labels)) + } + // The resolved star kept the original row (id 1234); the unresolved one + // is a positive carrying the rec copy's id but the original URL. + added := labels[1] + if added.Label != core.LabelPos || added.EntryID != 3001 { + t.Errorf("added row = %+v, want positive for rec entry 3001", added) + } + if added.URL != "https://example.com/post/unknown" { + t.Errorf("added row URL = %q, want the recommendation's article link", added.URL) } } diff --git a/cmds/relabel.go b/cmds/relabel.go new file mode 100644 index 0000000..9233daf --- /dev/null +++ b/cmds/relabel.go @@ -0,0 +1,77 @@ +// Star-driven relabeling of the label store. +// +// Negatives are sampled from read+unstarred entries, so "sampled as a +// negative, then starred" is a real trajectory: at capture time the entry was +// unstarred, and later samsc read it and liked it. Without relabeling that +// star changes nothing — the entry is already in the store, so it is deduped +// away by ID or URL — and the row keeps teaching the model that samsc did not +// want the very article he starred. +// +// That is the one feedback channel the recommendations feed creates (a star on +// a fluxrec pick whose original was already labeled), so it has to land as a +// positive rather than be swallowed as a duplicate. +// +// Flipping rewrites labels.jsonl in place: the same deliberate exception +// compact-labels takes (see core/labels.go). The row keeps its identity +// (entry, feed, title) and only its label and captured_at change, so the star +// reads as recent evidence — which is what the time-split validation window +// should see. The original file is copied to a backup before the rewrite. +package cmds + +import ( + "bytes" + "encoding/json" + "fmt" + "os" + "time" + + "fluxrec/core" +) + +// relabelStars flips negative rows to positives for entries starred since they +// were labeled. ids and urls are this run's stars: entry IDs as seen on the +// starred entries, and normalized URLs (which is how a star on a rec-feed item +// resolves to the original row). Rows are mutated in place; the count of +// flipped rows is returned. +func relabelStars(rows []core.LabelRow, ids map[int64]bool, urls map[string]bool, now time.Time) int { + flipped := 0 + for i := range rows { + if rows[i].Label != core.LabelNeg { + continue + } + if !ids[rows[i].EntryID] && !urls[core.NormalizeURL(rows[i].URL)] { + continue + } + rows[i].Label = core.LabelPos + rows[i].CapturedAt = now + flipped++ + } + return flipped +} + +// rewriteLabels replaces path with rows, copying the original to backup first +// (backup == "" skips the copy). Same rules as compact-labels' rewrite: the +// backup is written before anything is touched, so an interrupted rewrite +// leaves a recoverable file. +func rewriteLabels(path, backup string, rows []core.LabelRow) error { + orig, err := os.ReadFile(path) + if err != nil { + return fmt.Errorf("reading %s for backup: %w", path, err) + } + if backup != "" { + if err := os.WriteFile(backup, orig, 0o644); err != nil { + return fmt.Errorf("writing backup %s: %w", backup, err) + } + } + var buf bytes.Buffer + enc := json.NewEncoder(&buf) + for _, row := range rows { + if err := enc.Encode(row); err != nil { + return fmt.Errorf("encoding label row: %w", err) + } + } + if err := os.WriteFile(path, buf.Bytes(), 0o644); err != nil { + return fmt.Errorf("rewriting %s: %w", path, err) + } + return nil +} diff --git a/core/labels.go b/core/labels.go index bdff515..de14662 100644 --- a/core/labels.go +++ b/core/labels.go @@ -2,7 +2,10 @@ // // labels.jsonl is the canonical label store: one JSON LabelRow per line, // append-only. ReadLabels is strict (per-line errors with line numbers); -// AppendLabels is the only writer — rows are never rewritten in place. +// AppendLabels is the normal writer — rows are only rewritten in place by the +// two commands that deliberately do so, each backing the file up first: +// compact-labels (trimming per-feed negative skew) and export (relabeling a +// starred entry that was stored as a negative). package core import ( diff --git a/justfile b/justfile index f9fb1bd..df3d9d5 100644 --- a/justfile +++ b/justfile @@ -72,6 +72,11 @@ restart SERVER: ship SERVER: (deploy SERVER) (restart SERVER) @echo "model shipped; serve restarted — watch /api/status next run" +# Full model refresh: retrain + ship in one verb (no report gate — eyeball +# report.json first if unsure, or use `retrain` + `ship` separately) +refresh SERVER: retrain (ship SERVER) + @echo "refreshed: retrained locally + model shipped to {{SERVER}}" + # Trigger an out-of-schedule poll (serialized + cooldown-gated server-side) poll-now URL: curl -sS -X POST {{URL}}/api/poll -- cgit v1.2.3