diff --git a/apps/server/src/pullRequest/PullRequestService.test.ts b/apps/server/src/pullRequest/PullRequestService.test.ts index 6c6c8215aa315..b7f678c7f15a0 100644 --- a/apps/server/src/pullRequest/PullRequestService.test.ts +++ b/apps/server/src/pullRequest/PullRequestService.test.ts @@ -3717,6 +3717,69 @@ it.effect("reports a file pushed to since it was cleared as changed", () => }), ); +it.effect("clears a file against what the head has now, not what it had a moment ago", () => + Effect.gen(function* () { + const revisions = new Map([["src/a.ts", "blob-a"]]); + const service = yield* environmentViewedService(revisions, []); + + yield* service.setFilesViewed({ + ...GITLAB_REFERENCE, + files: [{ path: "src/a.ts", viewed: true }], + }); + revisions.set("src/a.ts", "blob-a-again"); + yield* service.setFilesViewed({ + ...GITLAB_REFERENCE, + files: [{ path: "src/a.ts", viewed: false }], + }); + yield* service.setFilesViewed({ + ...GITLAB_REFERENCE, + files: [{ path: "src/a.ts", viewed: true }], + }); + yield* service.invalidate({ reference: GITLAB_REFERENCE }); + + assert.deepStrictEqual((yield* service.filesViewed(GITLAB_REFERENCE)).files, [ + { path: "src/a.ts", state: "viewed" }, + ]); + }), +); + +it.effect("records shown revisions until the reader ticks the pushed version", () => + Effect.gen(function* () { + const asked: Array> = []; + const revisions = new Map([ + ["src/a.ts", "pushed-a"], + ["src/b.ts", "current-b"], + ]); + const service = yield* environmentViewedService(revisions, asked); + + yield* service.setFilesViewed({ + ...GITLAB_REFERENCE, + files: [ + { path: "src/a.ts", viewed: true, revision: "shown-a" }, + { path: "src/b.ts", viewed: true }, + ], + }); + assert.deepStrictEqual(asked, [["src/b.ts"]]); + yield* service.invalidate({ reference: GITLAB_REFERENCE }); + assert.deepStrictEqual((yield* service.filesViewed(GITLAB_REFERENCE)).files, [ + { path: "src/a.ts", state: "dismissed" }, + { path: "src/b.ts", state: "viewed" }, + ]); + + const reads = asked.length; + revisions.set("src/a.ts", "pushed-a-again"); + yield* service.setFilesViewed({ + ...GITLAB_REFERENCE, + files: [{ path: "src/a.ts", viewed: true, revision: "pushed-a-again" }], + }); + assert.strictEqual(asked.length, reads); + assert.deepStrictEqual((yield* service.filesViewed(GITLAB_REFERENCE)).files, [ + { path: "src/a.ts", state: "viewed" }, + { path: "src/b.ts", state: "viewed" }, + ]); + }), +); + it.effect("keeps a deleted file cleared, which the head has no version of at all", () => Effect.gen(function* () { const service = yield* environmentViewedService(new Map(), []); diff --git a/apps/server/src/pullRequest/PullRequestService.ts b/apps/server/src/pullRequest/PullRequestService.ts index 854d182f155ec..4a797f82842d6 100644 --- a/apps/server/src/pullRequest/PullRequestService.ts +++ b/apps/server/src/pullRequest/PullRequestService.ts @@ -1922,7 +1922,14 @@ export const make = Effect.gen(function* () { // Deliberately not `invalidatedByMutation`: ticking a file off says nothing about the // change request, and dropping a 300-file diff on every checkbox is the whole cost of // the feature. - Effect.ensuring(Effect.sync(() => bumpFilesViewedEpoch(ref))), + Effect.ensuring( + Effect.sync(() => { + bumpFilesViewedEpoch(ref); + if (input.files.some((file) => file.viewed && file.revision !== undefined)) { + bumpEpoch(fileRevisionEpochs, ref); + } + }), + ), ), ), ); @@ -2811,6 +2818,7 @@ export const make = Effect.gen(function* () { const filesViewedEpoch = (ref: PullRequestRef) => filesViewedEpochs.get(refScope(ref)) ?? 0; const bumpFilesViewedEpoch = (ref: PullRequestRef) => bumpEpoch(filesViewedEpochs, ref); + const fileRevisionEpochs = new Map(); let everyFileRevisionEpoch = 0; const viewedFiles = ViewedFiles.make({ filesViewedStore, @@ -2819,7 +2827,8 @@ export const make = Effect.gen(function* () { toPullRequestError, runFork, refEpoch, - fileRevisionsEpoch: () => everyFileRevisionEpoch, + fileRevisionsEpoch: (ref) => + `${everyFileRevisionEpoch}:${fileRevisionEpochs.get(refScope(ref)) ?? 0}`, }); const filtersOfKey = ( diff --git a/apps/server/src/pullRequest/pullRequestViewedFiles.ts b/apps/server/src/pullRequest/pullRequestViewedFiles.ts index 596808eb46a07..b0e343d528e1c 100644 --- a/apps/server/src/pullRequest/pullRequestViewedFiles.ts +++ b/apps/server/src/pullRequest/pullRequestViewedFiles.ts @@ -30,7 +30,7 @@ const MAX_FILE_REVISION_PATHS = 1_000; interface FileRevisionsDependencies { readonly runFork: (effect: Effect.Effect) => unknown; readonly refEpoch: (ref: PullRequestRef) => number; - readonly fileRevisionsEpoch: () => number; + readonly fileRevisionsEpoch: (ref: PullRequestRef) => string; readonly toPullRequestError: ( operation: string, ) => (error: PullRequestProviderError) => PullRequestError; @@ -50,7 +50,7 @@ const makeFileRevisions = (dependencies: FileRevisionsDependencies) => { const fileRevisionsKey = (project: SupportedProject, ref: PullRequestRef) => [ refEpoch({ ...ref, host: project.host, repository: project.repository }), - fileRevisionsEpoch(), + fileRevisionsEpoch({ ...ref, host: project.host, repository: project.repository }), ref.projectId, project.repository.trim().toLowerCase(), ref.number, @@ -133,10 +133,9 @@ const makeFileRevisions = (dependencies: FileRevisionsDependencies) => { return Effect.flatMap(Clock.currentTimeMillis, (now) => { const key = fileRevisionsKey(project, ref); const held = heldFileRevisionsFor(key, paths, now); - if (held === null) return fetch; + if (held === null || freshness === "fresh") return fetch; if (now - held.at <= Duration.toMillis(FILE_REVISIONS_CACHE_TTL)) return Effect.succeed(held.revisions); - if (freshness === "fresh") return fetch; if (refreshingFileRevisions.has(key)) return Effect.succeed(held.revisions); // Refresh independently; it outlives the answered caller. return Effect.sync(() => { @@ -260,7 +259,9 @@ export const make = (dependencies: Dependencies) => { input: PullRequestSetFilesViewedInput, ): Effect.fn.Return { const viewer = yield* requiredViewerOf(project, "setFilesViewed"); - const cleared = input.files.filter((file) => file.viewed).map((file) => file.path); + const cleared = input.files + .filter((file) => file.viewed && file.revision === undefined) + .map((file) => file.path); const revisions = cleared.length === 0 ? null @@ -279,7 +280,7 @@ export const make = (dependencies: Dependencies) => { // Missing paths have null baselines; empty revision means confirmed deletion. files: input.files.map((file) => ({ path: file.path, - revision: revisions?.get(file.path) ?? null, + revision: file.revision ?? revisions?.get(file.path) ?? null, viewed: file.viewed, })), viewedAt, diff --git a/apps/web/src/components/pullRequest/PullRequestCodeTab.tsx b/apps/web/src/components/pullRequest/PullRequestCodeTab.tsx index 94f4ccda82558..46bfc4bbbcfe1 100644 --- a/apps/web/src/components/pullRequest/PullRequestCodeTab.tsx +++ b/apps/web/src/components/pullRequest/PullRequestCodeTab.tsx @@ -722,7 +722,10 @@ function PullRequestCodeTab({ extendRange ? lastViewedFileKey : null, ); setLastViewedFileKey(fileKey); - for (const file of range) setViewed(file.path, viewed); + for (const file of range) { + // Commit diffs name the commit's version rather than the current head. + setViewed(file.path, viewed, commit === null ? file.fileDiff.newObjectId : undefined); + } setToggledFiles((current) => range.reduce( (next, file) => diff --git a/apps/web/src/components/pullRequest/filesViewedStore.test.ts b/apps/web/src/components/pullRequest/filesViewedStore.test.ts index 9633e1306689d..5303720c7b0c2 100644 --- a/apps/web/src/components/pullRequest/filesViewedStore.test.ts +++ b/apps/web/src/components/pullRequest/filesViewedStore.test.ts @@ -71,14 +71,17 @@ function setup(initial: PullRequestFilesViewedResult | null = answer(false)) { describe("createFilesViewedStore", () => { it("batches presses and holds them until a read that could have seen them", async () => { - using t = setup(); + const unchanged = { files: [{ path: "a.ts", state: "dismissed" as const }], truncated: false }; + using t = setup(unchanged); const scope = t.store.scope(first); const release = scope.subscribe(() => {}); - scope.setViewed("a.ts", true); + scope.setViewed("a.ts", true, "shown-revision"); expect(scope.getSnapshot().get("a.ts")).toBe(true); await vi.advanceTimersByTimeAsync(400); expect(t.send).toHaveBeenCalledTimes(1); - expect(t.send.mock.calls[0]?.[1]).toEqual([{ path: "a.ts", viewed: true }]); + expect(t.send.mock.calls[0]?.[1]).toEqual([ + { path: "a.ts", viewed: true, revision: "shown-revision" }, + ]); t.settles[0]?.(AsyncResult.success(undefined)); await vi.advanceTimersByTimeAsync(0); @@ -86,10 +89,7 @@ describe("createFilesViewedStore", () => { expect(scope.getSnapshot().get("a.ts")).toBe(true); // A push landed between the write and the next read: the host's answer wins. - t.answerFor(first, { - files: [{ path: "a.ts", state: "dismissed" }], - truncated: false, - }); + t.answerFor(first, unchanged); expect(scope.getSnapshot().size).toBe(0); release(); }); @@ -203,9 +203,6 @@ describe("createFilesViewedStore", () => { await vi.advanceTimersByTimeAsync(400); t.settles[0]?.(AsyncResult.success(undefined)); await vi.advanceTimersByTimeAsync(0); - // The same answer published again is not a new read. - t.answerFor(first, agreeing); - expect(scope.getSnapshot().get("a.ts")).toBe(true); t.answerFor(first, answer(false)); expect(scope.getSnapshot().size).toBe(0); release(); diff --git a/apps/web/src/components/pullRequest/filesViewedStore.ts b/apps/web/src/components/pullRequest/filesViewedStore.ts index cae490ded8d66..1ddd675e771a7 100644 --- a/apps/web/src/components/pullRequest/filesViewedStore.ts +++ b/apps/web/src/components/pullRequest/filesViewedStore.ts @@ -8,10 +8,12 @@ import type { PullRequestFilesViewedResult, PullRequestRef, } from "@t3tools/contracts"; -import { AsyncResult } from "effect/reactivity"; import { appAtomRegistry } from "~/rpc/atomRegistry"; -import { pullRequestEnvironment } from "~/state/pullRequests"; +import { + pullRequestEnvironment, + subscribePullRequestFilesViewedAnswers, +} from "~/state/pullRequests"; import { toastManager } from "../ui/toast"; import { @@ -20,6 +22,7 @@ import { toFileViewedBatch, toFileViewedStates, type FileViewedOverlay, + type FileViewedPress, } from "./pullRequestFilesViewed.logic"; export interface FilesViewedTarget { @@ -27,11 +30,15 @@ export interface FilesViewedTarget { readonly reference: PullRequestRef; } -type FilesViewedBatch = ReadonlyArray<{ readonly path: string; readonly viewed: boolean }>; +type FilesViewedBatch = ReadonlyArray<{ + readonly path: string; + readonly viewed: boolean; + readonly revision?: string; +}>; export interface FilesViewedStoreDeps { readonly flushDelayMs: number; - /** Calls `onAnswer` with the host's current answer at once, then with every later one. */ + /** Calls `onAnswer` only after completed server reads, excluding persisted values. */ readonly subscribeAnswers: ( target: FilesViewedTarget, onAnswer: (answer: PullRequestFilesViewedResult | null) => void, @@ -48,7 +55,7 @@ export interface FilesViewedScopeHandle { /** Holds the scope open while subscribed; the last reader to leave sends its last presses. */ readonly subscribe: (listener: () => void) => () => void; readonly getSnapshot: () => FileViewedOverlay; - readonly setViewed: (path: string, viewed: boolean) => void; + readonly setViewed: (path: string, viewed: boolean, revision?: string) => void; } const NO_OVERLAY: FileViewedOverlay = new Map(); @@ -58,17 +65,12 @@ interface FilesViewedScope { readonly listeners: Set<() => void>; readonly unsubscribeAnswers: () => void; overlay: FileViewedOverlay; - queued: Map; + queued: Map; readonly sentBy: Map; requests: number; timer: ReturnType | null; - answer: PullRequestFilesViewedResult | null; - /** - * The host's answer as it stood when a completed write was acknowledged, per path. The first - * answer that differs from it is the first read that could have seen the write, which is what - * retires the press rather than the host happening to agree with it. - */ - readonly answeredFrom: Map; + // Completed reads retire acknowledged presses even when equal data is structurally shared. + readonly answeredFrom: Set; } /** @@ -89,12 +91,11 @@ export function createFilesViewedStore(deps: FilesViewedStoreDeps) { const isLive = (key: string, scope: FilesViewedScope) => scopes.get(key) === scope; const settle = (scope: FilesViewedScope, answer: PullRequestFilesViewedResult | null) => { - if (answer === scope.answer) return; - scope.answer = answer; + if (answer === null) return; const pending = new Set([...scope.queued.keys(), ...scope.sentBy.keys()]); const answered = new Set(); - for (const [path, from] of scope.answeredFrom) { - if (pending.has(path) || from === answer) continue; + for (const path of scope.answeredFrom) { + if (pending.has(path)) continue; answered.add(path); scope.answeredFrom.delete(path); } @@ -135,7 +136,7 @@ export function createFilesViewedStore(deps: FilesViewedStoreDeps) { } // Answered for from the next read on, whatever it says. A push landing between the write // and that read comes back as `dismissed`, and the press must not stand over it. - for (const path of mine) scope.answeredFrom.set(path, scope.answer); + for (const path of mine) scope.answeredFrom.add(path); deps.refresh(scope.target); }); }; @@ -154,10 +155,8 @@ export function createFilesViewedStore(deps: FilesViewedStoreDeps) { if (scope === undefined) { const listeners = new Set<() => void>(); let created: FilesViewedScope | undefined; - let initial: PullRequestFilesViewedResult | null = null; const unsubscribeAnswers = deps.subscribeAnswers(target, (answer) => { - if (created === undefined) initial = answer; - else settle(created, answer); + if (created !== undefined) settle(created, answer); }); created = { target, @@ -168,8 +167,7 @@ export function createFilesViewedStore(deps: FilesViewedStoreDeps) { sentBy: new Map(), requests: 0, timer: null, - answer: initial, - answeredFrom: new Map(), + answeredFrom: new Set(), }; scope = created; scopes.set(key, scope); @@ -203,10 +201,10 @@ export function createFilesViewedStore(deps: FilesViewedStoreDeps) { const handle: FilesViewedScopeHandle = { subscribe: (listener) => acquire(key, owned, handle, listener), getSnapshot: () => scopes.get(key)?.overlay ?? NO_OVERLAY, - setViewed: (path, viewed) => { + setViewed: (path, viewed, revision) => { const scope = scopes.get(key); if (scope === undefined) return; - scope.queued.set(path, viewed); + scope.queued.set(path, { viewed, revision }); publish(scope, new Map(scope.overlay).set(path, viewed)); if (scope.timer !== null) clearTimeout(scope.timer); scope.timer = setTimeout(() => flush(key, scope), deps.flushDelayMs); @@ -234,12 +232,8 @@ const filesViewedAtom = ({ environmentId, reference }: FilesViewedTarget) => const filesViewedStore = createFilesViewedStore({ flushDelayMs: 400, - subscribeAnswers: (target, onAnswer) => - appAtomRegistry.subscribe( - filesViewedAtom(target), - (result) => onAnswer(AsyncResult.getOrElse(result, () => null)), - { immediate: true }, - ), + subscribeAnswers: ({ environmentId, reference }, onAnswer) => + subscribePullRequestFilesViewedAnswers({ environmentId, input: reference }, onAnswer), send: ({ environmentId, reference }, files) => runAtomCommand( appAtomRegistry, diff --git a/apps/web/src/components/pullRequest/pullRequestFilesViewed.logic.ts b/apps/web/src/components/pullRequest/pullRequestFilesViewed.logic.ts index 05f5fdc6dceda..89874004a9220 100644 --- a/apps/web/src/components/pullRequest/pullRequestFilesViewed.logic.ts +++ b/apps/web/src/components/pullRequest/pullRequestFilesViewed.logic.ts @@ -58,8 +58,15 @@ export function revertFileViewedOverlay( return next.size === overlay.size ? overlay : next; } +export interface FileViewedPress { + readonly viewed: boolean; + readonly revision: string | undefined; +} + export function toFileViewedBatch( - overlay: FileViewedOverlay, -): ReadonlyArray<{ readonly path: string; readonly viewed: boolean }> { - return [...overlay].map(([path, viewed]) => ({ path, viewed })); + presses: ReadonlyMap, +): ReadonlyArray<{ readonly path: string; readonly viewed: boolean; readonly revision?: string }> { + return [...presses].map(([path, { viewed, revision }]) => + revision === undefined ? { path, viewed } : { path, viewed, revision }, + ); } diff --git a/apps/web/src/components/pullRequest/usePullRequestFilesViewed.ts b/apps/web/src/components/pullRequest/usePullRequestFilesViewed.ts index f71b135b6044d..05923487b5ab3 100644 --- a/apps/web/src/components/pullRequest/usePullRequestFilesViewed.ts +++ b/apps/web/src/components/pullRequest/usePullRequestFilesViewed.ts @@ -15,7 +15,7 @@ export interface PullRequestFilesViewedView { readonly enabled: boolean; readonly isViewed: (path: string) => boolean; readonly isStale: (path: string) => boolean; - readonly setViewed: (path: string, viewed: boolean) => void; + readonly setViewed: (path: string, viewed: boolean, revision: string | undefined) => void; readonly truncated: boolean; readonly error: string | null; readonly refresh: () => void; diff --git a/apps/web/src/state/pullRequests.ts b/apps/web/src/state/pullRequests.ts index c8ac23ced0584..59a5c04e7af57 100644 --- a/apps/web/src/state/pullRequests.ts +++ b/apps/web/src/state/pullRequests.ts @@ -10,6 +10,7 @@ import { type EnvironmentId, type ProjectId, type PullRequestChecksState, + type PullRequestFilesViewedResult, type PullRequestListInput, type PullRequestFilterOptionsInput, type PullRequestListStatsInput, @@ -366,8 +367,44 @@ export function restoredPullRequestList( return restorePullRequestList(snapshotStorage(), targets); } +const filesViewedAnswerListeners = new Map< + string, + Set<(answer: PullRequestFilesViewedResult) => void> +>(); + +function filesViewedAnswerKey({ environmentId, input }: EnvironmentQueryTarget) { + return JSON.stringify([ + environmentId, + input.projectId, + input.host, + input.expectedAccountId, + input.allowStale, + input.repository, + input.number, + ]); +} + +export function subscribePullRequestFilesViewedAnswers( + target: EnvironmentQueryTarget, + onAnswer: (answer: PullRequestFilesViewedResult) => void, +) { + const key = filesViewedAnswerKey(target); + let listeners = filesViewedAnswerListeners.get(key); + if (listeners === undefined) filesViewedAnswerListeners.set(key, (listeners = new Set())); + listeners.add(onAnswer); + return () => { + listeners.delete(onAnswer); + if (listeners.size === 0) filesViewedAnswerListeners.delete(key); + }; +} + // Answers are stamped on arrival: older servers leave no other ordering for same-dated reads. export const pullRequestEnvironment = createPullRequestEnvironmentAtoms(connectionAtomRuntime, { + filesViewed: (target, answer) => + Effect.sync(() => { + for (const listener of filesViewedAnswerListeners.get(filesViewedAnswerKey(target)) ?? []) + listener(answer); + }), list: ({ environmentId, input }, answer, registry) => Effect.map(Clock.currentTimeMillis, (observedAt) => { rememberPullRequestListAnswer(snapshotStorage(), { environmentId, input }, answer); diff --git a/packages/client-runtime/src/state/pullRequests.ts b/packages/client-runtime/src/state/pullRequests.ts index ca5fb6d7a77d5..15550957e8fb9 100644 --- a/packages/client-runtime/src/state/pullRequests.ts +++ b/packages/client-runtime/src/state/pullRequests.ts @@ -148,6 +148,7 @@ export interface PullRequestAnswerHandlers { readonly list?: QueryAnswerHandler; readonly listStats?: QueryAnswerHandler; readonly detail?: QueryAnswerHandler; + readonly filesViewed?: QueryAnswerHandler; readonly checks?: QueryAnswerHandler; readonly reviewGuide?: QueryAnswerHandler; } @@ -422,6 +423,7 @@ export function createPullRequestEnvironmentAtoms( }), filesViewed: createEnvironmentRpcQueryAtomFamily(runtime, { label: "environment-data:pull-requests:files-viewed", + ...(onAnswer.filesViewed === undefined ? {} : { onAnswer: onAnswer.filesViewed }), persistence: persistQuery(PullRequestFilesViewedResult), tag: WS_METHODS.pullRequestsFilesViewed, execute: (input) => routedRequest(WS_METHODS.pullRequestsFilesViewed, input), diff --git a/packages/contracts/src/pullRequest.ts b/packages/contracts/src/pullRequest.ts index 64111ab4742b1..5bb8d104b2c32 100644 --- a/packages/contracts/src/pullRequest.ts +++ b/packages/contracts/src/pullRequest.ts @@ -714,6 +714,8 @@ export const PullRequestSetFilesViewedInput = Schema.Struct({ Schema.Struct({ path: FilePath, viewed: Schema.Boolean, + /** The displayed revision; omitted clients fall back to a fresh head read. */ + revision: Schema.optional(TrimmedNonEmptyString.check(Schema.isMaxLength(256))), }), ).check(Schema.isMaxLength(MAX_FILES_VIEWED_PRESSES)), }); diff --git a/packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.test.ts b/packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.test.ts index bf5ea9d944738..355199aa569de 100644 --- a/packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.test.ts +++ b/packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.test.ts @@ -248,6 +248,24 @@ layer("BitbucketPullRequestApi.layer", (it) => { }), ); + it.effect("reads the pushed revision on consecutive comparison reads", () => + Effect.gen(function* () { + for (const revision of ["2222222", "3333333"]) { + mockedRequest.mockReturnValueOnce( + Effect.succeed( + response( + `diff --git a/a.ts b/a.ts\nindex 1111111..${revision} 100644\n@@ -1 +1 @@\n-a\n+b\n`, + ), + ), + ); + } + const api = yield* BitbucketPullRequestApi.BitbucketPullRequestApi; + const input = { repository: "acme/web", number: 73, paths: ["a.ts"] }; + assert.strictEqual((yield* api.getFileRevisions(input)).revisions.get("a.ts"), "2222222"); + assert.strictEqual((yield* api.getFileRevisions(input)).revisions.get("a.ts"), "3333333"); + }), + ); + it.effect("says nothing about the files past the end of a patch it could not read whole", () => Effect.gen(function* () { mockedRequest.mockReturnValueOnce( diff --git a/packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.ts b/packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.ts index 9f55e34f43060..511e9f3aed21f 100644 --- a/packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.ts +++ b/packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.ts @@ -1,9 +1,6 @@ import { isCommitSha } from "@t3tools/source-control-core/server/pullRequestUtils"; -import * as Cache from "effect/Cache"; import * as Context from "effect/Context"; -import * as Duration from "effect/Duration"; import * as Effect from "effect/Effect"; -import * as Exit from "effect/Exit"; import * as Layer from "effect/Layer"; import * as Result from "effect/Result"; import * as Schema from "effect/Schema"; @@ -122,12 +119,6 @@ const MAX_LIST_PAGES = 10; const CONVERSATION_PAGE_SIZE = 50; const CONVERSATION_PAGES = 10; const DIFF_MAX_BYTES = 8 * 1024 * 1024; -/** Must expire before the caller's version cache so refreshes reach Bitbucket. */ -const REVISION_PATCH_TTL = Duration.seconds(5); -const REVISION_PATCH_CAPACITY = 16; -const encodeRevisionKey = Schema.encodeSync( - Schema.fromJsonString(Schema.Tuple([Schema.String, Schema.Number])), -); export interface BitbucketPullRequestBatch { readonly items: ReadonlyArray; readonly truncated: boolean; @@ -508,23 +499,6 @@ export const make = Effect.gen(function* () { ), ); - // Cache revisions, not large patch bodies that need reparsing. - const revisionPatches = yield* Cache.makeWith( - (key: string) => { - const [repository, number] = JSON.parse(key) as [string, number]; - return pullRequestDiff({ repository, number }).pipe( - Effect.map((diff) => ({ - revisions: parseDiffFileRevisions(diff.patch), - truncated: diff.truncated, - })), - ); - }, - { - capacity: REVISION_PATCH_CAPACITY, - timeToLive: (exit) => (Exit.isSuccess(exit) ? REVISION_PATCH_TTL : Duration.zero), - }, - ); - return BitbucketPullRequestApi.of({ getViewer: Effect.fn("BitbucketPullRequestApi.getViewer")(function () { return bitbucket.request({ method: "GET", url: "/user" }).pipe( @@ -598,12 +572,9 @@ export const make = Effect.gen(function* () { getFileRevisions: Effect.fn("BitbucketPullRequestApi.getFileRevisions")(function* (input) { if (input.paths.length === 0) return { revisions: new Map(), complete: false }; - return yield* Cache.get( - revisionPatches, - encodeRevisionKey([input.repository, input.number]), - ).pipe( + return yield* pullRequestDiff(input).pipe( Effect.map((diff) => { - const revisions = new Map(diff.revisions); + const revisions = new Map(parseDiffFileRevisions(diff.patch)); // A truncated patch cannot identify absent paths as deleted. if (!diff.truncated) { for (const path of input.paths) {