diff --git a/cmd/mavend/actions_query.go b/cmd/mavend/actions_query.go
index c36682d..13b33be 100644
--- a/cmd/mavend/actions_query.go
+++ b/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
}
diff --git a/internal/router/feeds.go b/internal/router/feeds.go
index 91a6dac..f88b16e 100644
--- a/internal/router/feeds.go
+++ b/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
diff --git a/internal/router/feeds_test.go b/internal/router/feeds_test.go
index 72ffdd1..4ae6c31 100644
--- a/internal/router/feeds_test.go
+++ b/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")
}
diff --git a/internal/rss/feed.go b/internal/rss/feed.go
index 71d5580..06edfa3 100644
--- a/internal/rss/feed.go
+++ b/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)
diff --git a/internal/rss/marks_test.go b/internal/rss/marks_test.go
new file mode 100644
index 0000000..6d25ace
--- /dev/null
+++ b/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(`u`)
+ for _, t := range titles {
+ fmt.Fprintf(&b, `%shttps://example.org/%s%s`, t, t, t)
+ }
+ b.WriteString(``)
+ 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(`d`)
+ for i := 0; i < n; i++ {
+ ts := base.Add(-time.Duration(i) * time.Hour)
+ fmt.Fprintf(&b, `item-%dhttps://example.org/%dg%d%s`,
+ i, i, i, ts.Format(time.RFC1123Z))
+ }
+ b.WriteString(``)
+ 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)
+ }
+}
diff --git a/internal/rss/poller.go b/internal/rss/poller.go
index 073f774..24e3049 100644
--- a/internal/rss/poller.go
+++ b/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:" 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 {