Skip to content

Commit

Plan activity reads only the outcome's events, so a busy repository never crowds it out

The plan page read the repository's 200 newest events and kept the outcome's afterwards, so on a busy repository other work pushed the outcome's activity out of the window. The events service's list now filters in the query: `numbers` (events whose data names one of these issues or pull requests as `number`, or as the `issue` a comment, review or link is on), `since` (an RFC 3339 time) and `actor` (an account id). The loader asks twice, in parallel: the outcome's issues and pull requests since it was applied, and issues g1t's agent filed since then; merged, newest first, at most 40. The WHERE clause is built by a pure function with tests.

syntaqxcommitted Parent1315c25Browse files
4 files+147−410/4 viewed
+22−14
2323 } from "../../lib/session.server";
2424 import { useRefreshWhile } from "../../lib/refresh";
2525
26+/** The most of an outcome's activity the page shows. */
27+const ACTIVITY_LIMIT = 40;
2628
2729 export function meta({ params, ...args }: Route.MetaArgs) {
2830 return page(args, { title: `Plan · ${params.owner}/${params.repo} · g1t` });
4951 // What happened across the outcome's issues and pull requests.
5052 let activity: G1tEvent[] = [];
5153 if (found.status === "applied" && found.progress.length > 0) {
52− const numbers = new Set([
53− ...found.progress.map((item) => item.number),
54− ...found.progress.flatMap((item) => (item.pull != null ? [item.pull] : [])),
54+ const numbers = [
55+ ...new Set([
56+ ...found.progress.map((item) => item.number),
57+ ...found.progress.flatMap((item) => (item.pull != null ? [item.pull] : [])),
58+ ]),
59+ ];
60+ const since = found.finishedAt ?? found.createdAt;
61+ // The log is read for this outcome alone, so a busy repository's other
62+ // work never crowds it out: events on its issues and pull requests, and
63+ // issues an agent filed while working on it, which belong to it too.
64+ const [own, filed] = await Promise.all([
65+ events.list({ repoId: found.repoId, numbers, since, limit: ACTIVITY_LIMIT }).catch(() => []),
66+ events
67+ .list({ repoId: found.repoId, types: ["issue.opened"], actor: "usr_g1t_agent", since, limit: ACTIVITY_LIMIT })
68+ .catch(() => []),
5569 ]);
56− const since = found.finishedAt ?? found.createdAt;
57− const recent = await events.list({ repoId: found.repoId, limit: 200 }).catch(() => []);
58− activity = recent
59− .filter((event) => event.time >= since)
60− .filter((event) => {
61− const data = event.data as { number?: number; issue?: number };
62− // Issues an agent filed while working on this outcome belong to it too.
63− if (event.type === "issue.opened" && event.actor === "usr_g1t_agent") return true;
64− return (data.number != null && numbers.has(data.number)) || (data.issue != null && numbers.has(data.issue));
65− })
66− .slice(0, 40);
70+ const seen = new Set<string>();
71+ activity = [...own, ...filed]
72+ .filter((event) => (seen.has(event.id) ? false : (seen.add(event.id), true)))
73+ .sort((a, b) => (a.id < b.id ? 1 : a.id > b.id ? -1 : 0))
74+ .slice(0, ACTIVITY_LIMIT);
6775 // Events name accounts by id; show names.
6876 const named = await identity
6977 .usernames([...new Set(activity.flatMap((event) => (event.actor ? [event.actor] : [])))])
+10−0
559559 /// Only events older than this event id.
560560 #[serde(default)]
561561 pub before: Option<String>,
562+ /// Only events by this account id.
563+ #[serde(default)]
564+ pub actor: Option<String>,
565+ /// Only events about these issues or pull requests: their `number`, or
566+ /// the `issue` a comment, review or link is on. All when empty.
567+ #[serde(default)]
568+ pub numbers: Vec<u32>,
569+ /// Only events at or after this RFC 3339 time.
570+ #[serde(default)]
571+ pub since: Option<String>,
562572 #[serde(default)]
563573 pub limit: Option<u32>,
564574 }
+6−0
483483 types?: EventType[];
484484 /** Return events older than this event id. */
485485 before?: string;
486+ /** Only events by this account id. */
487+ actor?: string;
488+ /** Only events about these issues or pull requests (`number`, or the `issue` a comment or review is on). */
489+ numbers?: number[];
490+ /** Only events at or after this RFC 3339 time. */
491+ since?: string;
486492 limit?: number;
487493 };
488494
+109−27
117117 /// Newest first. Callers must have checked that the viewer may see the
118118 /// repository asked about.
119119 async fn list(&self, a: ListArgs) -> Result<Vec<Event>> {
120− let mut conditions = Vec::new();
121− let mut values: Vec<JsValue> = Vec::new();
122− if let Some(repo_id) = &a.repo_id {
123− conditions.push("repo_id = ?".to_owned());
124− values.push(repo_id.as_str().into());
125− }
126− if !a.types.is_empty() {
127− let marks = vec!["?"; a.types.len()].join(", ");
128− conditions.push(format!("type IN ({marks})"));
129− values.extend(a.types.iter().map(|kind| JsValue::from(kind.as_str())));
130− } else {
131− // What CI and integrations report on commits goes to webhooks,
132− // and is read from each commit's checks; a timeline asked for
133− // everything would be little else on a busy repository.
134− let marks = vec!["?"; REPORTING.len()].join(", ");
135− conditions.push(format!("type NOT IN ({marks})"));
136− values.extend(REPORTING.iter().map(|kind| JsValue::from(*kind)));
137− }
138− if let Some(before) = &a.before {
139− conditions.push("id < ?".to_owned());
140− values.push(before.as_str().into());
141− }
142− let filter = if conditions.is_empty() {
143− String::new()
144− } else {
145− format!("WHERE {}", conditions.join(" AND "))
146− };
120+ let (filter, binds) = list_filter(&a);
121+ let mut values: Vec<JsValue> = binds
122+ .into_iter()
123+ .map(|bind| match bind {
124+ Bind::Text(text) => JsValue::from(text),
125+ Bind::Number(number) => JsValue::from(number),
126+ })
127+ .collect();
147128 values.push(a.limit.unwrap_or(DEFAULT_PAGE).min(MAX_PAGE).into());
148129 let rows = self
149130 .db
319300 Err(error) => worker::console_error!("audit entries kept past their plan's days: {error}"),
320301 }
321302 }
303+
304+/// A value bound to a `?` in [`list_filter`]'s clause.
305+#[derive(Debug, PartialEq)]
306+enum Bind {
307+ Text(String),
308+ Number(f64),
309+}
310+
311+/// The `WHERE` clause `list` reads with, and what it binds, in order.
312+fn list_filter(a: &ListArgs) -> (String, Vec<Bind>) {
313+ let mut conditions = Vec::new();
314+ let mut values = Vec::new();
315+ if let Some(repo_id) = &a.repo_id {
316+ conditions.push("repo_id = ?".to_owned());
317+ values.push(Bind::Text(repo_id.clone()));
318+ }
319+ if !a.types.is_empty() {
320+ let marks = vec!["?"; a.types.len()].join(", ");
321+ conditions.push(format!("type IN ({marks})"));
322+ values.extend(a.types.iter().map(|kind| Bind::Text(kind.clone())));
323+ } else {
324+ // What CI and integrations report on commits goes to webhooks,
325+ // and is read from each commit's checks; a timeline asked for
326+ // everything would be little else on a busy repository.
327+ let marks = vec!["?"; REPORTING.len()].join(", ");
328+ conditions.push(format!("type NOT IN ({marks})"));
329+ values.extend(REPORTING.iter().map(|kind| Bind::Text((*kind).to_owned())));
330+ }
331+ if let Some(actor) = &a.actor {
332+ conditions.push("actor = ?".to_owned());
333+ values.push(Bind::Text(actor.clone()));
334+ }
335+ if !a.numbers.is_empty() {
336+ // An issue or pull request's own events name it as `number`; a
337+ // comment, review or link on it names it as `issue`.
338+ let marks = vec!["?"; a.numbers.len()].join(", ");
339+ conditions.push(format!(
340+ "(json_extract(data, '$.number') IN ({marks}) OR json_extract(data, '$.issue') IN ({marks}))"
341+ ));
342+ for _ in 0..2 {
343+ values.extend(a.numbers.iter().map(|number| Bind::Number(f64::from(*number))));
344+ }
345+ }
346+ if let Some(since) = &a.since {
347+ conditions.push("time >= ?".to_owned());
348+ values.push(Bind::Text(since.clone()));
349+ }
350+ if let Some(before) = &a.before {
351+ conditions.push("id < ?".to_owned());
352+ values.push(Bind::Text(before.clone()));
353+ }
354+ let filter = if conditions.is_empty() {
355+ String::new()
356+ } else {
357+ format!("WHERE {}", conditions.join(" AND "))
358+ };
359+ (filter, values)
360+}
361+
362+#[cfg(test)]
363+mod tests {
364+ use super::*;
365+
366+ #[test]
367+ fn a_plain_list_leaves_out_what_is_reported_on_commits() {
368+ let (filter, binds) = list_filter(&ListArgs { repo_id: Some("rep_1".into()), ..ListArgs::default() });
369+ assert!(filter.starts_with("WHERE repo_id = ? AND type NOT IN ("));
370+ assert_eq!(binds[0], Bind::Text("rep_1".into()));
371+ assert_eq!(binds.len(), 1 + REPORTING.len());
372+ }
373+
374+ #[test]
375+ fn numbers_match_an_item_or_what_is_said_on_it_since_a_time() {
376+ let (filter, binds) = list_filter(&ListArgs {
377+ repo_id: Some("rep_1".into()),
378+ types: vec!["issue.opened".into()],
379+ numbers: vec![4, 9],
380+ since: Some("2026-10-01T00:00:00Z".into()),
381+ actor: Some("usr_g1t_agent".into()),
382+ ..ListArgs::default()
383+ });
384+ assert_eq!(
385+ filter,
386+ "WHERE repo_id = ? AND type IN (?) AND actor = ? AND \
387+ (json_extract(data, '$.number') IN (?, ?) OR json_extract(data, '$.issue') IN (?, ?)) AND time >= ?"
388+ );
389+ assert_eq!(
390+ binds,
391+ vec![
392+ Bind::Text("rep_1".into()),
393+ Bind::Text("issue.opened".into()),
394+ Bind::Text("usr_g1t_agent".into()),
395+ Bind::Number(4.0),
396+ Bind::Number(9.0),
397+ Bind::Number(4.0),
398+ Bind::Number(9.0),
399+ Bind::Text("2026-10-01T00:00:00Z".into()),
400+ ]
401+ );
402+ }
403+}