RFC: Patchset Version Tracking

[derekbarbosa]

Track version chains for patch series from both mailing lists (NNTP/lore) and forge webhooks (GitHub/GitLab).

When a maintainer posts [PATCH v2 0/5] or pushes new commits to an MR, Sashiko recognizes it as a new version, links it to the prior version, and injects prior review findings into the new review prompt so the AI reviewer can check whether previous issues were addressed.

Schema
Three new columns on the patchsets table: version INTEGER DEFAULT 1, previous_version_id INTEGER REFERENCES patchsets(id), and commit_range TEXT. All migrations are idempotent via try_add_column. Existing patchsets default to version 1 with no chain linkage. A partial index on version > 1 enables efficient lookups.

Version detection
The existing parse_subject_version() function (handles [PATCH v2], [PATCH V3 1/2], [PATCHv5], [PATCH RFC v2 8/8], multi-digit versions) is used to extract version numbers. The parsed version is persisted on patchset creation.

Signal-based chain linkage
find_previous_version() links v2 to v1 using signals, not time windows.
Priority:
(1) same thread_id with version = current - 1
(2) same author + stripped subject match. strip_subject_version() removes version tags from subjects for cross-version comparison.
If no signal confirms a match, the patchset stands alone -- no false positives.

Cross-version merge prevention
Patchsets with different explicit versions are never merged, even in the same thread. Implicit (None) versions still merge with explicit versions in the same thread, preserving the existing behavior where cover letters have [PATCH v6 00/33] but individual patches have [PATCH 01/33]. Patch reassignment between version-tracked patchsets is blocked in create_patch().

Forge versioning
find_patchset_by_mr_number() looks up the latest version for an MR/PR number. When the same MR fires with a different commit_range, a new patchset is created with the version incremented and previous_version_id linking to the prior version. When commit_range matches (metadata-only changes like Draft-to-Ready), only the title and URL are updated without creating a new version.

Prior review context injection
get_previous_version_findings() follows the previous_version_id chain one hop and returns non-preexisting findings sorted by severity. format_previous_findings() produces a structured context string capped at an approximate token limit (2000 tokens, severity-prioritized truncation). The context flows through the reviewer, worker/prompts, and review binary pipeline via the previous_context field on ReviewInput.

API and UI
All API responses include version and previous_version_id fields. The web UI displays a yellow version badge ("v2", "v3") next to patchset names for versions > 1 with light/dark mode support.

Input validation
sanitize_message_id() strips null bytes and control characters from message-IDs as defense in depth.

Depends on: #310 (webhook-input-hardening) -- this adds version chain logic to forge_webhook that trusts webhook payload data (MR numbers, commit ranges) for version decisions. For git-forges, input-hardening's HMAC signature verification and input validation must be in place first to ensure that payload data is authenticated and well-formed before it is used to create version chains or inject prior review findings into AI prompts. This also builds on #310 's revised ForgeProvider trait signature and access control flow.

[rgushchin]

Thanks, Derek!

One high-level comment: let's drop for now the part which injects prior context into the review context, it's subtle and requires more testing and thinking. It's easy to bias LLMs and make them less good at spotting new issues.

The rest overall looks good to me, thank you for working on it. I'll review it a bit more carefully soon.

[derekbarbosa]

Hi Roman,

One high-level comment: let's drop for now the part which injects prior context into the review context, it's subtle and requires more testing and thinking. It's easy to bias LLMs and make them less good at spotting new issues.

if it helps, we've been seeing it work pretty well at RH. I can try and collect some data by logging into our prod instance.

but if it ends up blocking this MR, i'm ok dropping it and hashing out specifics later :)

[rgushchin]

Yes, let's drop it for now so we can separate infra changes from the review quality changes. Thanks!

[derekbarbosa]

separate infra changes from the review quality changes

good point. I'll be more discerning of this going forward.
are you OK with lumping in the "hardening" changes in this one? or keep them as separate PRs?

[rgushchin]

I'd keep them separate, but not a hard preference. Hardening overall looks good to me. I'll try to merge it soon.

[derekbarbosa]

No rush, I want to finish a few more tests. I'll take it out of draft sometime tomorrow.

[derekbarbosa]

v2 -> v3:

• Wire sanitize_message_id() at extraction source in parse_email
  and in create_patch. Closes gap where patches.message_id was
  unsanitized while patchsets.cover_letter_message_id was sanitized.
• Read persisted version column from DB instead of re-parsing
  subject via parse_subject_version() in both cover-letter-ID and
  fuzzy matching query paths. Removes redundant string processing.
• Extract link_previous_version() method on Database. Removes raw
  SQL from the API layer in forge_webhook; used by both the forge
  path and create_patchset for version chain linkage.
• Consolidate cross-version patch reassignment guard from two
  separate version queries into a single self-join. Add tracing
  log when reassignment is blocked.
• Mark prior review context injection as deferred in design doc.

[derekbarbosa]

gentle ping for review, but considering the upcoming OOO, may want to hold off on merging this :)

[rgushchin]

Overall LGTM, but as you said, I'll postpone merging it for a couple of weeks. Sorry for the delay.