cmd/mavend/actions_query.go

@@ -241,15 +241,13 @@ func (h *reactiveHandler) queryFeeds(ctx context.Context, t *queryTurn) (string,
 		if !strings.HasPrefix(n.Source, rss.SourcePrefix) {
 			continue
 		}
-		if !router.CategoryMatches(n.Text, q.Category) {
+		if !router.CategoryMatches(rss.NoteCategory(n.Text), q.Category) {
 			continue
 		}
-		// The note carries title, summary and link; she reads the title.
-		title := n.Text
-		if i := strings.IndexByte(title, '\n'); i > 0 {
-			title = title[:i]
-		}
-		picked = append(picked, strings.TrimSpace(title))
+		// The note carries title, summary, category tag and link; she reads the
+		// title alone. The tag is for the match above, and piper reads brackets
+		// out loud.
+		picked = append(picked, rss.NoteHeadline(n.Text))
 		if len(picked) == feedReadOut {
 			break
 		}

internal/router/feeds.go

@@ -16,14 +16,25 @@ type FeedQuery struct {
 	Category string
 }
 
-// feedNouns — the words that make a question about the feeds themselves.
+// feedNouns — the words that name the feeds themselves. One of these is enough,
+// with an ask, to make the turn a feed question.
 var feedNouns = []string{
 	"лента", "ленте", "ленты", "лентах", "лентам",
 	"новости", "новостей", "новостях", "новостям",
-	"новое", "нового", "новенького",
 	"feed", "feeds", "news", "headlines",
 }
 
+// vagueNouns — the newness words that are NOT about the feeds by themselves.
+//
+// "что нового?" is the most common opener in the language and it is a greeting,
+// not a request for headlines. It used to match here, so the shipping daemon —
+// which has no feeds block — answered "я пока не читаю ленты, они не настроены",
+// a configuration status in reply to hello. With feeds on it answered "в лентах
+// пока ничего нового", which is no better. A vague noun claims the turn only
+// when the utterance narrows it: a named topic ("что нового по технологиям"), or
+// a feed noun somewhere in it ("что нового в лентах").
+var vagueNouns = []string{"новое", "нового", "новенького", "new"}
+
 // newnessMarkers — the "что нового" half. "нового" alone is in feedNouns
 // because it carries the question on its own ("что нового?"); a bare "лента"
 // needs the ask, which is what askMarkers below is for.
@@ -33,13 +44,15 @@ var askMarkers = []string{
 }
 
 // ParseFeedQuery reports whether an utterance asks what is new in the feeds, and
-// which topic if it names one after "по"/"о"/"про"/"about".
+// which topic if it names one after "по"/"об"/"про"/"about".
 //
-// Both a feed noun and an ask are required. "у меня новая лента в инстаграме" is
-// a statement and must not be read as a request to recite headlines.
+// A feed noun and an ask are required. "у меня новая лента в инстаграме" is a
+// statement and must not be read as a request to recite headlines. A vague
+// newness word counts as the noun only when a topic is named — see vagueNouns
+// for why the bare "что нового?" must fall through.
 func ParseFeedQuery(text string) (FeedQuery, bool) {
 	toks := planTokens(text)
-	noun, ask := false, false
+	noun, vague, ask := false, false, false
 	for _, t := range toks {
 		for _, n := range feedNouns {
 			if t == n {
@@ -47,6 +60,12 @@ func ParseFeedQuery(text string) (FeedQuery, bool) {
 				break
 			}
 		}
+		for _, n := range vagueNouns {
+			if t == n {
+				vague = true
+				break
+			}
+		}
 		for _, a := range askMarkers {
 			if t == a {
 				ask = true
@@ -54,24 +73,34 @@ func ParseFeedQuery(text string) (FeedQuery, bool) {
 			}
 		}
 	}
-	if !noun || !ask {
+	if !ask {
 		return FeedQuery{}, false
 	}
-	return FeedQuery{Category: feedCategory(toks)}, true
+	cat := feedCategory(toks)
+	if !noun && !(vague && cat != "") {
+		return FeedQuery{}, false
+	}
+	return FeedQuery{Category: cat}, true
 }
 
 // categoryPreps — the prepositions a topic follows. Russian marks the topic with
 // a preposition ("по технологиям", "про политику"), so the word after one is the
 // category; there is no stemming here, and the match against the configured
 // category is a prefix comparison for exactly that reason.
-var categoryPreps = map[string]bool{"по": true, "о": true, "об": true, "про": true, "about": true, "on": true}
+//
+// "о" is not in the list. It is one rune and it turns up as filler, a typo and
+// half of "о'кей", so any utterance carrying a stray "о" produced a category of
+// whatever word came next and she answered "по этой теме в лентах пока ничего"
+// to a question that named no theme. "об" and "про" carry the same meaning and
+// cannot be mistaken for anything else.
+var categoryPreps = map[string]bool{"по": true, "об": true, "про": true, "about": true, "on": true}
 
 func feedCategory(toks []string) string {
 	for i, t := range toks {
 		if categoryPreps[t] && i+1 < len(toks) {
 			next := toks[i+1]
 			// "по новостям" names no topic, it repeats the noun.
-			for _, n := range feedNouns {
+			for _, n := range append(append([]string{}, feedNouns...), vagueNouns...) {
 				if next == n {
 					return ""
 				}
@@ -82,12 +111,16 @@ func feedCategory(toks []string) string {
 	return ""
 }
 
-// CategoryMatches reports whether a note's text plausibly belongs to the
-// category he named. Russian inflects the topic ("технологиям" vs the configured
+// CategoryMatches reports whether a feed note's own category tag is the one he
+// named. Russian inflects the topic ("технологиям" vs the configured
 // "технологии"), and there is no stemmer in this repo, so the comparison is on a
 // common prefix — long enough that "полит" and "погод" stay apart, short enough
 // to survive a case ending.
-func CategoryMatches(text, category string) bool {
+//
+// tag is the note's stored category (rss.NoteCategory), NOT the whole note. It
+// used to be the whole note, which meant "что нового про погоду" matched any
+// tech headline whose link happened to contain "pogod".
+func CategoryMatches(tag, category string) bool {
 	if category == "" {
 		return true
 	}
@@ -95,7 +128,7 @@ func CategoryMatches(text, category string) bool {
 	if stem == "" {
 		return false
 	}
-	return strings.Contains(strings.ToLower(text), stem)
+	return strings.Contains(strings.ToLower(tag), stem)
 }
 
 // categoryStem cuts a word down to the part inflection leaves alone. 5 runes is

internal/router/feeds_test.go

@@ -9,13 +9,19 @@ func TestParseFeedQuery(t *testing.T) {
 		category string
 	}{
 		{"что нового в лентах?", true, ""},
-		{"что нового?", true, ""},
+		{"что нового по технологиям", true, "технологиям"},
 		{"какие новости?", true, ""},
 		{"что нового по технологиям?", true, "технологиям"},
 		{"расскажи новости про политику", true, "политику"},
 		{"что нового по новостям", true, ""},
 		{"what's new in the feeds?", true, ""},
 		{"any news about kubernetes", true, "kubernetes"},
+		// "что нового?" is a greeting. Claiming it made the shipping daemon
+		// answer hello with "я пока не читаю ленты — они не настроены".
+		{"что нового?", false, ""},
+		{"ну что нового", false, ""},
+		// A stray "о" is not a topic marker.
+		{"что нового в лентах, о боже", true, ""},
 		// Statements, not requests.
 		{"у меня новая лента в инстаграме", false, ""},
 		{"новости меня утомили", false, ""},
@@ -36,12 +42,16 @@ func TestParseFeedQuery(t *testing.T) {
 
 func TestCategoryMatches(t *testing.T) {
 	// The inflected form he says must match the form the config spells.
-	if !CategoryMatches("Новый релиз [технологии]", "технологиям") {
+	if !CategoryMatches("технологии", "технологиям") {
 		t.Error("inflected category did not match")
 	}
-	if CategoryMatches("Новый релиз [технологии]", "политику") {
+	if CategoryMatches("технологии", "политику") {
 		t.Error("unrelated category matched")
 	}
+	// The tag, not the note. The link in a tech headline is not a weather report.
+	if CategoryMatches("технологии", "погоду") {
+		t.Error("a tech note matched a weather question")
+	}
 	if !CategoryMatches("anything", "") {
 		t.Error("an empty category must match everything")
 	}

internal/rss/feed.go

@@ -72,9 +72,13 @@ type feedDoc struct {
 func Parse(r io.Reader) (Feed, error) {
 	var doc feedDoc
 	dec := xml.NewDecoder(r)
-	// Feeds in the wild declare windows-1251 and worse. We only ever read
-	// UTF-8; a charset we cannot decode is a feed we do not read, which is
-	// better than mojibake in his notes.
+	// Strict=false buys tolerance of the malformed markup feeds are full of:
+	// unclosed tags, stray entities. It has nothing to do with charsets.
+	//
+	// Charsets are handled by not handling them: CharsetReader stays nil, so a
+	// feed declaring windows-1251 fails to parse rather than being read as
+	// UTF-8. That is the behaviour we want — a charset we cannot decode is a
+	// feed we do not read, which beats mojibake in his notes.
 	dec.Strict = false
 	if err := dec.Decode(&doc); err != nil {
 		return Feed{}, fmt.Errorf("rss: bad xml: %w", err)

internal/rss/marks_test.go

@@ -0,0 +1,114 @@
+package rss
+
+import (
+	"context"
+	"fmt"
+	"strings"
+	"testing"
+	"time"
+)
+
+// undatedFeed — a feed whose items carry no pubDate. Plenty of real ones do not.
+func undatedFeed(titles ...string) string {
+	var b strings.Builder
+	b.WriteString(`<?xml version="1.0"?><rss version="2.0"><channel><title>u</title>`)
+	for _, t := range titles {
+		fmt.Fprintf(&b, `<item><title>%s</title><link>https://example.org/%s</link><guid>%s</guid></item>`, t, t, t)
+	}
+	b.WriteString(`</channel></rss>`)
+	return b.String()
+}
+
+// datedFeed — newest first, one hour apart, the standard shape.
+func datedFeed(base time.Time, n int) string {
+	var b strings.Builder
+	b.WriteString(`<?xml version="1.0"?><rss version="2.0"><channel><title>d</title>`)
+	for i := 0; i < n; i++ {
+		ts := base.Add(-time.Duration(i) * time.Hour)
+		fmt.Fprintf(&b, `<item><title>item-%d</title><link>https://example.org/%d</link><guid>g%d</guid><pubDate>%s</pubDate></item>`,
+			i, i, i, ts.Format(time.RFC1123Z))
+	}
+	b.WriteString(`</channel></rss>`)
+	return b.String()
+}
+
+func TestPoll_UndatedFeedIsNotReNotedAfterARestart(t *testing.T) {
+	// The seen-IDs map dies with the process, so before the durable mark was
+	// consulted every boot re-noted the whole front page, stamped now, on top of
+	// the recent-notes window. A crash loop made that a flood.
+	feed := FeedConfig{Name: "u", URL: "https://example.org/rss"}
+	fetch := &fakeFetch{body: undatedFeed("a", "b", "c")}
+	marks := newMarks()
+
+	notes1 := &fakeNotes{}
+	p1 := NewPoller([]FeedConfig{feed}, fetch, notes1, marks, nil, nil, Config{})
+	p1.PollDue(context.Background(), now)
+	if len(notes1.notes) != 3 {
+		t.Fatalf("first boot wrote %d notes, want 3", len(notes1.notes))
+	}
+	if marks.m["u"].IsZero() {
+		t.Fatal("an undated feed left no mark, so the next process cannot tell it has been read")
+	}
+
+	// Restart. Same process-lifetime dedup map, gone.
+	notes2 := &fakeNotes{}
+	p2 := NewPoller([]FeedConfig{feed}, fetch, notes2, marks, nil, nil, Config{})
+	p2.PollDue(context.Background(), now.Add(time.Hour))
+	if len(notes2.notes) != 0 {
+		t.Fatalf("a restart re-noted %d undated items: %v", len(notes2.notes), notes2.notes)
+	}
+	// And the same process still notices something genuinely new.
+	fetch.body = undatedFeed("a", "b", "c", "d")
+	p2.PollDue(context.Background(), now.Add(2*time.Hour))
+	if len(notes2.notes) != 1 {
+		t.Fatalf("a new undated item after the resync wrote %d notes, want 1", len(notes2.notes))
+	}
+}
+
+func TestPoll_BurstLargerThanMaxItemsIsPacedNotDropped(t *testing.T) {
+	// max_items reads as pacing in the config doc. Marking the newest item
+	// written put everything below the cap behind the mark, permanently.
+	feed := FeedConfig{Name: "d", URL: "https://example.org/rss"}
+	fetch := &fakeFetch{body: datedFeed(now.Add(-time.Minute), 12)}
+	notes := &fakeNotes{}
+	marks := newMarks()
+	p := NewPoller([]FeedConfig{feed}, fetch, notes, marks, nil, nil, Config{MaxItems: 5, MaxAge: 48 * time.Hour})
+
+	at := now
+	for i := 0; i < 3; i++ {
+		if _, err := p.PollFeed(context.Background(), feed, at); err != nil {
+			t.Fatal(err)
+		}
+		at = at.Add(time.Hour)
+	}
+	seen := map[string]bool{}
+	for _, n := range notes.notes {
+		title := strings.SplitN(n.text, "\n", 2)[0]
+		if seen[title] {
+			t.Errorf("item %q was noted twice", title)
+		}
+		seen[title] = true
+	}
+	if len(seen) != 12 {
+		t.Errorf("after three polls of a 12-item burst she has %d of them; the rest were dropped for good", len(seen))
+	}
+}
+
+func TestNoteHeadlineAndCategory(t *testing.T) {
+	text := NoteText(FeedConfig{Category: "технологии"}, Item{
+		Title: "Новая уязвимость", Summary: "Патч вышел", Link: "https://example.org/a",
+	})
+	if got := NoteHeadline(text); got != "Новая уязвимость" {
+		t.Errorf("NoteHeadline = %q; she reads the brackets out loud", got)
+	}
+	if got := NoteCategory(text); got != "технологии" {
+		t.Errorf("NoteCategory = %q, want технологии", got)
+	}
+	// A feed's own leading tag stays part of the title.
+	if got := NoteHeadline("[перевод] Что-то"); got != "[перевод] Что-то" {
+		t.Errorf("NoteHeadline stripped the feed's own tag: %q", got)
+	}
+	if got := NoteCategory("Без категории"); got != "" {
+		t.Errorf("NoteCategory on an untagged note = %q, want empty", got)
+	}
+}

internal/rss/poller.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"log"
+	"sort"
 	"strings"
 	"time"
 )
@@ -88,6 +89,7 @@ type Poller struct {
 	cfg     Config
 	nextDue map[string]time.Time
 	seen    map[string]map[string]bool // feed → item ID, for items with no date
+	polled  map[string]bool            // feed → polled at least once in THIS process
 }
 
 // NewPoller wires a poller. Returns nil when there is nothing to poll — a
@@ -96,7 +98,9 @@ func NewPoller(feeds []FeedConfig, fetch Fetcher, notes Notes, marks Marks, embe
 	var valid []FeedConfig
 	for _, f := range feeds {
 		if strings.TrimSpace(f.Name) == "" || strings.TrimSpace(f.URL) == "" {
-			log.Printf("rss: skipping a feed with no name or no url")
+			// Name the offender. A silent skip in a list of six feeds is a
+			// config typo nobody finds.
+			log.Printf("rss: skipping feed %q (%q): a feed needs both a name and a url", f.Name, f.URL)
 			continue
 		}
 		valid = append(valid, f)
@@ -118,6 +122,7 @@ func NewPoller(feeds []FeedConfig, fetch Fetcher, notes Notes, marks Marks, embe
 		embed: embed, ranker: ranker, cfg: cfg,
 		nextDue: map[string]time.Time{},
 		seen:    map[string]map[string]bool{},
+		polled:  map[string]bool{},
 	}
 }
 
@@ -164,14 +169,31 @@ func (p *Poller) PollFeed(ctx context.Context, f FeedConfig, now time.Time) (int
 		return 0, err
 	}
 
-	mark := p.mark(ctx, f.Name, now)
-	newest := mark
-	written := 0
+	mark, durable := p.mark(ctx, f.Name, now)
+	// resync — the first poll of this feed since the process started, on a feed
+	// we have read before. Undated items are deduped by an in-memory ID set that
+	// dies with the process, so on this poll they are all "unseen" again and
+	// would all be re-noted. See fresh.
+	resync := durable && !p.polled[f.Name]
+	p.polled[f.Name] = true
+
+	// Gather first, cap second, and write OLDEST first.
+	//
+	// The old loop walked the feed newest-first and stopped at MaxItems, then
+	// marked the newest item it had written. Feeds are newest-first, so with
+	// twenty new items and a cap of five it wrote the five newest and moved the
+	// mark past all twenty: items six through twenty were older than the mark on
+	// the next poll and were dropped for good. max_items reads as a pacing knob
+	// in the config doc, and that made it a silent loss. Writing the oldest five
+	// and marking the newest of THOSE is pacing: the rest arrive over the polls
+	// that follow, in order, each one exactly once.
+	var cands []Item
+	sawUndated := false
 	for _, it := range feed.Items {
-		if written >= p.cfg.MaxItems {
-			break
+		if it.Published.IsZero() {
+			sawUndated = true
 		}
-		if !p.fresh(f, it, mark, now) {
+		if !p.fresh(f, it, mark, now, resync) {
 			continue
 		}
 		if !Matches(f, it) {
@@ -185,6 +207,18 @@ func (p *Poller) PollFeed(ctx context.Context, f FeedConfig, now time.Time) (int
 				continue
 			}
 		}
+		cands = append(cands, it)
+	}
+	sort.SliceStable(cands, func(i, j int) bool {
+		return itemTime(cands[i], now).Before(itemTime(cands[j], now))
+	})
+	if len(cands) > p.cfg.MaxItems {
+		cands = cands[:p.cfg.MaxItems]
+	}
+
+	newest := time.Time{}
+	written := 0
+	for _, it := range cands {
 		if err := p.write(ctx, f, it, now); err != nil {
 			return written, err
 		}
@@ -193,33 +227,71 @@ func (p *Poller) PollFeed(ctx context.Context, f FeedConfig, now time.Time) (int
 			newest = it.Published
 		}
 	}
-	if p.marks != nil && newest.After(mark) {
-		if err := p.marks.SetMark(ctx, f.Name, newest); err != nil {
-			log.Printf("rss: feed %s: save mark: %v", f.Name, err)
-		}
-	}
+	p.advance(ctx, f.Name, mark, newest, sawUndated, now)
 	return written, nil
 }
 
-// mark — how far this feed was read. A feed with no mark starts MaxAge ago, so
-// a first poll takes today's headlines instead of the whole archive.
-func (p *Poller) mark(ctx context.Context, feed string, now time.Time) time.Time {
+// itemTime — an item's own date, or now when the feed did not give one. Undated
+// items sort last, which is the only defensible guess: they were seen now.
+func itemTime(it Item, now time.Time) time.Time {
+	if it.Published.IsZero() {
+		return now
+	}
+	return it.Published
+}
+
+// advance moves the durable mark to the newest item actually WRITTEN. Because
+// the cap is applied to the oldest candidates (see PollFeed), that is never
+// ahead of an item still waiting to be read.
+//
+// A feed whose items carry no dates gets the mark set to now instead. Nothing
+// else would ever set it, and the mark's existence is what tells the next
+// process that this feed has been read before.
+func (p *Poller) advance(ctx context.Context, feed string, mark, newest time.Time, sawUndated bool, now time.Time) {
+	if p.marks == nil {
+		return
+	}
+	at := newest
+	if at.IsZero() && sawUndated {
+		at = now
+	}
+	if at.IsZero() || !at.After(mark) {
+		return
+	}
+	if err := p.marks.SetMark(ctx, feed, at); err != nil {
+		log.Printf("rss: feed %s: save mark: %v", feed, err)
+	}
+}
+
+// mark — how far this feed was read, and whether that came from the durable
+// store. A feed with no mark starts MaxAge ago, so a first poll takes today's
+// headlines instead of the whole archive; durable is false in that case, and it
+// is what tells PollFeed the difference between "never read" and "read by an
+// earlier process".
+func (p *Poller) mark(ctx context.Context, feed string, now time.Time) (time.Time, bool) {
 	cold := now.Add(-p.cfg.MaxAge)
 	if p.marks == nil {
-		return cold
+		return cold, false
 	}
 	at, err := p.marks.LastMark(ctx, feed)
 	if err != nil || at.IsZero() {
-		return cold
+		return cold, false
 	}
-	return at
+	return at, true
 }
 
 // fresh — two dedup rules, because feeds are inconsistent about dates. A dated
 // item must be newer than the mark; an undated one is kept once per process by
-// ID. Both are needed: dates alone re-import undated feeds forever, IDs alone
-// lose their memory on restart.
-func (p *Poller) fresh(f FeedConfig, it Item, mark, now time.Time) bool {
+// ID.
+//
+// The ID set does not survive a restart, and on its own that re-notes an undated
+// feed's whole front page on every boot — five notes, then five more, all stamped
+// `now`, sitting at the top of the recent-notes window and crowding out the notes
+// he actually made. A crash loop turns it into a flood. So on the first poll after
+// a restart of a feed we have read before (resync), undated items are recorded as
+// seen and NOT written. The cost is the undated items that appeared while the
+// daemon was down. That is a bounded loss, and the alternative is an unbounded one.
+func (p *Poller) fresh(f FeedConfig, it Item, mark, now time.Time, resync bool) bool {
 	if !it.Published.IsZero() {
 		if !it.Published.After(mark) {
 			return false
@@ -239,7 +311,7 @@ func (p *Poller) fresh(f FeedConfig, it Item, mark, now time.Time) bool {
 		return false
 	}
 	p.seen[f.Name][id] = true
-	return true
+	return !resync
 }
 
 // write stores one item as a note. Source "rss:<feed>" is what the answer path
@@ -291,6 +363,47 @@ func NoteText(f FeedConfig, it Item) string {
 	return b.String()
 }
 
+// NoteHeadline is the part of a feed note she reads out: the first line with the
+// category tag taken off. The tag is bookkeeping for the answer path, and piper
+// says brackets out loud — "Заголовок [технологии]" is what he heard before.
+func NoteHeadline(text string) string {
+	line := text
+	if i := strings.IndexByte(line, '\n'); i >= 0 {
+		line = line[:i]
+	}
+	line = strings.TrimSpace(line)
+	if head, _, ok := splitTag(line); ok {
+		return head
+	}
+	return line
+}
+
+// NoteCategory is the category tag a feed note carries, empty when it has none.
+// Matching a topic against THIS rather than against the whole note is what keeps
+// "что нового про погоду" from matching a tech headline whose link happens to
+// contain "pogod".
+func NoteCategory(text string) string {
+	line := text
+	if i := strings.IndexByte(line, '\n'); i >= 0 {
+		line = line[:i]
+	}
+	_, tag, _ := splitTag(strings.TrimSpace(line))
+	return tag
+}
+
+// splitTag pulls a trailing "[...]" off a headline. Only a trailing one: a title
+// that opens with "[перевод]" is the feed's own word, not ours.
+func splitTag(line string) (head, tag string, ok bool) {
+	if !strings.HasSuffix(line, "]") {
+		return line, "", false
+	}
+	i := strings.LastIndexByte(line, '[')
+	if i < 0 {
+		return line, "", false
+	}
+	return strings.TrimSpace(line[:i]), strings.TrimSpace(line[i+1 : len(line)-1]), true
+}
+
 // trimRunes cuts on a rune boundary — a note is Russian as often as English and
 // half a cyrillic letter is a broken note.
 func trimRunes(s string, max int) string {