fix(feedback): Phase 2 follow-up — Codex review MAJORs + NIT
Independent review surfaced three majors and one nit. All actioned. MAJOR 1 — profile scoping: `decompose_and_store` and `extract_tasks_from_transcript_cmd` now accept an optional `profile_id` (wired from `profilesStore.activeProfileId` in MicroSteps.svelte and DictationPage.svelte), and thread it into the feedback-retrieval query so per-profile decomposition styles do not leak into each other. `record_feedback` gets the same treatment. MAJOR 2 — prompt-budget regression on long inputs: New `trim_to_budget` helper + `FEW_SHOT_CHAR_BUDGET = 2000` char cap in `src-tauri/src/commands/tasks.rs`. Retrieval still pulls up to 5 rows but they are char-counted and truncated against the budget before being sent to the LLM. Char cost matches the `Input: ...\n Good output: ...` render path so the budget maps cleanly to ~570 Qwen3 tokens, well inside the 8192-context reserve after the 512- or 768-token response allocation. Oldest-first drop order (iteration stops at cost exceeded) preserves the most recent correction which is the one carrying the user's live preference. MAJOR 3 — inline edit stale-rollback race: `saveEdit` in MicroSteps.svelte now stamps a monotonic per-step `saveToken`. Each edit bumps the token; on failure the rollback only fires if `saveToken[step.id] === myToken`, so a slow-failing first save can no longer overwrite a faster successful second save. NIT — retrieval ordering stability: `list_feedback_examples` ORDER BY now `created_at DESC, id DESC`. SQLite timestamp precision is one second; without the secondary key, bursty feedback within the same second would select non-deterministically. Also: malformed `context_json` now warns via eprintln! rather than disappearing silently — Codex minor. All green: cargo build + 249 tests + clippy -D warnings + fmt + svelte-check (0/0) + npm run build.
This commit is contained in:
@@ -1007,7 +1007,7 @@ pub async fn list_feedback_examples(
|
|||||||
WHERE target_type = ?
|
WHERE target_type = ?
|
||||||
AND profile_id = ?
|
AND profile_id = ?
|
||||||
AND rating >= ?
|
AND rating >= ?
|
||||||
ORDER BY created_at DESC
|
ORDER BY created_at DESC, id DESC
|
||||||
LIMIT ?",
|
LIMIT ?",
|
||||||
)
|
)
|
||||||
.bind(target_type.as_str())
|
.bind(target_type.as_str())
|
||||||
|
|||||||
@@ -174,11 +174,23 @@ pub async fn uncomplete_task_cmd(
|
|||||||
/// recorder has stored it. Rows without usable input are dropped —
|
/// recorder has stored it. Rows without usable input are dropped —
|
||||||
/// the prompt builder filters them too, but doing it here keeps the
|
/// the prompt builder filters them too, but doing it here keeps the
|
||||||
/// exemplar list tight and the prompt budget predictable.
|
/// exemplar list tight and the prompt budget predictable.
|
||||||
|
///
|
||||||
|
/// Malformed `context_json` is logged rather than silently dropped so
|
||||||
|
/// data-integrity regressions surface instead of disappearing.
|
||||||
fn to_llm_examples(rows: Vec<FeedbackRow>) -> Vec<LlmFeedbackExample> {
|
fn to_llm_examples(rows: Vec<FeedbackRow>) -> Vec<LlmFeedbackExample> {
|
||||||
rows.into_iter()
|
rows.into_iter()
|
||||||
.filter_map(|r| {
|
.filter_map(|r| {
|
||||||
let ctx: serde_json::Value =
|
let raw = r.context_json.as_deref().unwrap_or("{}");
|
||||||
serde_json::from_str(r.context_json.as_deref().unwrap_or("{}")).ok()?;
|
let ctx: serde_json::Value = match serde_json::from_str(raw) {
|
||||||
|
Ok(v) => v,
|
||||||
|
Err(e) => {
|
||||||
|
eprintln!(
|
||||||
|
"[feedback] skipping row id={} with malformed context_json: {e}",
|
||||||
|
r.id
|
||||||
|
);
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
};
|
||||||
let input = ctx
|
let input = ctx
|
||||||
.get("input")
|
.get("input")
|
||||||
.and_then(|v| v.as_str())
|
.and_then(|v| v.as_str())
|
||||||
@@ -196,10 +208,48 @@ fn to_llm_examples(rows: Vec<FeedbackRow>) -> Vec<LlmFeedbackExample> {
|
|||||||
.collect()
|
.collect()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Rough character budget for the few-shot block. Qwen3's tokenizer
|
||||||
|
/// averages ~3.5 chars per token in English, so 2000 chars is ~570
|
||||||
|
/// tokens — well inside the 64-token reserve + response-token gap
|
||||||
|
/// against the 8192-token context cap (see `LlmEngine::generate`).
|
||||||
|
///
|
||||||
|
/// Exceed this and we drop the oldest examples first. Rationale: the
|
||||||
|
/// retrieval already orders most-recent-first, and the most recent
|
||||||
|
/// correction is usually the one carrying the user's live preference.
|
||||||
|
const FEW_SHOT_CHAR_BUDGET: usize = 2000;
|
||||||
|
|
||||||
|
fn example_char_cost(ex: &LlmFeedbackExample) -> usize {
|
||||||
|
// Matches the render path in `prompts::render_feedback_exemplar`:
|
||||||
|
// "Input: {input}\nGood output: {good}". Prefix strings + newlines
|
||||||
|
// + the two bodies. Slight overestimate to leave headroom.
|
||||||
|
let good_len = ex
|
||||||
|
.corrected_output
|
||||||
|
.as_deref()
|
||||||
|
.or(ex.original_output.as_deref())
|
||||||
|
.map(str::len)
|
||||||
|
.unwrap_or(0);
|
||||||
|
ex.input.len() + good_len + 24
|
||||||
|
}
|
||||||
|
|
||||||
|
fn trim_to_budget(mut examples: Vec<LlmFeedbackExample>) -> Vec<LlmFeedbackExample> {
|
||||||
|
let mut running = 0usize;
|
||||||
|
let mut kept = Vec::with_capacity(examples.len());
|
||||||
|
for ex in examples.drain(..) {
|
||||||
|
let cost = example_char_cost(&ex);
|
||||||
|
if running + cost > FEW_SHOT_CHAR_BUDGET {
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
running += cost;
|
||||||
|
kept.push(ex);
|
||||||
|
}
|
||||||
|
kept
|
||||||
|
}
|
||||||
|
|
||||||
#[tauri::command]
|
#[tauri::command]
|
||||||
pub async fn decompose_and_store(
|
pub async fn decompose_and_store(
|
||||||
state: tauri::State<'_, AppState>,
|
state: tauri::State<'_, AppState>,
|
||||||
parent_task_id: String,
|
parent_task_id: String,
|
||||||
|
profile_id: Option<String>,
|
||||||
) -> Result<Vec<TaskDto>, String> {
|
) -> Result<Vec<TaskDto>, String> {
|
||||||
let parent = db_get_task(&state.db, &parent_task_id)
|
let parent = db_get_task(&state.db, &parent_task_id)
|
||||||
.await
|
.await
|
||||||
@@ -208,12 +258,21 @@ pub async fn decompose_and_store(
|
|||||||
|
|
||||||
// Pull recent micro-step feedback so the system prompt gets
|
// Pull recent micro-step feedback so the system prompt gets
|
||||||
// conditioned on the user's preferred decomposition style. We
|
// conditioned on the user's preferred decomposition style. We
|
||||||
// cap at 5 examples to keep the prompt under budget regardless
|
// cap at 5 examples AND at a char budget to keep the prompt
|
||||||
// of how much feedback has been captured.
|
// under token budget regardless of how much feedback has been
|
||||||
let examples = db_list_feedback_examples(&state.db, FeedbackTargetType::MicroStep, 5, 0, None)
|
// captured, and scope by profile so per-profile styles do not
|
||||||
.await
|
// leak into each other.
|
||||||
.map(to_llm_examples)
|
let examples = db_list_feedback_examples(
|
||||||
.unwrap_or_default();
|
&state.db,
|
||||||
|
FeedbackTargetType::MicroStep,
|
||||||
|
5,
|
||||||
|
0,
|
||||||
|
profile_id.as_deref(),
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.map(to_llm_examples)
|
||||||
|
.map(trim_to_budget)
|
||||||
|
.unwrap_or_default();
|
||||||
|
|
||||||
let engine = state.llm_engine.clone();
|
let engine = state.llm_engine.clone();
|
||||||
let parent_text = parent.text.clone();
|
let parent_text = parent.text.clone();
|
||||||
@@ -245,12 +304,19 @@ pub async fn decompose_and_store(
|
|||||||
pub async fn extract_tasks_from_transcript_cmd(
|
pub async fn extract_tasks_from_transcript_cmd(
|
||||||
state: tauri::State<'_, AppState>,
|
state: tauri::State<'_, AppState>,
|
||||||
transcript: String,
|
transcript: String,
|
||||||
|
profile_id: Option<String>,
|
||||||
) -> Result<Vec<String>, String> {
|
) -> Result<Vec<String>, String> {
|
||||||
let examples =
|
let examples = db_list_feedback_examples(
|
||||||
db_list_feedback_examples(&state.db, FeedbackTargetType::TaskExtraction, 5, 0, None)
|
&state.db,
|
||||||
.await
|
FeedbackTargetType::TaskExtraction,
|
||||||
.map(to_llm_examples)
|
5,
|
||||||
.unwrap_or_default();
|
0,
|
||||||
|
profile_id.as_deref(),
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.map(to_llm_examples)
|
||||||
|
.map(trim_to_budget)
|
||||||
|
.unwrap_or_default();
|
||||||
|
|
||||||
let engine = state.llm_engine.clone();
|
let engine = state.llm_engine.clone();
|
||||||
tokio::task::spawn_blocking(move || engine.extract_tasks_with_feedback(&transcript, &examples))
|
tokio::task::spawn_blocking(move || engine.extract_tasks_with_feedback(&transcript, &examples))
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
<script lang="ts">
|
<script lang="ts">
|
||||||
import { invoke } from '@tauri-apps/api/core';
|
import { invoke } from '@tauri-apps/api/core';
|
||||||
import { ListTree, Check, Timer, Loader2, ThumbsUp, ThumbsDown, Pencil } from 'lucide-svelte';
|
import { ListTree, Check, Timer, Loader2, ThumbsUp, ThumbsDown, Pencil } from 'lucide-svelte';
|
||||||
|
import { profilesStore } from '$lib/stores/profiles.svelte.ts';
|
||||||
|
|
||||||
let { parentTaskId, parentTaskText = '', reduceMotion = false } = $props();
|
let { parentTaskId, parentTaskText = '', reduceMotion = false } = $props();
|
||||||
|
|
||||||
@@ -24,6 +25,13 @@
|
|||||||
let editing = $state<Record<string, boolean>>({});
|
let editing = $state<Record<string, boolean>>({});
|
||||||
let draft = $state<Record<string, string>>({});
|
let draft = $state<Record<string, string>>({});
|
||||||
|
|
||||||
|
// Monotonic token per-step. Each time the user kicks off a save we
|
||||||
|
// bump the token; the pending save remembers its token and only
|
||||||
|
// rolls back if the failure belongs to the still-current edit.
|
||||||
|
// Without this, a slow first save that eventually fails will stomp
|
||||||
|
// a faster second save that already committed.
|
||||||
|
let saveToken = $state<Record<string, number>>({});
|
||||||
|
|
||||||
async function loadSubtasks() {
|
async function loadSubtasks() {
|
||||||
loading = true;
|
loading = true;
|
||||||
error = '';
|
error = '';
|
||||||
@@ -40,7 +48,10 @@
|
|||||||
decomposing = true;
|
decomposing = true;
|
||||||
error = '';
|
error = '';
|
||||||
try {
|
try {
|
||||||
subtasks = await invoke<Subtask[]>('decompose_and_store', { parentTaskId });
|
subtasks = await invoke<Subtask[]>('decompose_and_store', {
|
||||||
|
parentTaskId,
|
||||||
|
profileId: profilesStore.activeProfileId,
|
||||||
|
});
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
error = String(e);
|
error = String(e);
|
||||||
} finally {
|
} finally {
|
||||||
@@ -94,6 +105,7 @@
|
|||||||
originalText: step.text,
|
originalText: step.text,
|
||||||
correctedText: null,
|
correctedText: null,
|
||||||
contextJson: feedbackContextJson(),
|
contextJson: feedbackContextJson(),
|
||||||
|
profileId: profilesStore.activeProfileId,
|
||||||
},
|
},
|
||||||
});
|
});
|
||||||
} catch (_) { /* feedback capture is best-effort, never fatal */ }
|
} catch (_) { /* feedback capture is best-effort, never fatal */ }
|
||||||
@@ -116,6 +128,10 @@
|
|||||||
const original = step.text;
|
const original = step.text;
|
||||||
// Update in-memory first so the UI is snappy; roll back if the
|
// Update in-memory first so the UI is snappy; roll back if the
|
||||||
// persistence call fails so we never show stale-but-different text.
|
// persistence call fails so we never show stale-but-different text.
|
||||||
|
// The monotonic `myToken` guards against a stale rollback stomping
|
||||||
|
// a fresher successful save that landed while we were waiting.
|
||||||
|
const myToken = (saveToken[step.id] ?? 0) + 1;
|
||||||
|
saveToken = { ...saveToken, [step.id]: myToken };
|
||||||
const idx = subtasks.findIndex(s => s.id === step.id);
|
const idx = subtasks.findIndex(s => s.id === step.id);
|
||||||
if (idx >= 0) subtasks[idx] = { ...subtasks[idx], text: next };
|
if (idx >= 0) subtasks[idx] = { ...subtasks[idx], text: next };
|
||||||
try {
|
try {
|
||||||
@@ -132,10 +148,18 @@
|
|||||||
originalText: original,
|
originalText: original,
|
||||||
correctedText: next,
|
correctedText: next,
|
||||||
contextJson: feedbackContextJson(),
|
contextJson: feedbackContextJson(),
|
||||||
|
profileId: profilesStore.activeProfileId,
|
||||||
},
|
},
|
||||||
}).catch(() => {});
|
}).catch(() => {});
|
||||||
} catch (_) {
|
} catch (_) {
|
||||||
if (idx >= 0) subtasks[idx] = { ...subtasks[idx], text: original };
|
// Only roll back if our token is still the most recent — a later
|
||||||
|
// edit that already succeeded must not be overwritten.
|
||||||
|
if (saveToken[step.id] === myToken) {
|
||||||
|
const rollbackIdx = subtasks.findIndex(s => s.id === step.id);
|
||||||
|
if (rollbackIdx >= 0) {
|
||||||
|
subtasks[rollbackIdx] = { ...subtasks[rollbackIdx], text: original };
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -487,7 +487,10 @@
|
|||||||
if (settings.aiTier === "tasks" && llmLoaded) {
|
if (settings.aiTier === "tasks" && llmLoaded) {
|
||||||
markGenerating("Extracting tasks");
|
markGenerating("Extracting tasks");
|
||||||
try {
|
try {
|
||||||
const items = await invoke("extract_tasks_from_transcript_cmd", { transcript: text });
|
const items = await invoke("extract_tasks_from_transcript_cmd", {
|
||||||
|
transcript: text,
|
||||||
|
profileId: profilesStore.activeProfileId,
|
||||||
|
});
|
||||||
markGenerationDone(true);
|
markGenerationDone(true);
|
||||||
return items.map((taskText) => ({ text: taskText }));
|
return items.map((taskText) => ({ text: taskText }));
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
|
|||||||
Reference in New Issue
Block a user