feat(rules): the edit history is visible in the slide-over (#3243, milestone 323 step 4)
CI & Build / Python lint (push) Successful in 5s
CI & Build / Plugin hooks (push) Successful in 9s
CI & Build / TypeScript typecheck (push) Successful in 34s
CI & Build / integration (push) Successful in 35s
CI & Build / Python tests (push) Successful in 1m20s
CI & Build / Build & push image (push) Successful in 40s

Rule 27: a history nobody can read is not shipped. `RuleHistoryPanel.vue`
sits below the fields in `RuleEditorSlideOver`, where a rule is read in full
— not on the list row, where a history entry point would compete with the
row's job.

REUSE, DECIDED FIELD BY FIELD RATHER THAN ALL AT ONCE.

DiffView.vue is reused unchanged: it takes DiffLine[] and nothing
note-shaped.

HistoryPanel.vue is NOT, and its props are the reason — noteId +
currentBody, a NoteVersion carrying tags and pin columns, a fetch of
/api/notes/…, a restore emit, pin/unpin buttons. Rules have no tags, no
pins, and deliberately no restore, and a rule's text is EIGHT fields rather
than one body, which changes the reader's question from "what changed" to
"which fields moved". Recorded here rather than forked silently, per #3207.

THE FORK THAT WAS ALREADY THERE. The LCS walk existed three times —
privately in useAssist.ts, and again inside HistoryPanel.vue and
VersionHistorySection.vue — character-identical apart from quote style,
because computeDiff was never exported. Rather than add a fourth copy, it
moves to utils/diff.ts and the three become imports; the extraction was
verified equivalent to all three before anything was deleted. DiffLine is
re-exported from useAssist so its existing importers are untouched.

WHAT A ROW SHOWS: when, and which fields moved. A version holds the text the
edit REPLACED, so the edit is the step from a row to the next NEWER state —
the row above it, or, for the newest row, the rule as it stands now.
Comparing against the row below would attribute every change to the wrong
edit. A field nobody has fetched yet reads as neither changed nor unchanged.

An edit that touched verify_with is badged "check reset", because that edit
silently cleared verified_at (milestone 312) and put the rule back at the
top of the staleness sweep — a moment visible nowhere else.

The badge is a 12% color-mix TINT, not solid `--fs-warning`.
`--fs-warning-fg` is defined in theme.css as "warning TEXT on a warning
tint", so painting it over the solid token is exactly the same-hue contrast
failure #3141 records. Every var() the component references resolves against
theme.css, checked before pushing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2026-08-30 12:52:07 -04:00
co-authored by Claude Opus 5
parent 0704988528
commit a8b2040216
7 changed files with 388 additions and 68 deletions
+43
View File
@@ -228,6 +228,49 @@ export async function unrelateRules(relationId: number): Promise<void> {
return apiDelete(`/api/rule-relations/${relationId}`);
}
/**
* One entry in a rule's edit history.
*
* Each entry holds the text the edit REPLACED, not the text it introduced —
* so the newest entry is what the rule said before its most recent change,
* and what that change produced is the rule as it stands now. Read the other
* way round, every diff comes out backwards.
*
* The listing form omits the long fields; open one to get them.
*/
export interface RuleVersion {
id: number;
rule_id: number;
/** Who made the edit. Null when that account has since been deleted. */
user_id: number | null;
title: string;
created_at: string;
statement?: string;
why?: string;
how_to_apply?: string;
when_to_apply?: string;
tier?: string;
verify_with?: string;
expires_when?: string;
}
export async function listRuleVersions(ruleId: number): Promise<RuleVersion[]> {
const data = await apiGet<{ versions: RuleVersion[] }>(
`/api/rules/${ruleId}/versions`,
);
return data.versions;
}
export async function getRuleVersion(
ruleId: number, versionId: number,
): Promise<RuleVersion> {
return apiGet<RuleVersion>(`/api/rules/${ruleId}/versions/${versionId}`);
}
// No restoreRuleVersion, deliberately (milestone 323). Putting an old wording
// back goes through updateRule, which snapshots what it replaces — so the
// undo stays visible in the history like any other edit.
export async function deleteRule(id: number): Promise<void> {
return apiDelete(`/api/rules/${id}`);
}
+2 -22
View File
@@ -2,7 +2,7 @@
import { ref, computed, onMounted } from "vue";
import { apiGet, pinNoteVersion, unpinNoteVersion } from "@/api/client";
import DiffView from "@/components/DiffView.vue";
import type { DiffLine } from "@/composables/useAssist";
import { computeDiff, type DiffLine } from "@/utils/diff";
import { fmtStamp } from "@/utils/dateFormat";
interface NoteVersion {
@@ -33,28 +33,8 @@ const loadingDetail = ref(false);
const diff = computed<DiffLine[]>(() => {
if (!selectedVersion.value?.body) return [];
const a = props.currentBody;
const b = selectedVersion.value.body;
const aLines = a.split('\n');
const bLines = b.split('\n');
const m = aLines.length, n = bLines.length;
const dp: number[][] = Array.from({ length: m + 1 }, () => new Array(n + 1).fill(0));
for (let i = m - 1; i >= 0; i--)
for (let j = n - 1; j >= 0; j--)
dp[i][j] = aLines[i] === bLines[j]
? dp[i+1][j+1] + 1
: Math.max(dp[i+1][j], dp[i][j+1]);
const result: DiffLine[] = [];
let i = 0, j = 0;
while (i < m && j < n) {
if (aLines[i] === bLines[j]) { result.push({ type: 'equal', text: aLines[i++] }); j++; }
else if (dp[i+1][j] >= dp[i][j+1]) result.push({ type: 'delete', text: aLines[i++] });
else result.push({ type: 'insert', text: bLines[j++] });
}
while (i < m) result.push({ type: 'delete', text: aLines[i++] });
while (j < n) result.push({ type: 'insert', text: bLines[j++] });
return result;
return computeDiff(props.currentBody, selectedVersion.value.body);
});
async function loadVersions() {
@@ -2,7 +2,7 @@
import { ref, computed } from "vue";
import { apiGet } from "@/api/client";
import DiffView from "@/components/DiffView.vue";
import type { DiffLine } from "@/composables/useAssist";
import { computeDiff, type DiffLine } from "@/utils/diff";
interface NoteVersion {
id: number;
@@ -31,25 +31,7 @@ const loadingDetail = ref(false);
const diff = computed<DiffLine[]>(() => {
if (!selectedVersion.value?.body) return [];
const aLines = props.currentBody.split("\n");
const bLines = selectedVersion.value.body.split("\n");
const m = aLines.length, n = bLines.length;
const dp: number[][] = Array.from({ length: m + 1 }, () => new Array(n + 1).fill(0));
for (let i = m - 1; i >= 0; i--)
for (let j = n - 1; j >= 0; j--)
dp[i][j] = aLines[i] === bLines[j]
? dp[i + 1][j + 1] + 1
: Math.max(dp[i + 1][j], dp[i][j + 1]);
const result: DiffLine[] = [];
let i = 0, j = 0;
while (i < m && j < n) {
if (aLines[i] === bLines[j]) { result.push({ type: "equal", text: aLines[i++] }); j++; }
else if (dp[i + 1][j] >= dp[i][j + 1]) result.push({ type: "delete", text: aLines[i++] });
else result.push({ type: "insert", text: bLines[j++] });
}
while (i < m) result.push({ type: "delete", text: aLines[i++] });
while (j < n) result.push({ type: "insert", text: bLines[j++] });
return result;
return computeDiff(props.currentBody, selectedVersion.value.body);
});
function formatDate(iso: string): string {
@@ -3,6 +3,7 @@ import { computed, ref, watch, onMounted } from "vue";
import { useRulebooksStore } from "@/stores/rulebooks";
import { useCanonicalSystemsStore } from "@/stores/canonicalSystems";
import type { RuleTier } from "@/api/rulebooks";
import RuleHistoryPanel from "@/components/rules/RuleHistoryPanel.vue";
const props = defineProps<{ ruleId: number | null; topicId: number | null }>();
const emit = defineEmits<{ close: [] }>();
@@ -256,6 +257,17 @@ watch(() => props.ruleId, load);
How to apply
<textarea v-model="howToApply" rows="4" placeholder="When / where this kicks in." />
</label>
<!-- Only on an existing rule: a rule being created has no past, and an
"Edit history — none" line on a blank form reads as a broken panel.
Keyed on ruleId so switching rules reloads rather than showing the
previous rule's history under the new one's text. -->
<RuleHistoryPanel
v-if="!isCreating && ruleId !== null"
:key="ruleId"
:rule-id="ruleId"
:current="store.currentRule"
/>
</aside>
</div>
</template>
@@ -0,0 +1,274 @@
<script setup lang="ts">
/**
* What a rule USED TO SAY — inside the slide-over, where a rule is read in
* full. Not on the list row: a history entry point there would compete with
* the row's actual job.
*
* A SIBLING OF HistoryPanel.vue, NOT A REUSE OF IT, and the reason is in its
* props: `noteId` + `currentBody`, a `NoteVersion` carrying tags and pin
* columns, a fetch of /api/notes/…, a `restore` emit, and pin/unpin buttons.
* Every one of those is note-shaped. Rules have no tags, no pins, and
* deliberately no restore, and a rule's text is EIGHT fields rather than one
* body — which changes the central question from "what changed" to "which
* fields moved".
*
* What was genuinely shared is shared: DiffView.vue takes DiffLine[] and
* nothing note-shaped, and the LCS walk now lives in utils/diff.ts, which
* this file uses rather than copying a fourth time (#3207).
*/
import { computed, onMounted, ref, watch } from "vue";
import DiffView from "@/components/DiffView.vue";
import { computeDiff } from "@/utils/diff";
import {
listRuleVersions, getRuleVersion, type Rule, type RuleVersion,
} from "@/api/rulebooks";
import { useToastStore } from "@/stores/toast";
const props = defineProps<{ ruleId: number; current: Rule | null }>();
const toast = useToastStore();
const versions = ref<RuleVersion[]>([]);
const selected = ref<RuleVersion | null>(null);
const expanded = ref(false);
const loading = ref(false);
const loadingDetail = ref(false);
// The eight TEXT fields a version carries, in the order the editor shows
// them. Narrowed to its own type rather than `keyof RuleVersion`, which would
// also admit id/rule_id/user_id/created_at — none of which is text a reader
// compares, and all of which would widen every lookup below to `number`.
// Labels rather than column names: a reader is deciding whether to open a
// row, and "How to apply" reads where "how_to_apply" has to be decoded.
type TextField =
| "title" | "statement" | "when_to_apply" | "tier"
| "why" | "how_to_apply" | "verify_with" | "expires_when";
const FIELDS: Array<[TextField, string]> = [
["title", "Title"],
["statement", "Statement"],
["when_to_apply", "When to apply"],
["tier", "Tier"],
["why", "Why"],
["how_to_apply", "How to apply"],
["verify_with", "Check"],
["expires_when", "Ends when"],
];
/**
* Which fields this edit moved.
*
* A version holds the text the edit REPLACED, so the edit is the step from
* this row to the NEXT NEWER state — the version above it in the list, or,
* for the newest row, the rule as it stands now. Comparing against the row
* below instead would attribute every change to the wrong edit.
*/
function changedFields(index: number): string[] {
const before = versions.value[index];
// `Rule` carries all eight as required strings; a RuleVersion carries them
// only once opened, which is what the undefined check below is about.
const after: Pick<Rule, TextField> | RuleVersion | null =
index === 0 ? props.current : versions.value[index - 1] ?? null;
if (!before || !after) return [];
return FIELDS
.filter(([key]) => {
// A listing row carries only the title; the rest arrive when opened.
// Undefined means NOT LOADED, which is not the same as unchanged — so a
// field nobody has fetched is claimed as neither.
const a = before[key];
const b = after[key];
if (a === undefined || b === undefined) return false;
return (a ?? "") !== (b ?? "");
})
.map(([, label]) => label);
}
/** True when this edit rewrote or removed the rule's check.
*
* Worth its own marker because editing `verify_with` silently drops
* `verified_at` (milestone 312) — the moment a rule re-entered the staleness
* sweep. That happens nowhere a reader can see it, and this row is the only
* surface that can say when it happened. */
function checkChanged(index: number): boolean {
return changedFields(index).includes("Check");
}
const diff = computed(() => {
if (!selected.value || selected.value.statement === undefined) return [];
const now = props.current?.statement ?? "";
return computeDiff(now, selected.value.statement);
});
function stamp(iso: string): string {
return iso.slice(0, 10);
}
async function load() {
loading.value = true;
try {
versions.value = await listRuleVersions(props.ruleId);
} catch {
toast.show("Could not load this rule's history", "error");
} finally {
loading.value = false;
}
}
async function open(v: RuleVersion) {
if (selected.value?.id === v.id) {
selected.value = null;
return;
}
loadingDetail.value = true;
try {
const full = await getRuleVersion(props.ruleId, v.id);
// Merged back into the list so `changedFields` can compare against real
// text once a neighbour has been opened, instead of staying blind.
const at = versions.value.findIndex((x) => x.id === v.id);
if (at >= 0) versions.value[at] = { ...versions.value[at], ...full };
selected.value = versions.value[at] ?? full;
} catch {
toast.show("Could not open that version", "error");
} finally {
loadingDetail.value = false;
}
}
onMounted(load);
watch(() => props.ruleId, () => { selected.value = null; load(); });
</script>
<template>
<section class="history">
<button class="toggle" :aria-expanded="expanded" @click="expanded = !expanded">
<span>Edit history</span>
<span class="count">{{ versions.length || "none" }}</span>
</button>
<div v-if="expanded" class="body">
<p v-if="loading" class="state">Loading</p>
<!-- Never reworded is the ordinary case, and must not read as a fault. -->
<p v-else-if="!versions.length" class="state empty">
This rule has never been reworded. Nothing was recorded before the history
existed, so an older rule starts empty too.
</p>
<template v-else>
<p class="lede">
Each entry is what the rule said <em>before</em> that edit. The wording it
was changed to is the rule as it stands above.
</p>
<ol class="rows">
<li v-for="(v, i) in versions" :key="v.id" class="row">
<button
class="row-head"
:class="{ open: selected?.id === v.id }"
@click="open(v)"
>
<span class="when">{{ stamp(v.created_at) }}</span>
<span class="fields">
{{ changedFields(i).join(", ") || "opened to compare" }}
</span>
<span v-if="checkChanged(i)" class="check-moved">check reset</span>
</button>
<div v-if="selected?.id === v.id" class="detail">
<p v-if="loadingDetail" class="state">Loading</p>
<template v-else>
<p v-if="checkChanged(i)" class="warn">
This edit changed the rule's check, which cleared its verification
stamp the rule went back to the top of the staleness sweep here.
</p>
<dl class="fields-list">
<template v-for="[key, label] in FIELDS" :key="key">
<template v-if="key !== 'statement' && v[key]">
<dt>{{ label }}</dt>
<dd>{{ v[key] }}</dd>
</template>
</template>
</dl>
<h4>Statement</h4>
<DiffView v-if="diff.length" :diff="diff" />
<p v-else class="state">The statement did not change in this edit.</p>
</template>
</div>
</li>
</ol>
</template>
</div>
</section>
</template>
<style scoped>
.history { border-top: 1px solid var(--fs-border-color); padding-top: var(--fs-space-3); }
.toggle {
display: flex; align-items: center; gap: var(--fs-space-2); width: 100%;
background: none; border: none; padding: 0; cursor: pointer;
font: inherit; font-size: var(--fs-size-body-sm); color: var(--fs-text-secondary);
}
.toggle:hover { color: var(--fs-text-primary); }
.count {
margin-left: auto; font-size: var(--fs-size-tiny); color: var(--fs-text-tertiary);
font-variant-numeric: tabular-nums;
}
.body { margin-top: var(--fs-space-3); display: flex; flex-direction: column; gap: var(--fs-space-3); }
.state { margin: 0; font-size: var(--fs-size-body-sm); color: var(--fs-text-secondary); }
.state.empty { color: var(--fs-text-tertiary); }
.lede {
margin: 0; max-width: 62ch; font-size: var(--fs-size-tiny);
color: var(--fs-text-tertiary); line-height: var(--fs-leading-body);
}
.rows { list-style: none; margin: 0; padding: 0; display: flex; flex-direction: column; gap: var(--fs-space-2); }
.row { background: var(--fs-surface-raised); border-radius: var(--fs-radius-md); }
.row-head {
display: flex; align-items: baseline; gap: var(--fs-space-3); width: 100%;
background: none; border: none; cursor: pointer; text-align: left;
padding: var(--fs-space-2) var(--fs-space-3);
font: inherit; font-size: var(--fs-size-body-sm); color: var(--fs-text-primary);
}
.row-head:hover { background: var(--fs-surface-hover); border-radius: var(--fs-radius-md); }
.when {
font-variant-numeric: tabular-nums; color: var(--fs-text-secondary);
font-size: var(--fs-size-tiny);
}
.fields { color: var(--fs-text-primary); min-width: 0; overflow-wrap: anywhere; }
/* A TINT, not the solid token. `--fs-warning-fg` is defined as "warning text
ON A WARNING TINT" — painting it over solid `--fs-warning` is the same-hue
contrast failure #3141 records. The 12% mix is how theme.css builds its own
`-bg` pairs, and it keeps the value a resolvable var() rather than a raw hex
that check_design_tokens.py cannot see at all. */
.check-moved {
margin-left: auto; flex: none;
background: color-mix(in srgb, var(--fs-warning) 12%, transparent);
color: var(--fs-warning-fg);
border-radius: var(--fs-radius-pill);
padding: 0.1rem 0.5rem;
font-size: var(--fs-size-tiny); letter-spacing: var(--fs-tracking-tiny);
}
.detail {
padding: 0 var(--fs-space-3) var(--fs-space-3);
display: flex; flex-direction: column; gap: var(--fs-space-2);
}
.warn {
margin: 0; font-size: var(--fs-size-tiny); line-height: var(--fs-leading-body);
color: var(--fs-warning-fg);
background: color-mix(in srgb, var(--fs-warning) 12%, transparent);
border-radius: var(--fs-radius-sm); padding: var(--fs-space-2);
}
.fields-list { display: grid; grid-template-columns: auto 1fr; gap: 0.15rem var(--fs-space-3); margin: 0; }
.fields-list dt {
font-size: var(--fs-size-tiny); text-transform: uppercase;
letter-spacing: var(--fs-tracking-tiny); color: var(--fs-text-tertiary);
}
.fields-list dd {
margin: 0; font-size: var(--fs-size-body-sm);
color: var(--fs-text-primary); min-width: 0; overflow-wrap: anywhere;
}
h4 { margin: var(--fs-space-2) 0 0; font-size: var(--fs-size-tiny); color: var(--fs-text-tertiary); }
</style>
+5 -26
View File
@@ -1,4 +1,5 @@
import { ref, computed, watch, type Ref } from "vue";
import { computeDiff, type DiffLine } from "@/utils/diff";
import { apiPost, apiPut, apiDelete, apiSSEStream, type SSEStreamHandle } from "@/api/client";
import { useToastStore } from "@/stores/toast";
import {
@@ -9,17 +10,16 @@ import {
export type AssistState = "idle" | "streaming" | "review";
export type ScopeMode = "document" | "section";
// Re-exported: this composable was where DiffLine lived before the diff
// moved to a shared util, and every consumer still imports the type from here.
export type { DiffLine };
export interface AssistTarget {
text: string;
startOffset: number;
endOffset: number;
}
export interface DiffLine {
type: 'equal' | 'delete' | 'insert';
text: string;
}
export interface NoteDraft {
id: number;
note_id: number;
@@ -31,27 +31,6 @@ export interface NoteDraft {
updated_at: string;
}
function computeDiff(a: string, b: string): DiffLine[] {
const aLines = a.split('\n');
const bLines = b.split('\n');
const m = aLines.length, n = bLines.length;
const dp: number[][] = Array.from({ length: m + 1 }, () => new Array(n + 1).fill(0));
for (let i = m - 1; i >= 0; i--)
for (let j = n - 1; j >= 0; j--)
dp[i][j] = aLines[i] === bLines[j]
? dp[i+1][j+1] + 1
: Math.max(dp[i+1][j], dp[i][j+1]);
const result: DiffLine[] = [];
let i = 0, j = 0;
while (i < m && j < n) {
if (aLines[i] === bLines[j]) { result.push({ type: 'equal', text: aLines[i++] }); j++; }
else if (dp[i+1][j] >= dp[i][j+1]) result.push({ type: 'delete', text: aLines[i++] });
else result.push({ type: 'insert', text: bLines[j++] });
}
while (i < m) result.push({ type: 'delete', text: aLines[i++] });
while (j < n) result.push({ type: 'insert', text: bLines[j++] });
return result;
}
export function useAssist(body: Ref<string>, noteId?: Ref<number | null>, projectId?: Ref<number | null>) {
const toast = useToastStore();
+50
View File
@@ -0,0 +1,50 @@
/**
* Line diff — one copy, for every surface that shows what changed.
*
* WHY THIS FILE EXISTS. The same LCS walk was written out three times:
* privately in `useAssist.ts`, and again inside `HistoryPanel.vue` and
* `VersionHistorySection.vue`. The three were character-identical apart from
* quote style — nobody had diverged them on purpose, they were simply copied
* because `computeDiff` was never exported. Milestone 323 needed a fourth
* consumer (a rule's edit history), and a fourth copy is the cost #3207
* records: a fix or an improvement now has to be found in N places by someone
* who does not know N.
*/
export interface DiffLine {
type: "equal" | "delete" | "insert";
text: string;
}
/**
* Diff `a` against `b`, line by line.
*
* `delete` lines come from `a`, `insert` lines from `b` — so the caller
* decides which side reads as "before" by which argument it passes. Every
* caller here passes the CURRENT text as `a` and the older text as `b`, so a
* deletion is what the old version had and an insertion is what replaced it.
*
* O(m·n) in time and memory: fine for a note or a rule, and deliberately not
* generalised further, since nothing here diffs a file of thousands of lines.
*/
export function computeDiff(a: string, b: string): DiffLine[] {
const aLines = a.split("\n");
const bLines = b.split("\n");
const m = aLines.length, n = bLines.length;
const dp: number[][] = Array.from({ length: m + 1 }, () => new Array(n + 1).fill(0));
for (let i = m - 1; i >= 0; i--)
for (let j = n - 1; j >= 0; j--)
dp[i][j] = aLines[i] === bLines[j]
? dp[i + 1][j + 1] + 1
: Math.max(dp[i + 1][j], dp[i][j + 1]);
const result: DiffLine[] = [];
let i = 0, j = 0;
while (i < m && j < n) {
if (aLines[i] === bLines[j]) { result.push({ type: "equal", text: aLines[i++] }); j++; }
else if (dp[i + 1][j] >= dp[i][j + 1]) result.push({ type: "delete", text: aLines[i++] });
else result.push({ type: "insert", text: bLines[j++] });
}
while (i < m) result.push({ type: "delete", text: aLines[i++] });
while (j < n) result.push({ type: "insert", text: bLines[j++] });
return result;
}