feat: mark parts reviewed, kept locally by content hash

CONTEXT.md

@@ -76,6 +76,10 @@ _Avoid_: Supported model, certified model
 A fixed, typed request the reviewer makes about one part for deeper analysis, such as "explain" or "verify this claim".
 _Avoid_: Chat, prompt, question
 
+**Reviewed mark**:
+A part the reviewer has checked off, stored locally per pull request and keyed by the part's content, cleared automatically when that content changes, and mirrored to GitHub's **Viewed** only for whole files when the opt-in setting is on.
+_Avoid_: Tick, checkmark, done flag
+
 **Comment**:
 A review comment the reviewer sends to GitHub from the companion, as part of one pending GitHub review.
 _Avoid_: Bot comment, annotation

packages/engine/src/diff.ts

@@ -15,13 +15,16 @@ interface CurrentFile {
   newHeaderPath?: string;
   /** True while skipping the base85 payload of a `GIT binary patch`. */
   inBinaryPatch: boolean;
+  /** The blob ids the `index` line names, kept on the part only when it is binary. */
+  blobs?: { old: string; new: string };
 }
 
 const DIFF_GIT = /^diff --git (.*)$/;
 const OLD_MODE = /^old mode (\d+)$/;
 const NEW_MODE = /^new mode (\d+)$/;
 const DELETED_FILE_MODE = /^deleted file mode (\d+)$/;
 const NEW_FILE_MODE = /^new file mode (\d+)$/;
+const INDEX = /^index ([0-9a-f]+)\.\.([0-9a-f]+)(?: \d+)?$/;
 const RENAME_FROM = /^rename from (.+)$/;
 const RENAME_TO = /^rename to (.+)$/;
 const COPY_FROM = /^copy from (.+)$/;
@@ -306,7 +309,9 @@ export function parseDiff(diff: string): ParsedDiff {
     }
 
     if (/^(similarity|dissimilarity) index /.test(line) || line.startsWith('index ')) {
-      // Carried in the diff but not needed by any part field yet.
+      // Only a binary file needs its blob ids: no hunk shows its change.
+      match = INDEX.exec(line);
+      if (match) current.blobs = { old: match[1]!, new: match[2]! };
       hunk = undefined;
       lastDiffLine = undefined;
       continue;
@@ -482,6 +487,7 @@ function syntaxNotYetRun(): PartSyntax {
 /** Applies the header paths to a finished file, in precedence order. */
 function finishFile(current: CurrentFile): void {
   const { part } = current;
+  if (part.isBinary && current.blobs !== undefined) part.blobs = current.blobs;
   if (current.newHeaderPath !== undefined) {
     part.path = current.newHeaderPath;
   } else if (part.changeKind === 'deletion' && current.oldHeaderPath !== undefined) {

packages/engine/src/github.ts

@@ -51,7 +51,9 @@ const silentLog = {
 /**
  * The engine's view of GitHub, through the official client: read for the
  * review, its checks and their failed jobs' logs included, and one write
- * — submitting the review — when the reviewer sends it (ADR 0002).
+ * — submitting the review — when the reviewer sends it (ADR 0002), besides
+ * marking files "Viewed" when the reviewer's opt-in setting mirrors their
+ * reviewed marks there.
  *
  * The token lives only in the Octokit instance's memory: the client writes
  * it nowhere and echoes it in no error or log line.
@@ -301,6 +303,26 @@ export class GitHubClient {
     return { url: data.html_url };
   }
 
+  /**
+   * Marks files of the pull request "Viewed" on GitHub, the field the
+   * GitHub Pull Requests extension syncs too: one GraphQL mutation per
+   * path, after one query for the pull request's id. Only the reviewer's
+   * opt-in setting asks for it, and only for files whose every part they
+   * reviewed. Nothing is ever unmarked.
+   */
+  async markFilesAsViewed(ref: PullRequestRef, paths: readonly string[]): Promise<void> {
+    if (paths.length === 0) return;
+    const answer = graphqlData(
+      await this.requestGraphql(PULL_REQUEST_ID_QUERY, { owner: ref.owner, name: ref.repo, number: ref.number }),
+      'pull-request-id query',
+    ) as { repository?: { pullRequest?: { id?: unknown } | null } | null };
+    const id = answer.repository?.pullRequest?.id;
+    if (typeof id !== 'string') throw new Error(`GitHub has no pull request ${ref.owner}/${ref.repo}#${ref.number}`);
+    for (const path of paths) {
+      graphqlData(await this.requestGraphql(MARK_FILE_AS_VIEWED_MUTATION, { id, path }), 'mark-as-viewed mutation');
+    }
+  }
+
   /**
    * Reads the repository's root `.gitattributes` as stored at the given
    * commit, without a checkout: the contents endpoint serves the blob at
@@ -384,17 +406,32 @@ const LINKED_ISSUES_QUERY = `query($owner: String!, $name: String!, $number: Int
 
 /** A GraphQL answer that failed, with the message of its first error. */
 function linkedAnswer(response: unknown): GraphQLData {
+  return graphqlData(response, 'linked-issues query') as GraphQLData;
+}
+
+/** A GraphQL answer's data, or a plain error naming the request when GitHub reports one. */
+function graphqlData(response: unknown, request: string): unknown {
   const body = (response as GraphQLAnswer) ?? {};
   const errors = body.errors;
   if (errors !== undefined && errors.length > 0) {
     const message = errors[0]?.message;
     throw new Error(
-      `GitHub's linked-issues query failed${typeof message === 'string' ? `: ${message}` : ''}`,
+      `GitHub's ${request} failed${typeof message === 'string' ? `: ${message}` : ''}`,
     );
   }
   return body.data ?? {};
 }
 
+/** The query that reads the pull request's GraphQL id, which the mark-as-viewed mutation names it by. */
+const PULL_REQUEST_ID_QUERY = `query($owner: String!, $name: String!, $number: Int!) {
+  repository(owner: $owner, name: $name) { pullRequest(number: $number) { id } }
+}`;
+
+/** The mutation that marks one file of the pull request "Viewed" for the signed-in reviewer. */
+const MARK_FILE_AS_VIEWED_MUTATION = `mutation($id: ID!, $path: String!) {
+  markFileAsViewed(input: { pullRequestId: $id, path: $path }) { clientMutationId }
+}`;
+
 /** One linked issue, read defensively: anything GitHub leaves out reads as absent. */
 function linkedIssue(node: unknown, link: LinkedIssue['link']): LinkedIssue | undefined {
   if (typeof node !== 'object' || node === null) return undefined;

packages/engine/src/grouping.ts

@@ -172,18 +172,22 @@ export function groupingPrompt(
 
 /**
  * Checks an answer against the offered changes: every part needs a name
- * and at least one hunk, and every id must be one that was offered, named
- * once. Returns the problems; an empty list means the answer is usable.
- * Ids the answer leaves out are not a problem here: the coverage rule
- * collects them.
+ * and at least one hunk, no two parts share a name — including the one
+ * the part of left-out hunks takes, since the reviewed marks key on it —
+ * and every id must be one that was offered, named once. Returns the
+ * problems; an empty list means the answer is usable. Ids the answer
+ * leaves out are not a problem here: the coverage rule collects them.
  */
 export function groupingProblems(items: readonly GroupingItem[], answer: GroupingAnswer): string[] {
   const offered = new Set(items.map((item) => item.id));
   const placed = new Set<string>();
+  const names = new Set<string>();
   const problems: string[] = [];
   answer.parts.forEach((part, index) => {
     const name = part.name.replace(/\s+/g, ' ').trim();
     if (name === '') problems.push(`part ${index + 1} has no name`);
+    else if (names.has(name)) problems.push(`part ${index + 1}'s name is used by more than one part`);
+    else names.add(name);
     if (name.length > MAX_NAME_LENGTH) problems.push(`part ${index + 1}'s name is over ${MAX_NAME_LENGTH} characters`);
     if (part.hunks.length === 0) problems.push(`part ${index + 1} has no hunks`);
     for (const id of part.hunks) {
@@ -192,6 +196,9 @@ export function groupingProblems(items: readonly GroupingItem[], answer: Groupin
       placed.add(id);
     }
   });
+  if (names.has(NOT_GROUPED_BY_AGENT) && items.some((item) => !placed.has(item.id))) {
+    problems.push(`a part is named ${JSON.stringify(NOT_GROUPED_BY_AGENT)}, which is reserved for the hunks left out`);
+  }
   return problems;
 }
 
@@ -249,10 +256,11 @@ export interface AgentGroupingResult {
 
 /**
  * Asks the agent to group the files' hunks into parts, and checks its
- * answer. An answer that misses the schema or names an id that was not
- * offered, or one id twice, is retried once and then reported, and the
- * plain grouping stays; the hunks a valid answer leaves out go to a part
- * marked {@link NOT_GROUPED_BY_AGENT}. Sinking noise keeps its plain parts.
+ * answer. An answer that misses the schema, names an id that was not
+ * offered or one id twice, or names two parts alike, is retried once and
+ * then reported, and the plain grouping stays; the hunks a valid answer
+ * leaves out go to a part marked {@link NOT_GROUPED_BY_AGENT}. Sinking
+ * noise keeps its plain parts.
  */
 export async function groupWithAgent(
   files: readonly Part[],

packages/engine/src/index.ts

@@ -47,4 +47,5 @@ export * from './ci.js';
 export * from './criteria.js';
 export * from './criteria-mapping.js';
 export * from './draft-comment.js';
+export * from './reviewed-marks.js';
 export { runCli } from './cli.js';

packages/engine/src/parts.ts

@@ -6,7 +6,8 @@ const NAMED_ENTITIES = 3;
 /** The group every hunk outside all entities joins, one per file. */
 const TOP_LEVEL = 'top level';
 
-function entityKey(entity: Entity): string {
+/** An entity's key: its kind with its qualified name, which same-named entities of different kinds never share. */
+export function entityKey(entity: Entity): string {
   return `${entity.kind} ${entity.name}`;
 }
 

packages/engine/src/protocol.ts

@@ -178,6 +178,43 @@ export interface DraftComment {
   stamp: AgentStamp;
 }
 
+/**
+ * One reviewed mark: the reviewer ticked a part's checkbox. The local
+ * per-pull-request store keys it by the part's content hash when it was
+ * marked, and it covers the content hashes of the part's pieces — each
+ * hunk, or a file without hunks — so a part whose content changes is
+ * unmarked, and a regrouped part whose every piece was marked stays
+ * reviewed.
+ */
+export interface ReviewedMark {
+  /** The part's content hash when the reviewer marked it: the store's key. */
+  hash: string;
+  /** The part's identity when marked — its name with its files and entity kinds — so a part that changed since can say so. */
+  name: string;
+  /** The content hashes of the pieces the mark covers. */
+  pieces: string[];
+  /** When the reviewer marked it, as an ISO 8601 timestamp. */
+  markedAt: string;
+}
+
+/** The reviewed marks of one pull request, as its local store holds them. */
+export interface ReviewedMarks {
+  marks: ReviewedMark[];
+}
+
+/**
+ * Where a part stands against the reviewed marks: **reviewed** when every
+ * piece of it is marked; **changed since marked** when the reviewer marked
+ * it, or some of it, and its content changed since; **not reviewed**
+ * otherwise.
+ */
+export type ReviewedState = 'reviewed' | 'changed since marked' | 'not reviewed';
+
+/** The files marked "Viewed" on GitHub, by their paths. */
+export interface ViewedFiles {
+  paths: string[];
+}
+
 /** The review result the engine produces for one pull request. */
 export interface ReviewResult {
   /** Schema version; compare against {@link REVIEW_RESULT_VERSION}. */
@@ -1078,6 +1115,12 @@ export interface Part {
   changeKind: ChangeKind;
   /** True when the changed content is binary, so there are no hunks to read. */
   isBinary: boolean;
+  /**
+   * The abbreviated blob ids of the old and new sides, from the diff's
+   * `index` line; kept only for a binary file, whose change no hunk
+   * shows, so its content hash still changes with its content.
+   */
+  blobs?: { old: string; new: string };
   /** File mode on the old side, when the diff reports one. */
   oldMode?: string;
   /** File mode on the new side, when the diff reports one. */