You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
completed primary review; remaining group budget is insufficient for another measured pass and finalization
completed primary review; remaining group budget is insufficient for another measured pass and finalization
primary pass completed; optional refinement did not complete: bounded analysis window ended; explicit finalization required; the grace round did not complete the task
primary pass completed; optional refinement did not complete: finalization did not produce an explicit successful task_done for every group file
completed primary review; remaining group budget is insufficient for another measured pass and finalization
completed primary review; remaining group budget is insufficient for another measured pass and finalization
bounded analysis finalized with explicit task_done after reviewing all group files
bounded analysis finalized with explicit task_done after reviewing all group files
tool output exceeded the byte/line budget; use narrower file ranges or paginated searches; omitted output is not reviewed
… 3 additional warning(s) omitted.
1. Memory/src/storage/repositories.ts (L5226)
invalidateUnifiedL2Source scopes on l2.extract.snapshot.v1:% (and l2.extract.operation.v1:% below), but the unified pipeline only ever writes l2.extract.snapshot.v2:... (l2-extraction-pipeline.ts:304) and l2.extract.operation.v2:... (l2-extraction-pipeline.ts:415). As a result this purge will match zero rows for real data: when an L1 is deleted/withdrawn the v2 snapshots (which persist the full input.L1_RECORDS, i.e. user text) and operation tombstones are never scrubbed. The unit test seeds v1 keys itself, so it does not catch the mismatch. Align both LIKE prefixes with the actual v2 keys (and update the test fixtures accordingly).
This inline render duplicates renderL2Lesson exported from l2-content-revision.ts (used by revisedL2Memory). Reuse the shared helper so create/revise stay in sync; otherwise a change to the rendered format must be applied in two places.
Nested ternary (negative ? ... : ordinaryOnly ? ... : ...) is prohibited by the review checklist; move the decision into a small helper or precomputed const for readability. The same pattern is used on the next line for experience_type.
This is a nested ternary (confirmed ? ... : fullyReviewed ? ... : ...), which the checklist disallows. Consider computing retire/fields with an explicit if/else chain or a helper function.
This chained conditional expression is a multi-level nested ternary, which is disallowed by the review checklist and hurts readability. Prefer a switch (action) (or a lookup map of allowed key sets) to make the per-operation contract explicit.
The five-line "L2 compatibility" contract is duplicated verbatim in L2_SKILL_CRYSTALLIZE_PROMPT and SKILL_REBUILD_PROMPT, so a later edit to one variant will silently diverge from the other. Extract it into a shared constant (e.g. const L2_COMPATIBILITY_RULES = "...") and concatenate it in both prompts. Also note this block is appended to the crystallize prompt even though EXISTING_SKILL_SNAPSHOT is only supplied on the rebuild path (see skill-pipeline.enhanceSkillDraft), so that reference is unreachable in the crystallize contract.
numberField(policy, "schema_version") is computed three times inside this object literal, which makes the v2 branch easy to miss when editing defaults (experienceType, evidencePolarity, skillEligible). Hoist it once at the top of the function, e.g. const schemaVersion = numberField(policy, "schema_version");, and reuse it, so the v2 fallbacks stay in one obvious place.
The cache key is derived from a hardcoded "query" literal while applyEmbeddingVector re-derives it from item.role. Today both are "query" so the guard always passes, but if the L2 role is ever changed the write-side guard would silently become false and the cache would never be populated (reads keep using the query namespace). Derive the key from the same value that is stored on the item (e.g. build the item first with role, then set l2VectorCacheKey from item.role) so the two sides cannot drift.
The cached vector is keyed by item.text, but the embedding is computed by the callers on item.text || "(empty)" (runLeasedEmbeddingJobs/runLeasedEmbeddingItem). For an L2 row with empty text this stores a vector for the literal "(empty)" under the empty-string key, and any later empty-text L2 row (or the L2 extraction pipeline, which does not substitute) would reuse that unrelated vector. Skip the cache write when the text is empty.
The persistent L2 vector cache ignores config.cache (EmbeddingConfig.cache), which already disables the embedder's in-memory cache in model/embedder.ts. With cache: false vectors are still persisted to runtime_kv keyed by the text digest, so the opt-out is not honoured. Additionally, entries are never expired or pruned, and every L2 content revision produces a new key, so runtime_kv grows without bound. Please gate l2VectorCacheKey/readL2VectorCache on config.cache and provide a retention/pruning strategy for the l2.extract.vector.v1: prefix.
recomputePolicyStats is not a pure read: it writes the L2 row, appends a policy_stats_update change, calls onSkillRewardDrift (which mutates dependent skill eta/status) and enqueues skill_crystallization without a dedupeKey. Calling it once per updated trace means a policy shared by N traces in the same backprop is recomputed N times, producing redundant writes/change rows and N near-duplicate crystallization jobs. Collect the affected policy ids across the loop and recompute each once after the loop.
enhanceSkillDraft now returns llm-failed: configured skill model unavailable when the model is not configured, and this branch throws for every llm-failed: reason. Previously an unconfigured skill model produced the benign llm_disabled skip and the job completed; now every crystallization/revision job fails, burns its retry attempts, and (because the throw happens inside the policyMemories loop) also aborts the remaining policies in that job. A misconfiguration should not be treated as a transient retryable LLM failure — return a non-llm-failed skip reason (e.g. skill_model_unavailable) and continue.
policyMetaFromMemory(current) can return null (e.g. the policy payload was rewritten/invalidated concurrently); the ! assertion would then throw a raw TypeError: Cannot read properties of null from inside the write transaction instead of the intended retryable "source changed" error. Guard the parsed metadata explicitly and throw the retry error when it is missing.
After a successful revision the pending_policy_revisions[policy.id] entry is written by suspendForPolicyRevision but never cleared here (only policy_content_hashes is updated), so the skill keeps a stale pending revision forever. The crystallization gate later re-reads that map for every policy id, and any future content change on that policy silently re-enters the pending state without a new suspension/enqueue. Clear the satisfied entry as part of the commit fields.
In the failure path, persistFeedbackFallbacks runs renew() inside its own transaction. If the scope lease was taken over (or expired) renew() throws, so the fallback-persistence error replaces the original extraction error and the real cause is lost from logs/retry diagnostics. Wrap the fallback write so the primary error is always the one rethrown.
listL2Candidates is called once per candidate inside this loop, and each call re-loads and re-upserts every pending candidate for the episode (each upsert opening its own nested transaction and re-reading the source L1). That makes this loop O(n²) DB work inside a single transaction. Fetch the pending list once before the loop and look up each candidate by id/version instead.
The pending snapshot is written before the model call and stores the complete input (all L1 records plus historical candidates). If extraction keeps failing (transport/validation errors), these rows stay in runtime_kv forever with a full copy of the source text — nothing ever prunes pending snapshots (only the committed one is reused). Consider deleting/truncating the pending snapshot when the batch aborts, or add a retention sweep for non-committed l2.extract.snapshot.* entries.
This refresh relies on the implicit invariant that upsertL2Candidate merges the argument route into the previously stored routes. Passing evidence.routes[0] (an arbitrary member of the already-stored list) only works because nothing drops the other routes; if the merge logic ever validates or filters routes/signals, a plain refresh would silently truncate provenance. A dedicated refreshL2CandidateSource(repos, { sourceMemoryId, userId }) (no route argument) would make the intent explicit and remove the dependency.
The field set built here has drifted from the one used by createLesson in l2-extraction-pipeline.ts: this version also writes title/trigger/procedure/verification/boundary, decision_guidance, expected_outcome and exclusions into info/internal_info, while createLesson only stores them under policy. Consumers reading memory.info.title will therefore see different shapes for a created vs. a revised lesson. Extract a single shared field builder for create/revise so both produce the same schema.
21. Memory/src/service/memory-service.ts (L2978)
This per-page list() + traceMeta() scan is heavier than needed: traceMeta() issues a getRawTurn() query and JSON parsing for every L1 memory in the session, so redacting one raw turn costs O(session L1 size) DB round-trips. this.repos.runtime.l1MemoryIdsForRawTurn(rawTurn.id) already resolves the L1 ids for a raw turn (info/internal_info.trace paths); combining it with a single targeted lookup for internal_info.source_raw_turn_id and memories.getMany() would avoid the full scan. Also note that offset pagination is being used while invalidateMemoryDependencies() mutates dependent rows — if any touched row falls into the created_at DESC, updated_at DESC, id DESC ordering, later pages can skip entries.
22. Memory/src/model/llm.ts (L254-L255)
This early throw bypasses the failure bookkeeping used by every other exit path of completeJson: this.lastError is not updated and no json.failed event is logged, so status().lastError stays stale and the truncation is invisible in diagnostics. Consider reusing the shared failure path (set this.lastError, log with memoryErrorFields(...), include operation/attempt/finishReason) or at least log a json.failed entry before throwing.
23. Memory/src/config/index.ts (L1286-L1287)
recallTopK is floored while maxInputChars/maxOutputTokens are only floored at the lower bound without being made integral (e.g. 48000.7 would be stored as-is and compared against input.length). Suggest normalizing all counts consistently with Math.floor(...) and, for maxOutputTokens, keeping it integral since it is passed straight to the provider as maxTokens.
The literal "Continue using this approach:" is duplicated here and in refineFeedbackExperienceByRules (line ~2819) where it is concatenated into procedure. If that wording ever changes, this check silently stops matching and raw feedback text is no longer substituted (or, conversely, a non-praise procedure would be replaced by raw text). Extract the phrase into a shared const and build the procedure from it in both places.
💡 Suggested Change
Before:
const content = refinement.procedure.startsWith("Continue using this approach:")
? input.text
: refinement.procedure;
Nested ternary, which the project style rules disallow. Replace it with an if/else chain or a small helper so the precedence (check > principle > local_procedure) stays readable.
traceMemory is only narrowed inside the ternary above, so the non-null assertion is needed here even though candidate !== null implies it. Restructure to keep the narrowing (e.g. early const source = traceMemory && trace && traceMemory.userId === feedback.userId ? traceMemory : undefined; then if (!source) {...}, and read source.sessionId), so the invariant is enforced by types rather than by !.
Timestamps are compared as raw strings (also in the item.createdAt > revisedAt filter below), which only holds when both values use the exact same ISO-8601 form. content_revised_at comes from the at argument of suspendForPolicyRevision, so any caller passing a differently formatted/precision timestamp (or a row written by an older version) will silently include or exclude historical trials and skew attempted/passed. Compare parsed instants instead.
💡 Suggested Change
Before:
if (revisedAt && trial.createdAt <= revisedAt) return;
Automated tests were not run because the changed files do not map to an executable test scope.
Details: No executable test scope maps to the changed files. Automated tests were not run; add env.yaml source_mapping + execution for this path family, then rerun. Changed files: Memory/src/algorithm/plugin-algorithms.ts, Memory/src/config/index.ts, Memory/src/model/llm.ts, Memory/src/model/types.ts, Memory/src/service/embedding/embedding-job-processor.ts, Memory/src/service/embedding/l2-vector-cache.ts, Memory/src/service/evolution/evolution-job-processor.ts, Memory/src/service/evolution/l2-candidate-service.ts, Memory/src/service/evolution/l2-content-revision.ts, Memory/src/service/evolution/l2-extraction-contract.ts
Manual review or env.yaml source_mapping/execution coverage is required before merge.
Branch:l2-dev
This branch has not been deployed
No deployments
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
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.
修改内容
统一 L2 经验提取与更新流程,默认启用新版链路;对后续从 L2 提取 Skill 的提示词、来源追踪及失败恢复做初步兼容。保留旧版 L2 模式,Episode/Trace 直接提取 Skill 的路径不变。
L2 统一提取与自动修订
algorithm.l2Induction.mode默认使用unified,可显式切换为legacy使用原有链路。principle、local_procedure、check、troubleshooting四类,分别表达原则、局部流程、检查项和排障经验。support、revise、create、ignore。将冲突处理收敛为revise:生成完整新正文,通过来源、目标版本及内容校验后自动应用。L2 → Skill 初步兼容
Embedding 复用与 v1.2.7 兼容
v1.2.7,保留其事件循环让出及持久化重试退避;为两个 L2 任务入口补齐 evolution 与 embedding 资源准入,并升级任务策略版本,使已有排队任务使用新策略。验证结果(2026-10-30,本次提交)
tsc -p Memory/tsconfig.json --noEmit。验证范围
66ec4616,目标分支为MemTensor/memmy-agent:v1.2.7。客户端包沿用项目当前版本号1.2.4,未在本 PR 中调整产品版本号。