Repository navigation
fix: resolve five reproducible triage issues - #1048
Conversation
Greptile SummaryThe update simulator adds a safe autoplay preview for update states, but Confidence Score: 3/5Do not merge until self-dev forwards the update-preview setting and memory retrieval can fall back to lexical matching after a focused embedding failure. Two independent non-security functional failures remain: one prevents an accepted self-dev option from taking effect, and one suppresses relevant memory recall during an embedding-provider failure. Files Needing Attention: src/cli/dispatch.rs, src/cli/selfdev.rs, crates/jcode-base/src/memory_agent.rs
 T-Rex LogsWhat T-Rex did
|
![greptile-apps[bot]](/web-api/iframe-proxy?url=https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Addressed the completed Greptile review in commit --- — Jcode agent (automated triage), on behalf of @1jehuang |
| Ok(Err(e)) => { | ||
| crate::logging::event_rate_limited( | ||
| crate::logging::LogLevel::Info, | ||
| "memory_agent_retrieval_embedding_failed", | ||
| std::time::Duration::from_secs(60), | ||
| "MEMORY_RETRIEVAL_EMBEDDING_FAILED", | ||
| vec![ | ||
| ("session_id", session_id.to_string()), | ||
| ("error", e.to_string()), | ||
| ("fallback", "skip_memory_relevance".to_string()), | ||
| ], | ||
| ); | ||
| memory::set_state(MemoryState::Idle); | ||
| return Ok(()); | ||
| } |
There was a problem hiding this comment.
When the broad-context embedding succeeds but the focused-query embedding is rate-limited or otherwise fails, this branch returns before find_similar_hybrid runs. That skips BM25 as well as dense retrieval, so a relevant memory can be missed even when the focused query alone is sufficient to find it. Fall back to lexical-only retrieval rather than returning before the hybrid search.
Artifacts
Focused embedding BM25 harness source
- The authored in-crate harness seeds a BM25-specific memory and exercises successful and 429 focused-embedding paths through `MemoryAgent::process_context`, proving the compared behavior.
Focused embedding success control
- Ran the real memory-agent control with two HTTP 200 embedding responses and observed `BM25 pending result=true`, showing hybrid BM25 surfaces the memory when focused embedding succeeds.
Focused embedding 429 reproduction
- Ran the real memory-agent reproduction with broad embedding HTTP 200 and focused embedding HTTP 429 and observed `BM25 pending result=false`, proving the early return skips hybrid BM25.
- Ran Git verification after the tests and confirmed the temporary modification to `memory_agent_tests.rs` was restored with exit code 0.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/jcode-base/src/memory_agent.rs
Line: 658-672
Comment:
**Focused embedding failure skips lexical retrieval**
When the broad-context embedding succeeds but the focused-query embedding is rate-limited or otherwise fails, this branch returns before `find_similar_hybrid` runs. That skips BM25 as well as dense retrieval, so a relevant memory can be missed even when the focused query alone is sufficient to find it. Fall back to lexical-only retrieval rather than returning before the hybrid search.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
| Ok(Err(e)) => { | ||
| crate::logging::event_rate_limited( | ||
| crate::logging::LogLevel::Info, | ||
| "memory_agent_retrieval_embedding_failed", | ||
| std::time::Duration::from_secs(60), | ||
| "MEMORY_RETRIEVAL_EMBEDDING_FAILED", | ||
| vec![ | ||
| ("session_id", session_id.to_string()), | ||
| ("error", e.to_string()), | ||
| ("fallback", "skip_memory_relevance".to_string()), | ||
| ], | ||
| ); | ||
| memory::set_state(MemoryState::Idle); | ||
| return Ok(()); | ||
| } |
There was a problem hiding this comment.
When embedding the focused retrieval text fails, this branch returns before find_similar_hybrid runs. That prevents its BM25 pass from finding memories that match the focused text even though lexical retrieval does not require a focused embedding. Fall back to lexical-only retrieval rather than returning on this error.
Artifacts
Focused embedding-failure harness source
- This executable Rust harness makes the broad embedding succeed and the focused embedding fail while seeding a matching BM25 sentinel, demonstrating the claimed branch.
Focused embedding-failure harness output
- This captured cargo-test output records the two-request scenario, absent pending memory, one passing test, and exit code 0, confirming hybrid/BM25 retrieval was skipped.
- This captured cleanup command output records removal of the temporary repository integration test so only the retained evidence artifacts remain.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/jcode-base/src/memory_agent.rs
Line: 658-672
Comment:
**Focused embedding failure disables lexical retrieval**
When embedding the focused retrieval text fails, this branch returns before `find_similar_hybrid` runs. That prevents its BM25 pass from finding memories that match the focused text even though lexical retrieval does not require a focused embedding. Fall back to lexical-only retrieval rather than returning on this error.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
| Ok(Err(e)) => { | ||
| crate::logging::event_rate_limited( | ||
| crate::logging::LogLevel::Info, | ||
| "memory_agent_retrieval_embedding_failed", | ||
| std::time::Duration::from_secs(60), | ||
| "MEMORY_RETRIEVAL_EMBEDDING_FAILED", | ||
| vec![ | ||
| ("session_id", session_id.to_string()), | ||
| ("error", e.to_string()), | ||
| ("fallback", "skip_memory_relevance".to_string()), | ||
| ], | ||
| ); | ||
| memory::set_state(MemoryState::Idle); | ||
| return Ok(()); | ||
| } | ||
| Err(e) => { | ||
| crate::logging::info(&format!("Retrieval embedding task failed: {}", e)); | ||
| memory::set_state(MemoryState::Idle); | ||
| return Ok(()); | ||
| } |
There was a problem hiding this comment.
When the focused retrieval embedding fails, this branch sets the memory state to idle and returns before find_similar_hybrid executes. That prevents the hybrid search's BM25 pass from finding relevant memories even though lexical retrieval does not require a focused embedding. Fall back to lexical-only retrieval rather than returning on this error.
Artifacts
Review-authored focused embedding failure harness source
- This Rust harness runs the memory agent against a local endpoint that succeeds for context embedding and fails for focused embedding, with the takeaway that it directly exercises the claimed early-return path.
Review-authored harness manifest
- This manifest provides the isolated dependencies used to compile and run the review harness, with the takeaway that the runtime reproduction is independently runnable.
Current memory-agent control-flow capture
- This command capture shows the current early return at lines 658-671 before `find_similar_hybrid` at lines 701-709, with the takeaway that the source control flow precludes BM25 fallback on focused embedding failure.
Completed focused embedding failure harness output
- This successful runtime capture records two intentional embedding calls, zero hybrid completion events, Idle state, and exit code 0, with the takeaway that lexical BM25 fallback was not invoked.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/jcode-base/src/memory_agent.rs
Line: 658-677
Comment:
**Focused embedding failure disables lexical retrieval**
When the focused retrieval embedding fails, this branch sets the memory state to idle and returns before `find_similar_hybrid` executes. That prevents the hybrid search's BM25 pass from finding relevant memories even though lexical retrieval does not require a focused embedding. Fall back to lexical-only retrieval rather than returning on this error.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
| .await | ||
| { | ||
| Ok(Ok((embedding, _model))) => embedding, | ||
| Ok(Err(e)) => { |
There was a problem hiding this comment.
- Bug
- When the broad context embedding succeeds but
embed_query_active(&retrieval_text)fails, the focused-error arm returnsOk(()). The laterfind_similar_hybridcall, which performs dense-plus-BM25 fusion, is never reached, so a lexical-only match cannot be retrieved.
- When the broad context embedding succeeds but
- Cause
- The focused embedding error handling at
crates/jcode-base/src/memory_agent.rslines 658-677 treats the dense embedding failure as terminal rather than degrading hybrid retrieval to a lexical-only path.
- The focused embedding error handling at
- Fix
- On focused embedding failure, execute lexical/BM25 retrieval (or make the hybrid API accept an absent dense vector) rather than returning before
find_similar_hybrid.
- On focused embedding failure, execute lexical/BM25 retrieval (or make the hybrid API accept an absent dense vector) rather than returning before
Artifacts
- Executed Python control-flow harness against current memory_agent.rs with broad embedding success and focused embedding failure, showing the return precedes find_similar_hybrid; BM25 is skipped.
1 participant
Summary
Fixes five clear, independently verified issues found during open-issue triage:
allowed-toolsin skill frontmatter (Skill loader rejects YAML listallowed-toolsin SKILL.md frontmatter ("invalid type: sequence, expected a string") #1041).extra_content.google.thought_signaturedropped on replay #1040).Verification
cargo fmt --all -- --check--no-default-features.The all-default-features TUI test attempt reached the final TUI build but was terminated by host memory pressure while compiling
aws-sdk-bedrock; the targeted no-default-features TUI suite then passed.--- — Jcode agent (automated triage), on behalf of @1jehuang