diff --git a/backend/src/agent/podman.ts b/backend/src/agent/podman.ts index c08f13d..b912cb5 100644 --- a/backend/src/agent/podman.ts +++ b/backend/src/agent/podman.ts @@ -9,11 +9,42 @@ import { recordIntervention, updateInterventionStatus, } from '../memory/store.js'; -import { getGitStates } from '../memory/db.js'; +import { getGitStates, type GitState } from '../memory/db.js'; import { recallSimilar } from '../memory/vectors.js'; import { shouldIntervene, preferredAction } from '../memory/policy.js'; import { publishHermesIntervention } from '../action/hermes.js'; +/** Strip a git-status prefix ("M ", "?? ") and reduce a path to its lowercased + * basename — matches comparableFile() in memory/store.ts so keys line up. */ +function comparableBasename(raw?: string): string { + return ( + (raw ?? '') + .trim() + .replace(/^(\?\?|[MADRCU!]{1,2})\s+/, '') + .split(/[\\/]/) + .pop() + ?.toLowerCase() ?? '' + ); +} + +/** Canonicalize an engineer name for case/whitespace-insensitive matching, so + * "Karti" and "karti" resolve to the same engineer's git state. */ +function canonicalName(raw?: string): string { + return (raw ?? '').trim().toLowerCase(); +} + +/** Git ground truth: do ALL involved engineers currently have the collided file + * in their changedFiles? Computed at detection time while git state is fresh. */ +function engineersOverlapOnFile(collision: Collision, gitStates: Map): boolean { + const target = comparableBasename(collision.file); + if (!target || collision.engineers.length < 2) return false; + const byCanon = new Map(); + for (const [name, st] of gitStates) byCanon.set(canonicalName(name), st.changedFiles); + return collision.engineers.every((e) => + (byCanon.get(canonicalName(e)) ?? []).some((f) => comparableBasename(f) === target), + ); +} + export class PodMan { private contexts = new Map(); /** @@ -75,6 +106,12 @@ export class PodMan { if (!current.has(key)) this.activeConflicts.delete(key); } + // Capture git ground-truth overlap now, while engineer_states are fresh, so + // the outcome-time verifier never depends on a stale sidecar or a late click. + for (const collision of collisions) { + collision.gitOverlap = engineersOverlapOnFile(collision, gitStates); + } + for (const collision of collisions) await this.handle(collision); } @@ -85,14 +122,7 @@ export class PodMan { * basename. */ private conflictKey(collision: Collision): string { - return ( - (collision.file ?? '') - .trim() - .replace(/^(\?\?|[MADRCU!]{1,2})\s+/, '') - .split(/[\\/]/) - .pop() - ?.toLowerCase() ?? '' - ); + return comparableBasename(collision.file); } private async handle(collision: Collision): Promise { diff --git a/backend/src/memory/store.ts b/backend/src/memory/store.ts index 8125385..97bff45 100644 --- a/backend/src/memory/store.ts +++ b/backend/src/memory/store.ts @@ -5,7 +5,7 @@ import type { InterventionOutcome, InterventionStatus, } from '@podman/shared'; -import { collections } from './db.js'; +import { collections, getGitStates } from './db.js'; import { enrichCollisionMemory } from './vectors.js'; function comparableFile(raw?: string): string { @@ -84,13 +84,53 @@ export async function updateInterventionStatus( ); } +/** + * Step 3 — derive whether a flagged collision was REAL from git ground truth, + * instead of trusting the client (which historically hardcoded `true`). A + * collision counts as real only if BOTH named engineers currently have the + * collided file in their git `changedFiles`. Conservative: returns false when + * the collision is orphaned/missing or git state is stale/unavailable. + * Verifier supervision per docs/continual-learning/spec.md:98-108, policy.md:35-42. + */ +export async function deriveWasRealCollision(outcome: InterventionOutcome): Promise { + try { + const c = await collections(); + const collision = await c.collisions.findOne({ id: outcome.collisionId }); + if (!collision) return false; + // Prefer the overlap evidence captured at detection time (fresh git state): + // immune to late clicks, stale sidecars, and the engineer_states TTL. + if (typeof collision.gitOverlap === 'boolean') return collision.gitOverlap; + // Fallback for collisions detected before gitOverlap was captured: re-derive + // from latest git state, matching engineers on case/whitespace-canonical names. + if (!Array.isArray(collision.engineers) || collision.engineers.length < 2) return false; + const target = comparableFile(collision.file); + if (!target) return false; + const byCanon = new Map(); + for (const [name, st] of await getGitStates(outcome.podId)) { + byCanon.set(name.trim().toLowerCase(), st.changedFiles); + } + return collision.engineers.every((e) => + (byCanon.get(e.trim().toLowerCase()) ?? []).some((f) => comparableFile(f) === target), + ); + } catch (err) { + console.error(`[memory] wasRealCollision verifier failed: ${(err as Error).message}`); + return false; + } +} + export async function recordOutcome(outcome: InterventionOutcome): Promise { + // Backend is authoritative for wasRealCollision: derive it from git overlap + // rather than trusting the client-supplied value. (RSI Step 3) + const verified: InterventionOutcome = { + ...outcome, + wasRealCollision: await deriveWasRealCollision(outcome), + }; await persist('outcome', async () => { const c = await collections(); - await c.outcomes.insertOne({ ...outcome }); + await c.outcomes.insertOne({ ...verified }); await c.interventions.updateOne( - { id: outcome.interventionId }, - { $set: { status: outcome.accepted ? 'accepted' : 'dismissed' } }, + { id: verified.interventionId }, + { $set: { status: verified.accepted ? 'accepted' : 'dismissed' } }, ); }); } diff --git a/docs/PLAN.md b/docs/PLAN.md index eb1d4a7..1a7893b 100644 --- a/docs/PLAN.md +++ b/docs/PLAN.md @@ -509,11 +509,26 @@ no schema change. Owner: RSI track. Independent of the MongoDB-cleanup handoff. - Spec: `docs/continual-learning/policy.md:62-63` (prefer prior accepted kind), `plan.md:66` (second similar event behaves differently). -Follow-ups (separate rungs, not in this change): Step 3 derive -`wasRealCollision` from git overlap; Step 4-5 `strategy_versions` + -Gemini-proposed `LearningProposal` slice; seed a clean demo pod with a repeated -dismissed signature (the historic 85 dismissals are orphaned — `collisionId` -resolves to no collision — so they cannot drive the demo verifier). +3. **Step 3 - derive `wasRealCollision` from git overlap (backend-authoritative)** ✅ + - Overlap is captured AT detection time as `Collision.gitOverlap` + (`backend/src/agent/podman.ts`), while `engineer_states` are still fresh — + true only if ALL involved engineers have the collided file in their git + `changedFiles`, matched on case/whitespace-canonical names. + - `backend/src/memory/store.ts` `recordOutcome` overrides the client value + with `deriveWasRealCollision()`, which prefers the stored `gitOverlap` + (immune to late clicks / stale sidecars / the 120s TTL) and only falls back + to a live canonical-name re-derivation for pre-existing collisions. + `frontend/.../useInterventions.ts` stops sending hardcoded `true`. + - Restores the (accepted × wasReal) 2×2 the spec assumes; keeps `learned_from` + edges (`graph/live.ts:413`) from being silently zeroed on stage. + - Spec: `docs/continual-learning/spec.md:98-108`, `policy.md:35-42`. + - Hardened per Codex review (name canonicalization + detection-time capture). + +Follow-ups (separate rungs, not in this change): Step 4-5 `strategy_versions` + +Gemini-proposed `LearningProposal` slice; Step 6 durable `owns` write; seed a +clean demo pod with a repeated dismissed signature (the historic dismissals are +orphaned — `collisionId` resolves to no collision — so they cannot drive the +demo verifier). ### P1 - polish the money moment diff --git a/frontend/src/livekit/useInterventions.ts b/frontend/src/livekit/useInterventions.ts index 48af2fe..f6b4816 100644 --- a/frontend/src/livekit/useInterventions.ts +++ b/frontend/src/livekit/useInterventions.ts @@ -66,7 +66,9 @@ export function useInterventions(room: Room | null) { interventionId: active.id, collisionId: active.collisionId, podId: active.podId, - wasRealCollision: true, + // Placeholder only — the backend derives the authoritative value from + // git overlap at outcome time (the client cannot know). (RSI Step 3) + wasRealCollision: false, accepted, recordedAt: new Date().toISOString(), }); diff --git a/shared/src/collision.ts b/shared/src/collision.ts index 84ecbaf..7739b3d 100644 --- a/shared/src/collision.ts +++ b/shared/src/collision.ts @@ -16,6 +16,14 @@ export interface Collision { severity: CollisionSeverity; /** Snapshot of relevant GitHub state at detection time. */ githubState?: GithubStateSnapshot; + /** + * Git ground-truth overlap captured AT detection time, while engineer_states + * are still fresh: true when every involved engineer had `file` in their git + * changedFiles. Read as the authoritative wasRealCollision evidence at outcome + * time, so a late click, a stale sidecar, or the engineer_states freshness TTL + * cannot retroactively zero it out. + */ + gitOverlap?: boolean; detectedAt: string; }