feat(feff10): add parity coverage and SFCONV support#8
Conversation
Greptile SummaryThis PR wires an optional Confidence Score: 5/5Safe to merge; only P2 style findings remain No P0 or P1 issues found. The SFCONV injection logic is correctly guarded at the Tauri command layer, type safety is maintained across the Rust/TS boundary, the 0.2 API upgrade is clean, and the parity tests provide meaningful regression coverage. Two P2 suggestions (restore frontend CI and add a library-level guard for use_sfconv) do not block merge. .github/workflows/rust.yml (CI coverage gap) and xraytsubaki-app/src-tauri/src/commands/fitting.rs (use_sfconv library-level validation)
|
| Filename | Overview |
|---|---|
| .github/workflows/rust.yml | Removes the entire gui CI job (TypeScript type-check, lint, Tauri build check) with no replacement |
| crates/xraytsubaki/src/xafs/fitting/runner.rs | Upgrades FEFF10 runner to 0.2 API; adds SFCONV injection via ensure_other_card_present; improves strict-card parse error hints |
| crates/xraytsubaki/src/xafs/fitting/types.rs | Adds use_sfconv field to FeffRunRequest with serde default = false |
| crates/xraytsubaki/tests/feff10_s02k_parity.rs | Adds FEFF10 vs FEFF85L parity test and A×S02(k) amplitude stability regression test across k-ranges |
| xraytsubaki-app/src-tauri/src/commands/fitting.rs | Threads use_sfconv through run_feff_paths Tauri command with validation guard; adds unit tests for resolve_feff_run_mode |
| xraytsubaki-app/src-tauri/src/dto.rs | Adds use_sfconv bool field with serde default to FeffRunConfig DTO |
| xraytsubaki-app/src/backend/types.ts | Adds optional use_sfconv field to FeffRunConfig TypeScript interface |
| xraytsubaki-app/src/panels/FitPanel.tsx | Adds SFCONV checkbox to FEFF run section; wires use_sfconv into runConfig state |
| Cargo.lock | Routine dependency version bumps (anyhow, nalgebra, syn, bitflags, etc.) |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[FitPanel.tsx\nuse_sfconv checkbox] -->|FeffRunConfig| B[run_feff_paths\nTauri command]
B --> C{resolve_feff_run_mode}
C -->|explicit_executable AND use_sfconv| D[Error: SFCONV only in FEFF10 mode]
C -->|no explicit_executable| E[Feff10Pipeline mode]
C -->|explicit_executable\nuse_sfconv=false| F[Feff85LModules mode]
E --> G[run_feff10_pipeline]
G --> H{use_sfconv?}
H -->|true| I[ensure_other_card_present\nappend SFCONV card]
H -->|false| J[FeffInput unchanged]
I --> K[FeffPipeline.run]
J --> K
F --> L[run_feff85l_modules]
K --> M[FeffRunResult\npath_files]
L --> M
M --> N[FitPanel paths state\nfeff_dat_path list]
Prompt To Fix All With AI
This is a comment left during a code review.
Path: xraytsubaki-app/src-tauri/src/commands/fitting.rs
Line: 34-46
Comment:
**`use_sfconv` silently ignored at the library level**
The Tauri command layer correctly rejects `use_sfconv: true` combined with an explicit executable, but this guard exists only there. A library consumer who directly builds a `FeffRunRequest` with `mode: Feff85LModules` and `use_sfconv: true` and then calls `run_feff()` will get a silent no-op — the flag is never read by `run_feff85l_modules`. Consider adding a `debug_assert!(!(request.use_sfconv && request.mode == FeffExecutionMode::Feff85LModules))` inside `run_feff85l_modules`, or documenting clearly that `use_sfconv` is only honoured in `Feff10Pipeline` mode.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: .github/workflows/rust.yml
Line: 76
Comment:
**Frontend CI coverage removed**
The `gui` job — which ran TypeScript type-checking (`npm run type-check`), ESLint (`npm run lint`), and `cargo check` on the Tauri backend crate — was dropped entirely with no replacement. This PR introduces new TypeScript types and a React checkbox component; without the job, regressions in the frontend can slip through undetected. Consider restoring the job or adding a leaner type-check/lint step.
How can I resolve this? If you propose a fix, please make it concise.Reviews (3): Last reviewed commit: "fix: address FEFF10 PR review findings" | Re-trigger Greptile
Summary
use_sfconvflag through the Tauri and React fit flow for FEFF10 runsTesting