Skip to content

fix(deps): follow-up fixes from PR #135 review + quality-gate cycle #136

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wgordon17 merged 2 commits into from
Aug 19, 2026
Merged

fix(deps): follow-up fixes from PR #135 review + quality-gate cycle #136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9a15d64
fix(deps): reorders title-parsing patterns, closes label-fallback gaps
wgordon17 Aug 18, 2026
0bcd571
refactor(deps): extracts shared body-fetch helper, adds failure cooldown
wgordon17 Aug 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 40 additions & 6 deletions src/app/components/dashboard/DashboardPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,13 @@ const [depMeta, setDepMeta] = createSignal<ReadonlyMap<number, VersionInfo>>(loa
let _fetchingDashboardBodies = false;
let _fetchingDepBodies = false;

// Skip retrying a dep-PR body fetch that errored/timed out on its last attempt until
// this cooldown elapses — prevents burning a full GRAPHQL_BODY_FETCH_TIMEOUT_MS timeout,
// an undeduped Sentry event, and a repeated user notification every poll cycle for as
// long as the underlying failure (e.g. a secondary-rate-limit stall) persists.
export const DEP_BODY_FAILURE_COOLDOWN_MS = 5 * 60 * 1000;
const _depBodyFailureCooldown = new Map<number, number>();

// Clear dashboard data and stop polling on logout to prevent cross-user data leakage
onAuthCleared(() => {
resetDashboardData();
Expand All @@ -193,6 +200,7 @@ onAuthCleared(() => {
localStorage.removeItem?.(DEP_META_STORAGE_KEY);
_fetchingDashboardBodies = false;
_fetchingDepBodies = false;
_depBodyFailureCooldown.clear();
resetAbandonedPatternCache();
const coord = _coordinator();
if (coord) {
Expand Down Expand Up @@ -1194,7 +1202,7 @@ export default function DashboardPage() {
if (relevant.length === 0) return;

const nodeIds = relevant.map((di) => di.nodeId);
const bodyMap = await fetchDashboardIssueBodies(octokit, nodeIds);
const { bodies: bodyMap } = await fetchDashboardIssueBodies(octokit, nodeIds);

const newAbandonedMap = new Map<string, AbandonedDependency[]>();
const newUrlMap = new Map<string, string>();
Expand Down Expand Up @@ -1223,7 +1231,7 @@ export default function DashboardPage() {
createEffect(() => {
if (!config.dependencies.enabled) return;
if (_fetchingDepBodies) {
console.debug("[dashboard] depBodies effect: skipped — fetch already in flight (this run's tracked deps are now narrowed to config.dependencies.enabled only)");
console.debug("[dashboard] depBodies effect: skipped — fetch already in flight");
return;
}
const octokit = getClient();
Expand All @@ -1232,7 +1240,23 @@ export default function DashboardPage() {
const meta = depMeta();
const depPrs = dependencyPullRequests();
const visibleDepPrs = visibleDependencyPullRequests();
const toFetch = visibleDepPrs.filter((pr) => !meta.has(pr.id) && needsBodyFallback(pr));
const now = Date.now();

// Prune cooldown entries for PRs no longer in the dependency set on every
// effect run, regardless of whether anything needs fetching this cycle —
// otherwise a PR that fails, leaves the set, and reopens before some OTHER
// PR happens to trigger a fetch would incorrectly stay excluded on a stale
// cooldown entry for up to DEP_BODY_FAILURE_COOLDOWN_MS.
const depPrIds = new Set(depPrs.map((pr) => pr.id));
for (const k of [..._depBodyFailureCooldown.keys()]) {
if (!depPrIds.has(k)) _depBodyFailureCooldown.delete(k);
}

const toFetch = visibleDepPrs.filter((pr) => {
if (meta.has(pr.id) || !needsBodyFallback(pr)) return false;
const failedAt = _depBodyFailureCooldown.get(pr.id);
return !failedAt || now - failedAt >= DEP_BODY_FAILURE_COOLDOWN_MS;
});
if (toFetch.length === 0) {
console.debug("[dashboard] depBodies effect: nothing to fetch", { metaSize: meta.size, visibleDepPrCount: visibleDepPrs.length });
return;
Expand All @@ -1244,8 +1268,19 @@ export default function DashboardPage() {
void (async () => {
try {
const nodeIds = toFetch.map((pr) => pr.nodeId!);
const bodyMap = await fetchDepPRBodies(octokit, nodeIds);
console.debug(`[dashboard] depBodies effect: fetch resolved after ${Date.now() - effectStart}ms`, { requested: toFetch.length, returned: bodyMap.size });
const { bodies: bodyMap, failedIds } = await fetchDepPRBodies(octokit, nodeIds);
console.debug(`[dashboard] depBodies effect: fetch resolved after ${Date.now() - effectStart}ms`, { requested: toFetch.length, returned: bodyMap.size, failed: failedIds.size });

// Record/clear per-PR cooldown so a batch that errored or timed out isn't
// retried again until DEP_BODY_FAILURE_COOLDOWN_MS has elapsed.
for (const pr of toFetch) {
if (pr.nodeId && failedIds.has(pr.nodeId)) {
_depBodyFailureCooldown.set(pr.id, now);
} else {
_depBodyFailureCooldown.delete(pr.id);
}
}

if (bodyMap.size === 0) return;

const merged = new Map(meta);
Expand All @@ -1254,7 +1289,6 @@ export default function DashboardPage() {
if (parsed) merged.set(id, parsed);
}
// Prune entries for PRs no longer in the dependency set
const depPrIds = new Set(depPrs.map((pr) => pr.id));
for (const k of [...merged.keys()]) {
if (!depPrIds.has(k)) merged.delete(k);
}
Expand Down
28 changes: 19 additions & 9 deletions src/app/lib/dependency-detection.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,17 @@ export function extractVersionInfo(title: string): VersionInfo | null {
return { packageName: actionMatch[1]!, to: actionMatch[2]! };
}

// Generic "from A to B" anywhere. Checked before crateMatch/dockerMatch below so that a title
// combining the literal "crate"/"docker tag" keyword with "from...to" phrasing (e.g. "Update
// crate pyo3 from 0.29.0 to 0.29.1") is classified via the correct semver-diff path instead of
// having crateMatch/dockerMatch's lazy capture group absorb the "from" clause into the package
// name (their (.+?) group expands until the first " to " match, which would land after the
// "from" version, not after the package name).
const genericMatch = /\bfrom\s+([\w.\-+]+)\s+to\s+([\w.\-+]+)/i.exec(body);
if (genericMatch) {
return { from: genericMatch[1]!, to: genericMatch[2]!, updateType: semverUpdateType(genericMatch[1]!, genericMatch[2]!) ?? undefined };
}

// "Update (rust) crate X to vY"
const crateMatch = /^Update\s+(?:rust\s+)?crate\s+(.+?)\s+to\s+(v?[\w.\-+]+)/i.exec(body);
if (crateMatch && /^v?\d/.test(crateMatch[2]!)) {
Expand All @@ -157,14 +168,13 @@ export function extractVersionInfo(title: string): VersionInfo | null {
return { packageName: dockerMatch[1]!, to: dockerMatch[2]! };
}

// Generic "from A to B" anywhere
const genericMatch = /\bfrom\s+([\w.\-+]+)\s+to\s+([\w.\-+]+)/i.exec(body);
if (genericMatch) {
return { from: genericMatch[1]!, to: genericMatch[2]!, updateType: semverUpdateType(genericMatch[1]!, genericMatch[2]!) ?? undefined };
}

// Generic "Update X to vY" (last resort, single-target version only)
const singleTargetMatch = /^Update\s+(.+?)\s+to\s+(v?[\w.\-+]+)$/i.exec(body);
// Generic "Update X to vY" (last resort, single-target version only). The package-name group
// requires a single whitespace-free token (rather than any characters) so a human-authored,
// multi-word title like "Update the docs to v2" — reachable via isDependencyPr()'s label-only
// admission path, which needs no bot-like title at all — isn't misclassified as a dependency
// bump. Real bot-generated titles for this fallback (e.g. "update renovate to v44", "update
// node to v24.18.1") are single-token package names and are unaffected.
const singleTargetMatch = /^Update\s+(\S+)\s+to\s+(v?[\w.\-+]+)$/i.exec(body);
if (singleTargetMatch && /^v?\d/.test(singleTargetMatch[2]!)) {
return { packageName: singleTargetMatch[1]!, to: singleTargetMatch[2]! };
}
Expand Down Expand Up @@ -249,7 +259,7 @@ export function needsBodyFallback(pr: PullRequest): boolean {
if (/lock\s*file\s+maintenance/i.test(pr.title)) return false;
for (const l of pr.labels) {
const name = l.name.toLowerCase();
if (name === "major" || name === "minor" || name === "patch") return false;
if (name === "major" || name === "minor" || name === "patch" || name === "digest" || name === "pin" || name === "maintenance") return false;
}
return true;
}
Expand Down
182 changes: 97 additions & 85 deletions src/app/services/api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1162,9 +1162,9 @@ export async function fetchPREnrichment(
return { enrichments, errors };
}

// Shared timeout guard for body-fetch GraphQL calls. Prevents a hung request
// (e.g. octokit's secondary-rate-limit retry logic stalling indefinitely)
// from wedging the caller's fetch-in-progress gate.
// ── Body-fetch timeout guard ────────────────────────────────────────────────
// Prevents a hung request (e.g. octokit's secondary-rate-limit retry logic
// stalling indefinitely) from wedging the caller's fetch-in-progress gate.

export const GRAPHQL_BODY_FETCH_TIMEOUT_MS = 20_000;

Expand All @@ -1185,70 +1185,113 @@ export function raceWithTimeout<T>(promise: Promise<T>, ms: number, controller:
});
}

// ── Dashboard issue body fetch ────────────────────────────────────────────────

const DASHBOARD_ISSUE_BODIES_QUERY = `
query($ids: [ID!]!) {
nodes(ids: $ids) {
... on Issue { id body }
}
rateLimit { cost limit remaining resetAt }
}
`;

interface DashboardIssueBodiesResponse {
nodes: Array<{ id: string; body: string | null } | null>;
interface NodeBodiesResponse<TNode> {
nodes: Array<TNode | null>;
rateLimit?: GraphQLRateLimit;
}

/** Fetches issue bodies for Dashboard issues by node ID (single nodes() batch query). */
export async function fetchDashboardIssueBodies(
export interface NodeBodiesFetchResult<K, V> {
bodies: Map<K, V>;
/** Requested node IDs belonging to a batch that errored or timed out. */
failedIds: Set<string>;
}

/**
* Shared batched-fetch helper for GraphQL `nodes()` queries that pull a single
* field (e.g. issue/PR body text) for a set of node IDs. Chunks ids into
* NODES_BATCH_SIZE batches, races each batch via raceWithTimeout, and reports
* failures via console.warn + Sentry + a single deduped pushNotification.
*
* `mapNode` lets each caller decide how to fold a raw node into the result map
* (e.g. whether to keep or skip a null body) without duplicating the batching,
* timeout, rate-limit, and error-reporting plumbing. Batches that error or time
* out contribute their requested ids to `failedIds` so callers can avoid
* retrying a persistently-failing id on every poll cycle.
*/
async function fetchNodeBodiesBatched<TNode, K, V>(
octokit: GitHubOctokit,
issueNodeIds: string[]
): Promise<Map<string, string | null>> {
const result = new Map<string, string | null>();
if (issueNodeIds.length === 0) return result;
nodeIds: string[],
query: string,
source: string,
notificationMessage: string,
mapNode: (node: TNode, bodies: Map<K, V>) => void,
): Promise<NodeBodiesFetchResult<K, V>> {
const bodies = new Map<K, V>();
const failedIds = new Set<string>();
if (nodeIds.length === 0) return { bodies, failedIds };

const batches = chunkArray(issueNodeIds, NODES_BATCH_SIZE);
let hadFailure = false;
const batches = chunkArray(nodeIds, NODES_BATCH_SIZE);
await Promise.allSettled(batches.map(async (batch) => {
const batchStart = Date.now();
console.debug(`[api] dashboardBodies batch started (${batch.length} ids) at ${batchStart}`);
console.debug(`[api] ${source} batch started (${batch.length} ids) at ${batchStart}`);
const controller = new AbortController();
try {
const response = await raceWithTimeout(
octokit.graphql<DashboardIssueBodiesResponse>(
DASHBOARD_ISSUE_BODIES_QUERY,
{ ids: batch, request: { apiSource: "dashboardBodies", signal: controller.signal } }
octokit.graphql<NodeBodiesResponse<TNode>>(
query,
{ ids: batch, request: { apiSource: source, signal: controller.signal } }
),
GRAPHQL_BODY_FETCH_TIMEOUT_MS,
controller,
);
if (response.rateLimit) updateGraphqlRateLimit(response.rateLimit);
for (const node of response.nodes) {
if (!node || !node.id) continue;
result.set(node.id, node.body);
if (!node) continue;
mapNode(node, bodies);
}
} catch (err) {
hadFailure = true;
console.warn("[api] dashboardBodies batch failed or timed out:", err);
Sentry.captureException(err, { tags: { source: "dashboardBodies" } });
for (const id of batch) failedIds.add(id);
console.warn(`[api] ${source} batch failed or timed out:`, err);
Sentry.captureException(err, { tags: { source } });
const partialErr =
err && typeof err === "object" && "data" in err && err.data && typeof err.data === "object"
? (err.data as Partial<DashboardIssueBodiesResponse>)
? (err.data as Partial<NodeBodiesResponse<TNode>>)
: null;
if (partialErr?.rateLimit) updateGraphqlRateLimit(partialErr.rateLimit);
// Partial failures return null bodies — callers handle missing entries gracefully
} finally {
console.debug(`[api] dashboardBodies batch settled after ${Date.now() - batchStart}ms`);
console.debug(`[api] ${source} batch settled after ${Date.now() - batchStart}ms`);
}
}));

if (hadFailure && getClient() === octokit) {
pushNotification("dashboardBodies", "Some dependency dashboard data could not be loaded", "warning");
if (failedIds.size > 0 && getClient() === octokit) {
pushNotification(source, notificationMessage, "warning");
}

return result;
return { bodies, failedIds };
}

// ── Dashboard issue body fetch ────────────────────────────────────────────────

const DASHBOARD_ISSUE_BODIES_QUERY = `
query($ids: [ID!]!) {
nodes(ids: $ids) {
... on Issue { id body }
}
rateLimit { cost limit remaining resetAt }
}
`;

interface DashboardIssueBodyNode {
id: string;
body: string | null;
}

/** Fetches issue bodies for Dashboard issues by node ID (single nodes() batch query). */
export async function fetchDashboardIssueBodies(
octokit: GitHubOctokit,
issueNodeIds: string[]
): Promise<NodeBodiesFetchResult<string, string | null>> {
return fetchNodeBodiesBatched<DashboardIssueBodyNode, string, string | null>(
octokit,
issueNodeIds,
DASHBOARD_ISSUE_BODIES_QUERY,
"dashboardBodies",
"Some dependency dashboard data could not be loaded",
(node, bodies) => {
if (!node.id) return;
bodies.set(node.id, node.body);
},
);
}

// ── Dependency PR body fetch ─────────────────────────────────────────────────
Expand All @@ -1262,57 +1305,26 @@ const DEP_PR_BODIES_QUERY = `
}
`;

interface DepPRBodiesResponse {
nodes: Array<{ databaseId: number; body: string | null } | null>;
rateLimit?: GraphQLRateLimit;
interface DepPRBodyNode {
databaseId: number;
body: string | null;
}

export async function fetchDepPRBodies(
octokit: GitHubOctokit,
prNodeIds: string[]
): Promise<Map<number, string>> {
const result = new Map<number, string>();
if (prNodeIds.length === 0) return result;

const batches = chunkArray(prNodeIds, NODES_BATCH_SIZE);
let hadFailure = false;
await Promise.allSettled(batches.map(async (batch) => {
const batchStart = Date.now();
console.debug(`[api] depPRBodies batch started (${batch.length} ids) at ${batchStart}`);
const controller = new AbortController();
try {
const response = await raceWithTimeout(
octokit.graphql<DepPRBodiesResponse>(
DEP_PR_BODIES_QUERY,
{ ids: batch, request: { apiSource: "depPRBodies", signal: controller.signal } }
),
GRAPHQL_BODY_FETCH_TIMEOUT_MS,
controller,
);
if (response.rateLimit) updateGraphqlRateLimit(response.rateLimit);
for (const node of response.nodes) {
if (!node || node.databaseId == null || !node.body) continue;
result.set(node.databaseId, node.body);
}
} catch (err) {
hadFailure = true;
console.warn("[api] depPRBodies batch failed or timed out:", err);
Sentry.captureException(err, { tags: { source: "depPRBodies" } });
const partialErr =
err && typeof err === "object" && "data" in err && err.data && typeof err.data === "object"
? (err.data as Partial<DepPRBodiesResponse>)
: null;
if (partialErr?.rateLimit) updateGraphqlRateLimit(partialErr.rateLimit);
} finally {
console.debug(`[api] depPRBodies batch settled after ${Date.now() - batchStart}ms`);
}
}));

if (hadFailure && getClient() === octokit) {
pushNotification("depPRBodies", "Some dependency PR types could not be determined — badges may be missing", "warning");
}

return result;
): Promise<NodeBodiesFetchResult<number, string>> {
return fetchNodeBodiesBatched<DepPRBodyNode, number, string>(
octokit,
prNodeIds,
DEP_PR_BODIES_QUERY,
"depPRBodies",
"Some dependency PR types could not be determined — badges may be missing",
(node, bodies) => {
if (node.databaseId == null || !node.body) return;
bodies.set(node.databaseId, node.body);
},
);
}

/**
Expand Down
Loading