review(p9-fb-17): 회차 1 nit 반영
- `append_turn` 의 doc 은 "wrap in one transaction" 보장하지만 실제 코드는 auto-commit `conn.execute` 두 번이라 두번째 실패 시 first row 가 commit 된 채 inconsistent 됨. 진짜 transaction 으로 교체: `conn.transaction()` → `tx.execute(insert)` → `tx.execute(update)` → `tx.commit()`. SQLite BEGIN 으로 감싸 두 statement atomic. `lock_conn()` 이 `MutexGuard<Connection>` 반환하므로 `let mut conn = self.lock_conn(); let tx = conn.transaction()` 패턴 가능 (MutexGuard 의 DerefMut 활용). 9 chat_sessions 테스트 + clippy 통과. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -100,37 +100,45 @@ impl ChatSessionRepo for SqliteStore {
|
||||
}
|
||||
|
||||
fn append_turn(&self, turn: &ChatTurnRow) -> Result<()> {
|
||||
let conn = self.lock_conn();
|
||||
// Wrap insert + parent updated_at in one transaction so a
|
||||
// crash between the two never leaves a turn under a stale
|
||||
// `updated_at`.
|
||||
let tx_result: Result<()> = (|| {
|
||||
conn.execute(
|
||||
"INSERT INTO chat_turns
|
||||
(turn_id, session_id, turn_index, question, answer,
|
||||
citations_json, created_at)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?)",
|
||||
params![
|
||||
turn.turn_id,
|
||||
turn.session_id,
|
||||
turn.turn_index,
|
||||
turn.question,
|
||||
turn.answer,
|
||||
turn.citations_json,
|
||||
turn.created_at,
|
||||
],
|
||||
)
|
||||
let mut conn = self.lock_conn();
|
||||
// p9-fb-17 R1 fix: real transaction. The pre-fix code called
|
||||
// `conn.execute` twice in auto-commit mode, so a failure in
|
||||
// the second statement (UPDATE chat_sessions.updated_at) would
|
||||
// leave the first (INSERT chat_turns row) committed —
|
||||
// inconsistent state where the turn exists under a stale
|
||||
// session updated_at. `conn.transaction()` opens BEGIN, both
|
||||
// statements share it, `commit()` lands them atomically.
|
||||
let tx = conn
|
||||
.transaction()
|
||||
.map_err(StoreError::from)
|
||||
.context("append_turn: insert")?;
|
||||
conn.execute(
|
||||
"UPDATE chat_sessions SET updated_at = ? WHERE session_id = ?",
|
||||
params![turn.created_at, turn.session_id],
|
||||
)
|
||||
.context("append_turn: begin transaction")?;
|
||||
tx.execute(
|
||||
"INSERT INTO chat_turns
|
||||
(turn_id, session_id, turn_index, question, answer,
|
||||
citations_json, created_at)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?)",
|
||||
params![
|
||||
turn.turn_id,
|
||||
turn.session_id,
|
||||
turn.turn_index,
|
||||
turn.question,
|
||||
turn.answer,
|
||||
turn.citations_json,
|
||||
turn.created_at,
|
||||
],
|
||||
)
|
||||
.map_err(StoreError::from)
|
||||
.context("append_turn: insert")?;
|
||||
tx.execute(
|
||||
"UPDATE chat_sessions SET updated_at = ? WHERE session_id = ?",
|
||||
params![turn.created_at, turn.session_id],
|
||||
)
|
||||
.map_err(StoreError::from)
|
||||
.context("append_turn: bump updated_at")?;
|
||||
tx.commit()
|
||||
.map_err(StoreError::from)
|
||||
.context("append_turn: bump updated_at")?;
|
||||
Ok(())
|
||||
})();
|
||||
tx_result
|
||||
.context("append_turn: commit")?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn list_turns(&self, session_id: &str) -> Result<Vec<ChatTurnRow>> {
|
||||
|
||||
Reference in New Issue
Block a user