Repository navigation
feat(rpc): TSF の変更系送信を Change / Restore へ移行し、BG 系の応答型を確定する(#56 PR (b)) - #68
Conversation
|
PR #67 の 10/3 のコメント(C について本文に書く 3 点と、レビューで見る 3 点)を確認しました。この PR はそのコメントの前に出していたので、本文に「C の補足」の節を足しました。
レビューで見る 3 点のうち、 |
PR #68 を確認しました。A〜E と、10/3 に求めた C の補足 3 点は、本文のとおり実装されていることをコードで確かめました。その上で、マージ前に直してほしい点が 1 つ、確認したい点が 2 つ、細かい点が数件あります。コードを読んでの照合で、実機では確かめていません(実機の確認は、(b) の修正が済んでから別に行います)。 確認できたこと
修正してほしい点1. conv_cache の poison で、変更系も
|
|
先ほどのレビューコメント(https://github.com/fukuyori/rakukan/pull/68#issuecomment-5987071786)の poison の節に、2 か所の誤りがあり、コメントを直しました。修正してほしい点 1 の趣旨と、他の指摘は変わりません。
すでに初版を読んでいた場合は、上のコメントの訂正後の内容を確認してください。 |
|
レビューをありがとうございます。訂正後の内容で、 1. conv_cache の poison — 直します(a) までと同じく、poison でも入力が動き続ける形に戻します。致命扱いにはしません。
poison を検知してホストを自己再起動させる処理は、#43 の範囲と考え、この PR には入れません。 2. 打鍵ごとの RPC — 1 回にまとめます入口の所有権の確認と、診断ログ用の 3. 変更の失敗の握りつぶし — 表示に続く経路には及ぼします
範囲を全部に広げるほうがよければ、そうします。 細かい点
テスト
1 の形( |
|
方針を確認しました。 実装の前に、方針に足してほしい点が 4 つあります。1 つ目は、方針の目的(入力が動き続けること)に関わります。 1. 入口の所有権確認の読み取りも、poison で失敗させない
方針では、BG の読み取りを 次の 2 点を、方針に足してください。
2.
|
|
確認をありがとうございます。4 点とも、 1. 入口の所有権確認の読み取り
2.
|
|
10/7 の方針を確認しました。 実装の前に、足してほしい点が 4 つあります。1 つ目は、方針の目的(poison でも入力を続ける)に関わり、10/5 のレビューと 10/6 の追加の要求のどちらでも、私が漏らしていた点です。 1. BG と無関係な読み取りも、poison で失敗するホストの TSF は、これらの読み取りを 次を方針に足してください。
2.
|
|
訂正(10/8): 10/5 のレビューから、先ほどのコメントまで、私は conv_cache の poison が配布ビルドでも起きる前提で書いていました。この前提が誤っていました。poison に関する要求を取り下げ、範囲を縮めます。 根拠
したがって、標準のタスクの既定でビルドした場合、poison が起きうるのは、巻き戻しのあるビルド( ただし、スクリプトは 取り下げる要求
取り下げの代わりにお願いすること10/5 のコメントで示した 2 つの選択肢のうち、「直さない」側にします。その場合に理由を残すようお願いしていたので、上の取り下げの代わりに、次の小さな追加だけお願いします。
引き続きお願いする点10/6 に了承した次の点は、変わりません。
確認したい点nick さんの手元で、次のいずれかに当てはまる場合は教えてください。その場合は、poison の扱いを改めて決めます。
10/6・10/7 に検討してもらった内容の多くを、取り下げる形になります。上の確認したい点に該当しなければ、上の範囲で実装を進めてください。 |
|
訂正と範囲の整理をありがとうございます。根拠の 2 点( 確認したい点への回答どちらにも該当しません。
配布ビルドで panic を起こしての確認は、こちらでもしていません。 実装する範囲示していただいた範囲で進めます。
main が |
Part (b) of fukuyori#56. The TSF now sends every composition change through `Change`, starts each composition with `Restore { .., then }`, and keeps the reading and pending romaji returned with each outcome as the restore source. Composition reads go through `Read { expect, request }` and return `Result`; failures are no longer turned into empty values. BG conversion answers are typed (`BgView`, `BgStartOutcome`, `BgTakeOutcome`) and are derived from `(bg_owner, BgSlot)`. `BgSlot` is a new engine DLL read, so the engine ABI goes 10 -> 11 in both places. The old change/read variants become `_Reserved…` and the host answers `Error`. v6 is unreleased, so no further protocol version bump. Refs fukuyori#56 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Extract the engine operations start_bg needs into a BgStarter trait so the fake used by the read-side tests can drive it. Behaviour is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Fold the per-key ownership check and the diagnostic bg_status into one read: recover_on_input returns the BgView it observed. - push_raw / force_preedit return Result and callers stop handling the key on failure instead of reading a stale preedit. - Do not save a brand-new composition whose first change failed without sending anything; skip bg_reclaim when no owned composition exists. - Keep conv_cache poison as Response::Error and document why (release builds use panic = "abort", so the mutex cannot be poisoned). - Route the BgTake replay test through take_bg, pin the appended Request/Response discriminants, and fix four stale comments. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
f2fc9bb to
6e5d52a
Compare
|
10/8 の範囲で実装し、push しました(main
fmt、TSF / RPC / engine-abi の Clippy、TSF 206 件・RPC 69 件のテストは、手元で通っています。engine と engine-abi のテストは、今回は再実行していません。実機では確かめていません。 |
|
10/8 に残した依頼について、 確認できたこと
マージしますこの PR は、このままマージします。 照合の中で、次の点が見つかりました。1〜3 は、10/6 に合意した「
実機での確認(ホストの再起動をまたいだ入力の復元など)は、RPC protocol v6 をリリースする前の統合試験で行います。今回も、コードを読んでの照合で、実機では確かめていません。 長い期間、対応をありがとうございました。 |
ホスト再起動後も、composition ごとに保持した読みと未確定ローマ字から編集を復元できるようにします。最初の変更は
Restore.thenで送り、返された世代を後続のChangeに使います。復元は有効なフォーカス先へのキー入力に限り、タイマーは所有権を取り戻しません。復元した Space は Preedit に戻し、次の Space で変換します。BG の状態と開始・取得結果を型で区別し、旧要求は宣言順とフィールドを保った
_Reserved…にしてホストから Error を返します。読み取りにも所有者・世代の照合を加え、取得失敗で表示を空にしたり Idle と扱ったりしません。受入条件と対応
crates/rakukan-engine/src/ffi.rsのENGINE_ABI_VERSIONとcrates/rakukan-engine-abi/src/lib.rsのEXPECTED_ENGINE_ABI_VERSIONを同時に 10 → 11。cpu / vulkan / cuda の 3 variant すべてが 11 を返す。every_backend_exports_abi11_and_exact_pending_romajiがテストで確かめるのは、そのとき実行しているビルドの variant の版と未確定ローマ字だけです(手元で実行したのは cpu と Vulkan feature です)。3 variant に共通という点は、版を返す export に feature の条件が無いことをコードで読んでの判断です。CUDA Toolkit がないため、CUDA ビルドでの実行は未確認です。bg_reclaim()は、engine_gen/ownerの照合に通った後だけ。回収するのはbg_ownerが無いDoneだけで、自分のDoneは読み取りでは回収しない。冪等(2 回目は状態不変)で、応答・再送に影響しない。この 3 点をテストで確かめる。checked_read_slotとrejected_reads_do_not_reclaim/reads_preserve_own_done_and_reclaim_orphan_once_without_touching_replayで確認します。BgStart内の自分のDoneの回収はtry_lock()ではなく blocking lock(空振りを起こさない)。poison は読み取り失敗と同じ扱い。WorkerBusyに自分のDoneを含めない。start_bg/reclaim_done_fromと、開始判定・ロック競合・poison のテストで確認します。BgSlotの doc に、same_readingはpendingがあればpendingのキーで判定/順位はQueued>Running>Done>Empty/Queuedは旧要求の変換が実行中で次の要求が待っている場合を含む、を書く。conv_cache::BgSlotの doc とclassify_slotのテストに反映しています。BgTakeCandidates再送では保持したBgTakeOutcomeを返し、取り出しを再実行しない。テストあり。apply_recorded_changeとbg_take_replay_uses_retained_outcome_without_taking_twiceで、空の Taken も含めて確認します。このテストは、apply_changeのBgTakeCandidatesが呼ぶのと同じtake_bgを通します。NotYours(所有者なし・別の所有者で、変換器に触れない)とNotReadyは、bg_take_touches_the_converter_only_for_its_ownerで確認します。LearnAction::Skipを使う。判定は新設)。見送った事実をログに残す。catch_up_live_previewと新設learn_live_actionで処理し、Skip 判定をテストしています。アプリへの確定は現在のプレビューで続けます。Response::Errorで返し、TSF はReadErrorとして扱う。Rejectedは照合に落ちた意味に限定する。poison_is_error_not_idle_or_rejectionと TSF の ReadError 分類テストで確認します。Space の WorkerBusy は独立した分岐で辞書候補を表示し、MODEL_WAIT の上限を適用します。Waiting とライブ WorkerBusy の上限は 10 秒です。ライブの時計はタイマーを再開しても維持し、別の BG 状態を観測した場合に解除します。時計のテストを追加しています。
検証
cargo fmt --all -- --check:成功です。-D warnings):TSF / RPC / engine-abi が成功です(10/9 の修正後に再実行)。engine / engine-host は 10/3 の結果で、10/9 の修正では再実行していません(engine は変更なし、engine-host は RPC の変更だけを受けます)。C の補足(10/3 のご依頼の 3 点)
bg_ownerのないDoneの回収)はtry_lockのままです。空振りしたDoneは、10/2 の案どおりWorkerBusyに含めます(reclaim_unowned/checked_read_slot、テストはqueued_running_and_missed_orphan_done_are_worker_busy)。blocking にしたのは、BgStartの中の、自分のDoneの回収だけです。入口は読み取り要求でも通るので、待ちうる箇所を増やさないほうを選びました。conv_cache::reclaim_done_blocking()を新しく足し(ABI はengine_bg_reclaim_blocking)、呼ぶのはホストのstart_bgが自分のDoneを見たときだけです。try_reclaim_done()と、それを使うbg_start()(lib.rs)・start_load_model(ffi.rs)・bg_reclaim()→reclaim_nonblocking()の経路は、(a) のままです。RestoreとBgReclaimの回収の挙動は変わりません。pendingを取り出してRunningを書く区間と、Doneを書く区間の 2 つだけです(変換そのものはロックの外です)。stall watchdog はstall_probe→ キャッシュのロック(run_state/confirm_stalled)→ 解放 →healthの順で、エンジンのロックは取りません。循環はありません。待ちが長引いた場合。 blocking の待ちに上限は置いていません。理由は次の 2 点です。
Doneを書く区間も、値の移動とnotify_allだけです。BgStartの待ちは延びません。その場合は stall watchdog が同じロックで状態を読み、既存の手順でホストを終了します。キー処理の中でこのロックを blocking で取る経路は、(a) 以前から
take_ready/status/wait_done_timeoutにあり、今回の追加で性質は変わりません。テスト。 競合時に取り損ねず待つこと(
reclaim_waits_for_contention_instead_of_missing_the_lock)と、poison を失敗として返すこと(poisoned_slot_and_blocking_reclaim_return_failure)はエンジン側にあります。ホスト側の poison →Response::Errorは、読み取り(BgStatus)にテストがあります(poison_is_error_not_idle_or_rejection)。BgStart側はbg_start_poison_is_error_and_does_not_startを足しました(状態を読む段階と、blocking の回収の段階の両方)。合意に無い判断(ご確認ください)
BgSlot以外の追加があります。 未確定ローマ字の取得(復元元に必要)、poison を区別する取り出し・peek、blocking の回収(C)です。Request::Read { expect, request }に集約しました。 旧の読み取り variant(PreeditDisplayなど)とConvertSync/BgWaitMsも_Reserved…にしています。Restoreします)。この読み取りは、入口の診断ログ用のbg_statusと 1 回にまとめました(10/9)。ふだんの打鍵の往復数は、(a) までと同じです。GetFocusと文脈の一致、DM 台帳での生存を使っています。フォーカス通知が遅れて届く間も最初の打鍵を扱えるよう、一致した DM をその場で記録します。A〜E の内容は、10/3 の回答から変えていません。
10/5 のレビューへの対応(10/9)
範囲は、10/8 の訂正のとおりです。
BgView::Unreadable、Restored.thenの「適用しなかった」、poison 下で入力を続けるための変更は入れていません。conv_cache の poison は
Response::Errorのまま変換キャッシュの mutex が poison していると、
apply_changeの冒頭の回収とbg_slot()、dispatch_restoreのbg_slot()、読み取りのchecked_read_slotが失敗し、BG と無関係な変更・復元・読み取りもResponse::Errorになります。直していません。理由は次のとおりです。cargo make build-engine/build-tsf)でビルドした配布物は、[profile.release]がpanic = "abort"なので、panic で巻き戻しが起きず、mutex は poison しません。cargo test、-Profile debug)だけです。その場合はErrorを返し、入力の継続は保証しません。conv_cache::status()は"idle"を返すので、変更要求で呼ばれるapply_health_actionは、ホストの終了を要求しません。Restoreは冒頭で早期 return するので、apply_health_actionに届きません。同じ内容を
apply_changeの doc コメントに書いています。既存の poison のテスト(poison_is_error_not_idle_or_rejection/bg_start_poison_is_error_and_does_not_startと、エンジン側のテスト)は、そのまま残しています。配布ビルドで panic を起こしての確認はしていません。入口の読み取りを 1 回に
recover_on_inputが、所有権の確認に使ったBgStatusの値を返し、入口の診断ログはその値を使います。フォーカスの無い文脈(recover_on_inputを呼ばない)と、復元した直後は、これまでどおり診断ログの側で 1 回読みます。WorkerBusyの時計の解除は、入口の読み取りでもbg_status()と同じ条件で行います。変更の失敗を止める範囲
push_raw/force_preeditはResultを返し、呼び出し元(27 か所)は、失敗したらそのキーの処理を中断します。commit/reset_*/learn*/bg_reclaimは、後ろに読み取りが続かない終端の操作なので、警告ログのままです。細かい点
Commitなどの再送できない要求)は、手元の状態を保存しません。送った後の失敗(通信の失敗、Restoreの拒否)は、再送用の要求を残すために、これまでどおり保存します。first_change_that_sends_nothing_is_not_savedで確認します。abi/lib.rs(pending_romajiの上)、client.rs(ReadErrorの上)、protocol.rs(Response::Restored)、candidate_window.rs(機械置換で崩れた 3 か所)を、実装に合わせて直しました。bg_reclaim(): 所有権を持つ composition が無ければ、送らずに戻ります。warn は出ません。WorkerBusyの待機表示: 10/1 の合意どおり、既存のモデル待ちの表示のままです。テスト
take_bg(apply_changeのBgTakeCandidatesの本体)を通す形に直しました。取り出しの回数は、偽の変換器の側で数えます。codec.rsにappended_variants_keep_their_slotsを足し、Request::Read(Changeの 2 つ後ろ、56)と、Response::Bg/Response::OptionalString(Changedの 1 つ後ろ・2 つ後ろ、17 / 18)の番号を固定しました。既知の制限
Restoreはinput_logを復元しません。EditStateが持つのは、読みと未確定ローマ字だけです。ホストの再起動をまたいだ composition では、打鍵ログを使う操作(F9 / F10 のラテン変換など)が、再起動前の打鍵を対象にできない可能性があります(コードを読んでの判断で、実機では確かめていません)。(b) の範囲外としています。Refs #56
🤖 Generated with Claude Code