Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion ARCHITECTURE.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ server/ # webhook server (GitHub App)
- **read-only default**: every GitHub mutation (labels, comments, closes, issue creation) funnels through one `write-gate.ts` gate that defaults to dry-run. CLI writes only under `--apply-labels`; the webhook server writes only when `PRISM_APPLY=1`. `--dry-run` always wins. (Fixes the prior leak where `ensureLabelsExist` created labels even under `--dry-run`, and the server writing unconditionally.)
- **cross-repo**: config accepts multiple repos, dupe detection works across repo boundaries
- **canonical selection**: one `selectCanonical()` (src/canonical.ts) picks each cluster's source of truth for the report, the starmap payload, and the live triage bot alike. issue-majority clusters resolve to the earliest report (the original bug); PR-majority ranks by lifecycle state (merged > open > closed), then a CI veto (a known-red build never outranks a same-state green sibling, before score), then quality score. the veto stops a high-scored PR with failing checks from becoming bestPick over the green fix that actually landed; only `ciStatus === "failure"` demotes, so a not-yet-reported PR is never penalized. fully deterministic - every tie bottoms out at item number - so re-runs name the same canonical. confirmed (identity) clusters override to the earliest-created rule: byte-identical dupes resolve by which-was-first, never score (a copy can outscore its original)
- **incident awareness**: a repository-wide event (visibility flip, bulk close, migration) can close hundreds of PRs for reasons unrelated to their quality. because `selectCanonical()` ranks lifecycle state before score, those items would otherwise rank as rejections and sink below genuinely-closed siblings. `prism.config.yaml` accepts an `incidents:` list of `{start, end, reason}` windows; `store.ts` stamps `incidentClosed` onto each item at hydration via `isIncidentClosed()` (src/incident.ts), and `statePriority()` ranks an incident-closed PR as open. the raw `closedAt` is what gets persisted, never the derived flag, so correcting a mis-set window is a config edit rather than a rescan, for rows scanned since the feature landed. rows stored earlier carry no `closedAt`, and a default scan fetches open items only, so after a bulk close the affected PRs are no longer returned at all and their stored rows keep `state: open`. `prism scan --state all` is needed after each incident, not once. window bounds require an explicit UTC offset and are rejected at load if unparseable or inverted: an offset-less timestamp resolves in the host timezone, so the same config would select different items on a laptop than in CI. **CLI-only.** the server / GitHub-App path (`server/db.ts`, `server/scheduler.ts`) does not apply windows: `ServerConfig` has no `incidents` field and the scheduler fetches open items only, so `closedAt` is never populated there. that path is not otherwise frozen: `scheduler.ts` calls `findDuplicateClusters`, so the *built-in* bot list applies there. the `cluster.*` config does not, because `ServerConfig` has no `cluster` field, so a repo-specific `cluster.bot_authors` is honoured by the CLI and ignored by the App. wiring incident awareness there needs a `ServerConfig.incidents` field, a scheduler that fetches closed items, and `server/triage.ts`'s hand-rolled `metadata: { author, state }` literal replaced with `itemMetadata()` the way `scheduler.ts` now is: tracked in #27. the starmap payload carries `incidentClosed: true` (omitted when false, keeping the contract additive) so a consumer can bucket them for re-triage rather than treating them as rejected
- **incident awareness**: a repository-wide event (visibility flip, bulk close, migration) can close hundreds of PRs for reasons unrelated to their quality. because `selectCanonical()` ranks lifecycle state before score, those items would otherwise rank as rejections and sink below genuinely-closed siblings. `prism.config.yaml` accepts an `incidents:` list of `{start, end, reason}` windows; `store.ts` stamps `incidentClosed` onto each item at hydration via `isIncidentClosed()` (src/incident.ts), and `statePriority()` ranks an incident-closed PR as open. the raw `closedAt` is what gets persisted, never the derived flag, so correcting a mis-set window is a config edit rather than a rescan, for rows scanned since the feature landed. rows stored earlier carry no `closedAt`, and a default scan fetches open items only, so after a bulk close the affected PRs are no longer returned at all and their stored rows keep `state: open`. `prism scan --state all` is needed after each incident, not once. window bounds require an explicit UTC offset and are rejected at load if unparseable or inverted: an offset-less timestamp resolves in the host timezone, so the same config would select different items on a laptop than in CI. the server / GitHub-App path applies windows too: they are declared per repo in `{dataDir}/{owner}-{repo}/config.json` under `incidents`, validated on load (a readable config with a malformed window throws rather than silently becoming "no incident"), and passed through `openRepoDB()` into the same read-time flag. a scan fetches closed items only when that repo declares a window, since a window matches on `closedAt` and closed items otherwise cost API calls for nothing. the `cluster` block is declared in the same per-repo file (camelCase there: `botAuthors`, `includeBotAuthors`, matching that file's convention rather than the CLI yaml's snake_case) and reaches both server clustering sites plus the webhook triage matcher. the weekly digest loads that repo's config rather than a global one: it previously clustered every repo at `DEFAULT_REPO_CONFIG.similarityThreshold`, so it reported different clusters than the backlog scan for the same repo. the starmap payload carries `incidentClosed: true` (omitted when false, keeping the contract additive) so a consumer can bucket them for re-triage rather than treating them as rejected

- **cluster confidence**: clustering is single-linkage (BFS over pairs >= threshold) with a centroid-refinement pass to break chained mega-clusters. because single-linkage can still chain in loosely-related members, each cluster reports both `avgSimilarity` and `minSimilarity` (lowest pairwise). the report/dupes output surfaces min as a confidence tier (high >= 90%, solid >= 80%, loose < 80%) so a low-min "loose" cluster gets eyeballed before anything is closed. avg and min are computed exactly over all pairs (no sampling), so the tier a maintainer sees is reproducible run to run

Expand Down
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ all notable changes to pr-prism are documented here.
## [unreleased]

### changed
- the `cluster` block (bot filtering) now reaches the server/GitHub-App path, declared per repo in that repo's `config.json` alongside `incidents`. previously a repo-specific bot login was honoured by the CLI and silently ignored by the App. note the key names differ by file format: the CLI's yaml uses `cluster.bot_authors` / `cluster.include_bot_authors`, the App's json uses `cluster.botAuthors` / `cluster.includeBotAuthors`, matching each file's existing convention, so the same repo produced different clusters depending on which path you looked at. closes #27
- the weekly digest reads per-repo settings instead of a global copy. it was passed `DEFAULT_REPO_CONFIG.similarityThreshold` and clustered *every* repo at the default, ignoring whatever that repo had configured, so its cluster counts disagreed with the backlog scan's for the same repo. `WeeklyDigestConfig` loses `similarityThreshold` and `autoClose`: the first now comes from the repo, the second was already dead
- incident windows now work on the server/GitHub-App path, not just the CLI. declare them per repo under `incidents` in `{dataDir}/{owner}-{repo}/config.json`. a readable config carrying a malformed window throws on load rather than quietly loading as "no incident", which would rank the affected backlog as rejected. a backlog scan fetches closed items only for repos that declare a window: a window matches on `closedAt`, so without one the extra API calls buy nothing. part of #27
- the webhook path no longer overwrites what a scan learned. `server/triage.ts` wrote `metadata: { author, state }` wholesale, and because `upsert` replaces `metadata_json` outright, a `pull_request.opened` webhook draining after a backlog scan dropped every other field the scan had stored (labels, diff size, ci status, closing refs, `authorIsBot`). it now writes only the fields the event can actually observe and leaves the rest of the row alone. field names come from the shared `itemMetadata()`, and a name that is not part of it throws rather than silently creating a key nothing reads. part of #27
- bot-authored items are excluded from clustering by default. automation reuses titles for unrelated content - dependabot files "chore(deps): bump the npm group with 2 updates" week after week - so consecutive bot PRs embed as near-identical and surfaced as duplicates nobody could act on. measured on odysseus-dev/odysseus: every false positive among the extra clusters a high-recall model found was a recurring bot PR, and filtering removed exactly those two clusters (39 -> 37) while leaving all 11 genuine duplicates. set `cluster.include_bot_authors: true` to restore the old behaviour, or list repo-specific automation under `cluster.bot_authors` when a self-hosted bot is not in the built-in list. applies to confirmed (identity) clusters too, not just fuzzy ones
- confirmed (identity) duplicate clusters now pick canonical by earliest-created instead of quality score: byte-identical duplicates are a which-was-first question, and a copied PR can outscore the original it was lifted from. fuzzy clusters keep the state/CI/score rule. starmap `canonical`/`contested` for confirmed clusters shift accordingly (value change, schema stays v1)

Expand All @@ -14,7 +18,7 @@ all notable changes to pr-prism are documented here.
- `prism benchmark --out <path>` writes a run's results to a chosen file. every run previously wrote `data/benchmark-results.json`, so a second run silently destroyed the first one's numbers - an hour of embedding lost to starting the next comparison. an empty or directory-shaped value is rejected rather than falling back to the default
- benchmark results now record `clusterMembership` per model per threshold. cluster counts cannot tell you whether a model finding more clusters is catching real duplicates or chaining unrelated items, and re-deriving membership means re-embedding the whole corpus
- starmap items now carry `createdAt` alongside `updatedAt` (additive), so consumers can render and reason about which-was-first without re-fetching from github. star-map's importer rejects unknown fields; the coordinated patch is star-map PR #11, which must land before it consumes a dataset carrying this field
- incident-aware ranking: `prism.config.yaml` accepts an `incidents:` list of `{start, end, reason}` windows. a repository-wide event (visibility flip, bulk close, migration) closes items for reasons unrelated to their quality, and because `selectCanonical()` ranks lifecycle state before score those items sank below genuinely-closed siblings, inverting triage order for exactly the backlog a maintainer needs. PRs closed inside a window now rank as open. `closedAt` is captured from both API paths and stored in `metadata_json` (no schema migration); `incidentClosed` is derived at read time, so correcting a window is a config edit rather than a rescan. window bounds require an explicit UTC offset and are rejected at load if unparseable or inverted. an offset-less timestamp parses in the host timezone and would select different PRs on a laptop than in CI. starmap carries `incidentClosed: true` on items *and* on every reference to them (canonical, runnerUp, partition, tracker ref and candidates), omitted when false. a consumer contract has to accept it in all of those positions, not just on items. **CLI only**: the server/GitHub-App path applies no incident windows, because `ServerConfig` has no `incidents` field and the scheduler fetches open items only (#27). NOTE for star-map: same coordinated importer patch as `createdAt` below, since its importer rejects unknown fields
- incident-aware ranking: `prism.config.yaml` accepts an `incidents:` list of `{start, end, reason}` windows. a repository-wide event (visibility flip, bulk close, migration) closes items for reasons unrelated to their quality, and because `selectCanonical()` ranks lifecycle state before score those items sank below genuinely-closed siblings, inverting triage order for exactly the backlog a maintainer needs. PRs closed inside a window now rank as open. `closedAt` is captured from both API paths and stored in `metadata_json` (no schema migration); `incidentClosed` is derived at read time, so correcting a window is a config edit rather than a rescan. window bounds require an explicit UTC offset and are rejected at load if unparseable or inverted. an offset-less timestamp parses in the host timezone and would select different PRs on a laptop than in CI. starmap carries `incidentClosed: true` on items *and* on every reference to them (canonical, runnerUp, partition, tracker ref and candidates), omitted when false. a consumer contract has to accept it in all of those positions, not just on items. the server/GitHub-App path honours windows too, declared per repo in that repo's `config.json` (#27). NOTE for star-map: same coordinated importer patch as `createdAt` below, since its importer rejects unknown fields
- `server/scheduler.ts` now builds stored metadata with the shared `itemMetadata()` instead of its own literal. the hand-rolled copy had already drifted behind the real one, so items scanned by the App path were missing fields the CLI path stored. `server/triage.ts` still hand-rolls its own and is tracked in #27
- NOTE: rows scanned before this release carry no `closedAt`, and a default scan only fetches open items. run `prism scan --state all` to pick them up. this is per-incident, not a one-time backfill: a default scan fetches open items only, so after a bulk close the affected PRs are no longer returned at all and their stored rows keep `state: open` with no `closedAt`. re-scan with `--state all` after each incident, or the window matches nothing

Expand Down
Loading
Loading