fix(memory): pin proposal target_file to the kind's canonical file - #1563
Closed
anikinsasha wants to merge 1 commit into
Closed
fix(memory): pin proposal target_file to the kind's canonical file#1563anikinsasha wants to merge 1 commit into
anikinsasha wants to merge 1 commit into
Conversation
A memory proposal's target_file is emitted as free text by the reviewer (an LLM).
PROPOSAL_KIND_TO_FILES already maps most proposal kinds to exactly one canonical
file, so the target is fully determined by the kind — but enqueueProposal stored
the model's path verbatim. A hallucinated path (wrong casing, a dropped path
segment) therefore got persisted and then silently mis-filed or failed to apply
("target file missing"), reopening the proposal forever.
Pin target_file to the kind's canonical file via a new pinProposalTargetFile
helper: for a single-file kind the supplied path is ignored and the canonical
one is used; for the 2-file identity kind the supplied path must be one of the
allowed files, else the proposal is rejected (EINVAL_ITEM) rather than stored
with an unresolvable path. Adds MemoryTypes smokeTest coverage for the helper.
Owner
|
Ported into source, thank you. Exactly as designed: single-file kinds pin to the canonical path (a hallucinated reviewer path can no longer mis-file a proposal), identity validates within its two-file set and rejects out-of-set with EINVAL_ITEM. Your five smoke asserts came along; the suite runs 57/57. Context on why this closes without merging: the public repo is generated from the private source tree at release, so PRs are ported rather than merged, with credit at the fix site and in the README. Ships with the next release. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Memory proposals carry
target_fileas free text emitted by the reviewer (an LLM).PROPOSAL_KIND_TO_FILESalready maps most proposal kinds to exactly one canonical file, so the target is fully determined by the kind — yetenqueueProposalpersists the model's path verbatim.When the model hallucinates that path (wrong casing, a dropped path segment), the bad path is stored and then either silently mis-files the edit or fails to apply with
target file missing, reopening the proposal on every review with no resolution.Fix
Pin
target_fileto the kind's canonical file at enqueue time via a new pure helperpinProposalTargetFile(kind, suppliedPath)inMemoryTypes.ts:identity→ PRINCIPAL_IDENTITY / DA_IDENTITY): can't be pinned, so the supplied path must be one of the two allowed files; otherwise the proposal is rejected withEINVAL_ITEMrather than stored with an unresolvable path.enqueueProposalcalls the helper and rejects an out-of-set identity path.Testing
bun MemoryTypes.ts test— added 5 assertions covering the helper (single-file pin ignores a hallucinated path; identity in-set kept; identity out-of-set → null). Full smoke suite passes (57/57).Notes
Backward-compatible: the reverse-path inference default and the existing apply/allowlist behavior are unchanged; this only stops a hallucinated path from being persisted at the source.