Skip to content

fix(reflection): per-section legacy filtering prevents resolved items from being revived (P1+P2) [CLEAN] #595

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rwmjhb merged 4 commits into from
May 4, 2026
+357 −82
Merged

fix(reflection): per-section legacy filtering prevents resolved items from being revived (P1+P2) [CLEAN] #595

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2f5a322
fix(reflection): per-section legacy filtering prevents resolved items…
jlin53882 Apr 12, 2026
bf649d4
fix(reflection): MR1+F4 - proper per-section filtering with normaliza…
jlin53882 Apr 16, 2026
640cfbf
merge: resolve conflict — graft resolved filtering (P1+F4+MR1) into m…
May 1, 2026
af67091
fix: P2 resolved filtering in legacy fallback (buildInvariantCandidat…
jlin53882 May 3, 2026
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
6 changes: 6 additions & 0 deletions src/reflection-item-store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@ export interface ReflectionItemMetadata {
baseWeight: number;
quality: number;
sourceReflectionPath?: string;
/** Unix timestamp when the item was marked resolved. Undefined = unresolved. */
resolvedAt?: number;
/** Agent ID that marked this item resolved. */
resolvedBy?: string;
/** Optional note explaining why the item was resolved. */
resolutionNote?: string;
}

export interface ReflectionItemPayload {
Expand Down
245 changes: 163 additions & 82 deletions src/reflection-store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,45 +231,110 @@ export interface LoadReflectionSlicesParams {
invariantMaxAgeMs?: number;
}

export function loadAgentReflectionSlicesFromEntries(params: LoadReflectionSlicesParams): {
invariants: string[];
derived: string[];
} {
const now = Number.isFinite(params.now) ? Number(params.now) : Date.now();
const deriveMaxAgeMs = Number.isFinite(params.deriveMaxAgeMs)
? Math.max(0, Number(params.deriveMaxAgeMs))
: DEFAULT_REFLECTION_DERIVED_MAX_AGE_MS;
const invariantMaxAgeMs = Number.isFinite(params.invariantMaxAgeMs)
? Math.max(0, Number(params.invariantMaxAgeMs))
: undefined;

const reflectionRows = params.entries
.map((entry) => ({ entry, metadata: parseReflectionMetadata(entry.metadata) }))
.filter(({ metadata }) => isReflectionMetadataType(metadata.type) && isOwnedByAgent(metadata, params.agentId))
.sort((a, b) => b.entry.timestamp - a.entry.timestamp)
.slice(0, 160);

const itemRows = reflectionRows.filter(({ metadata }) => metadata.type === "memory-reflection-item");
const legacyRows = reflectionRows.filter(({ metadata }) => metadata.type === "memory-reflection");

const invariantCandidates = buildInvariantCandidates(itemRows, legacyRows);
const derivedCandidates = buildDerivedCandidates(itemRows, legacyRows, params.agentId);

const invariants = rankReflectionLines(invariantCandidates, {
now,
maxAgeMs: invariantMaxAgeMs,
limit: 8,
});

const derived = rankReflectionLines(derivedCandidates, {
now,
maxAgeMs: deriveMaxAgeMs,
limit: 10,
});

return { invariants, derived };
}

export function loadAgentReflectionSlicesFromEntries(params: LoadReflectionSlicesParams): {
invariants: string[];
derived: string[];
} {
const now = Number.isFinite(params.now) ? Number(params.now) : Date.now();
const deriveMaxAgeMs = Number.isFinite(params.deriveMaxAgeMs)
? Math.max(0, Number(params.deriveMaxAgeMs))
: DEFAULT_REFLECTION_DERIVED_MAX_AGE_MS;
const invariantMaxAgeMs = Number.isFinite(params.invariantMaxAgeMs)
? Math.max(0, Number(params.invariantMaxAgeMs))
: undefined;

const reflectionRows = params.entries
.map((entry) => ({ entry, metadata: parseReflectionMetadata(entry.metadata) }))
.filter(({ metadata }) => isReflectionMetadataType(metadata.type) && isOwnedByAgent(metadata, params.agentId))
.sort((a, b) => b.entry.timestamp - a.entry.timestamp)
.slice(0, 160);

const itemRows = reflectionRows.filter(({ metadata }) => metadata.type === "memory-reflection-item");
const legacyRows = reflectionRows.filter(({ metadata }) => metadata.type === "memory-reflection");

// [P1] Filter out resolved items — passive suppression for #447
// resolvedAt === undefined means unresolved (default)
const unresolvedItemRows = itemRows.filter(({ metadata }) => metadata.resolvedAt === undefined);
const resolvedItemRows = itemRows.filter(({ metadata }) => metadata.resolvedAt !== undefined);

const hasItemRows = itemRows.length > 0;
const hasLegacyRows = legacyRows.length > 0;

// Collect normalized text of resolved items so we can detect whether legacy
// rows are pure duplicates of already-resolved content.
const resolvedInvariantTexts = new Set(
resolvedItemRows
.filter(({ metadata }) => metadata.itemKind === "invariant")
.flatMap(({ entry }) => sanitizeInjectableReflectionLines([entry.text]))
.map((line) => normalizeReflectionLineForAggregation(line))
);
const resolvedDerivedTexts = new Set(
resolvedItemRows
.filter(({ metadata }) => metadata.itemKind === "derived")
.flatMap(({ entry }) => sanitizeInjectableReflectionLines([entry.text]))
.map((line) => normalizeReflectionLineForAggregation(line))
);

// Check whether legacy rows add any content not already covered by resolved items.
// F4 fix: apply same normalization pipeline to both sides
const legacyHasUniqueInvariant = legacyRows.some(({ metadata }) =>
sanitizeInjectableReflectionLines(toStringArray(metadata.invariants)).some(
(line) => !resolvedInvariantTexts.has(normalizeReflectionLineForAggregation(line))
)
);
const legacyHasUniqueDerived = legacyRows.some(({ metadata }) =>
sanitizeInjectableReflectionLines(toStringArray(metadata.derived)).some(
(line) => !resolvedDerivedTexts.has(normalizeReflectionLineForAggregation(line))
)
);

// Suppress when:
// 1) there were item rows, all are resolved, and there are no legacy rows, OR
// 2) there were item rows, all are resolved, legacy rows exist BUT all of their
// content duplicates already-resolved items (prevents legacy fallback from
// reviving just-resolved advice — the P1 bug fixed here).
const shouldSuppress =
hasItemRows &&
unresolvedItemRows.length === 0 &&
(!hasLegacyRows || (!legacyHasUniqueInvariant && !legacyHasUniqueDerived));
if (shouldSuppress) {
return { invariants: [], derived: [] };
}

// [P2] Per-section legacy filtering: only pass legacy rows that have unique
// content for this specific section. Prevents resolved items in section A from being
// revived when section B has unique legacy content (cross-section legacy fallback bug).
// MR1 fix: exclude rows where ALL lines are resolved, not just some.
const invariantLegacyRows = legacyRows.filter(({ metadata }) => {
const lines = sanitizeInjectableReflectionLines(toStringArray(metadata.invariants));
if (lines.length === 0) return false;
// Keep row only if at least one line is NOT resolved
return lines.some((line) => !resolvedInvariantTexts.has(normalizeReflectionLineForAggregation(line)));
});
const derivedLegacyRows = legacyRows.filter(({ metadata }) => {
const lines = sanitizeInjectableReflectionLines(toStringArray(metadata.derived));
if (lines.length === 0) return false;
// Keep row only if at least one line is NOT resolved
return lines.some((line) => !resolvedDerivedTexts.has(normalizeReflectionLineForAggregation(line)));
});

const invariantCandidates = buildInvariantCandidates(unresolvedItemRows, invariantLegacyRows, resolvedInvariantTexts);
const derivedCandidates = buildDerivedCandidates(unresolvedItemRows, derivedLegacyRows, params.agentId, resolvedDerivedTexts);

const invariants = rankReflectionLines(invariantCandidates, {
now,
maxAgeMs: invariantMaxAgeMs,
limit: 8,
});

const derived = rankReflectionLines(derivedCandidates, {
now,
maxAgeMs: deriveMaxAgeMs,
limit: 10,
});

return { invariants, derived };
}
type WeightedLineCandidate = {
line: string;
timestamp: number;
Expand All @@ -280,10 +345,11 @@ type WeightedLineCandidate = {
usedFallback: boolean;
};

function buildInvariantCandidates(
itemRows: Array<{ entry: MemoryEntry; metadata: Record<string, unknown> }>,
legacyRows: Array<{ entry: MemoryEntry; metadata: Record<string, unknown> }>
): WeightedLineCandidate[] {
function buildInvariantCandidates(
itemRows: Array<{ entry: MemoryEntry; metadata: Record<string, unknown> }>,
legacyRows: Array<{ entry: MemoryEntry; metadata: Record<string, unknown> }>,
resolvedTexts: Set<string>
): WeightedLineCandidate[] {
const itemCandidates = itemRows
.filter(({ metadata }) => metadata.itemKind === "invariant")
.flatMap(({ entry, metadata }) => {
Expand All @@ -304,28 +370,40 @@ function buildInvariantCandidates(
}));
});

if (itemCandidates.length > 0) return itemCandidates;

return legacyRows.flatMap(({ entry, metadata }) => {
const defaults = getReflectionItemDecayDefaults("invariant");
const timestamp = metadataTimestamp(metadata, entry.timestamp);
const lines = sanitizeInjectableReflectionLines(toStringArray(metadata.invariants));
return lines.map((line) => ({
line,
timestamp,
midpointDays: defaults.midpointDays,
k: defaults.k,
baseWeight: defaults.baseWeight,
quality: defaults.quality,
usedFallback: metadata.usedFallback === true,
}));
});
}
if (itemCandidates.length > 0) return itemCandidates;

// Legacy fallback: filter out resolved lines (P2 fix).
// resolvedTexts must be the already-normalized Set so line.normalized === setMember
// to pass the resolved filter check.
return legacyRows
.filter(({ metadata }) => {
const lines = sanitizeInjectableReflectionLines(toStringArray(metadata.invariants));
if (lines.length === 0) return false;
return lines.some((line) => !resolvedTexts.has(normalizeReflectionLineForAggregation(line)));
})
.flatMap(({ entry, metadata }) => {
const defaults = getReflectionItemDecayDefaults("invariant");
const timestamp = metadataTimestamp(metadata, entry.timestamp);
const lines = sanitizeInjectableReflectionLines(toStringArray(metadata.invariants));
return lines
.filter((line) => !resolvedTexts.has(normalizeReflectionLineForAggregation(line)))
.map((line) => ({
line,
timestamp,
midpointDays: defaults.midpointDays,
k: defaults.k,
baseWeight: defaults.baseWeight,
quality: defaults.quality,
usedFallback: metadata.usedFallback === true,
}));
});
}

function buildDerivedCandidates(
itemRows: Array<{ entry: MemoryEntry; metadata: Record<string, unknown> }>,
legacyRows: Array<{ entry: MemoryEntry; metadata: Record<string, unknown> }>,
agentId: string
agentId: string,
resolvedTexts: Set<string>
): WeightedLineCandidate[] {
const itemCandidates = itemRows
.filter(({ metadata }) => metadata.itemKind === "derived")
Expand Down Expand Up @@ -363,27 +441,30 @@ function buildDerivedCandidates(
return owner === agentId; // 其他 agent 的 derived → 限本人
})
.flatMap(({ entry, metadata }) => {
const timestamp = metadataTimestamp(metadata, entry.timestamp);
const lines = sanitizeInjectableReflectionLines(toStringArray(metadata.derived));
if (lines.length === 0) return [];

const defaults = {
midpointDays: REFLECTION_DERIVE_LOGISTIC_MIDPOINT_DAYS,
k: REFLECTION_DERIVE_LOGISTIC_K,
baseWeight: resolveLegacyDeriveBaseWeight(metadata),
quality: computeDerivedLineQuality(lines.length),
};

return lines.map((line) => ({
line,
timestamp,
midpointDays: readPositiveNumber(metadata.decayMidpointDays, defaults.midpointDays),
k: readPositiveNumber(metadata.decayK, defaults.k),
baseWeight: readPositiveNumber(metadata.deriveBaseWeight, defaults.baseWeight),
quality: readClampedNumber(metadata.deriveQuality, defaults.quality, 0.2, 1),
usedFallback: metadata.usedFallback === true,
}));
});
const timestamp = metadataTimestamp(metadata, entry.timestamp);
const lines = sanitizeInjectableReflectionLines(toStringArray(metadata.derived));
if (lines.length === 0) return [];

const defaults = {
midpointDays: REFLECTION_DERIVE_LOGISTIC_MIDPOINT_DAYS,
k: REFLECTION_DERIVE_LOGISTIC_K,
baseWeight: resolveLegacyDeriveBaseWeight(metadata),
quality: computeDerivedLineQuality(lines.length),
};

// Legacy fallback: filter out resolved lines (P2 fix).
return lines
.filter((line) => !resolvedTexts.has(normalizeReflectionLineForAggregation(line)))
.map((line) => ({
line,
timestamp,
midpointDays: readPositiveNumber(metadata.decayMidpointDays, defaults.midpointDays),
k: readPositiveNumber(metadata.decayK, defaults.k),
baseWeight: readPositiveNumber(metadata.deriveBaseWeight, defaults.baseWeight),
quality: readClampedNumber(metadata.deriveQuality, defaults.quality, 0.2, 1),
usedFallback: metadata.usedFallback === true,
}));
});
}

function rankReflectionLines(
Expand Down
Loading
Loading