Merge pull request #41 from karti-ai/feat/rsi-step3-wasreal-verifier
feat(continual-learning): derive wasRealCollision from git overlap (RSI step 3)
This commit is contained in:
@@ -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<string, GitState>): boolean {
|
||||
const target = comparableBasename(collision.file);
|
||||
if (!target || collision.engineers.length < 2) return false;
|
||||
const byCanon = new Map<string, string[]>();
|
||||
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<string, EngineerContext>();
|
||||
/**
|
||||
@@ -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<void> {
|
||||
|
||||
@@ -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<boolean> {
|
||||
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<string, string[]>();
|
||||
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<void> {
|
||||
// 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' } },
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
+20
-5
@@ -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
|
||||
|
||||
|
||||
@@ -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(),
|
||||
});
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user