Skip to content

partial-with-a-gallop-hit is cached as terminal: 30-day TTL, never revalidated #155

Description

@lukaso-bot

resolve.ts classifies a soft-deadline partial that carries a firstRelease as a terminal answer. Both predicates test firstRelease before partial:

function hardTtlFor(r: LookupResult): number {
  if (r.firstRelease) return HARD_TTL_RELEASED;   // 30 days
  if (r.partial) return HARD_TTL_PARTIAL;         // 60s — never reached for this shape
  return HARD_TTL_PENDING;
}

function isFresh(entry: CacheEntry<LookupResult>): boolean {
  if (entry.value.firstRelease) return true;      // fresh forever
  if (entry.value.partial) return entry.ageSeconds < HARD_TTL_PARTIAL;
  return entry.ageSeconds < FRESH_WINDOW_PENDING;
}

The justification for the terminal exemption is that which release first contains a commit cannot change. That holds for a completed lookup. It does not hold for this shape: find-release.ts:293-305 returns a partial whose firstRelease is the galloping hit, and the bisect that would confirm no earlier release contains the commit is exactly what the deadline cut short:

the gallop-found tag is almost always the right answer; bisect just verifies "could there be an earlier one."

find-release.ts:724 says the same in the issue/PR path: "but mark partial so caller knows it's not necessarily the earliest."

So a gallop-only answer is written with a 30-day hard TTL and reported fresh forever — never revalidated. On a large repo that reliably blows the 24s soft deadline, the first lookup that happens to gallop can pin a possibly-not-earliest release for a month, on the public permalink and badge routes.

The result card does render the best-effort caveat, so the human-facing damage is bounded — a reader sees "partial". The caching is the problem: the answer is never re-derived, so it cannot self-correct once the repo is warm enough to complete a full traversal.

Not a regression. This is on main today and predates #144. #144 only adds a bound for the OG consumer, which cannot render a caveat — it rejects the shape on the way out, but does not fix the underlying classification, deliberately (widening #144 would have made it harder to review and merge).

Suggested fix

Test partial before firstRelease in both predicates, so a truncated traversal is trusted for its own 60 seconds regardless of what it found:

function hardTtlFor(r: LookupResult): number {
  if (r.partial) return HARD_TTL_PARTIAL;
  if (r.firstRelease) return HARD_TTL_RELEASED;
  return HARD_TTL_PENDING;
}

function isFresh(entry: CacheEntry<LookupResult>): boolean {
  if (entry.value.partial) return entry.ageSeconds < HARD_TTL_PARTIAL;
  if (entry.value.firstRelease) return true;
  return entry.ageSeconds < FRESH_WINDOW_PENDING;
}

Worth checking the cost first: a 60s TTL on the shape means a deadline-blowing repo re-runs a full traversal each minute on the shared token. A middle TTL for "partial but answered" (minutes, not 30 days) may be the better trade.

Guard

packages/web/test/resolve.test.ts — seed a { partial, firstRelease } entry aged past the chosen TTL and assert it is revalidated rather than served. Mutate by restoring the firstRelease-first ordering; the test must go red.

Found by the review of #144 (packages/web/src/resolve.ts:75-86).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions