Conversation
- Major backend refactor: replaced ASP.NET Core/SignalR host with new stdio JSON-RPC host (`SlideGenerator.Ipc`) - Removed all SignalR, Hangfire, and ASP.NET Core infrastructure, DTOs, and related code - Introduced new projects: `SlideGenerator.Ipc` (JSON-RPC host), `SlideGenerator.Configs` (config system), `SlideGenerator.Generating` (generation runtime) - Overhauled configuration system: new `ConfigManager`, YAML-based config, simplified schema, removed old DTOs/records - Removed all legacy job abstractions, enums, and orchestration logic; new runtime encapsulates generation and validation - Updated build scripts, solution files, and documentation to reference new backend structure and usage - Updated coding conventions: enforced one top-level type per file, added StyleCop.Analyzers - Removed backend test tasks and legacy test projects - Updated Electron integration: frontend now communicates with backend via JSON-RPC IPC, removed all SignalR dependencies - Updated all documentation, guides, and developer workflow to reflect new architecture and APIs - Misc: deleted all code from old Application/Domain/Infrastructure layers, modernized C# usage, improved API docs
- Remove old projects: Configs, Jobs, Generating, Scanning; add Features and Services for domain and application logic - Move config, job, generation, and scanning code to new Features and Services projects with updated namespaces - Refactor generation models (SlidesGenerateRequest, Info/Configs separation) and scanning models/services - Update Ipc project references and endpoints to use new structure - Revise solution and project files to reflect new dependencies - Update architecture docs to describe new boundaries and rules - No business logic changes; safe refactor for maintainability and clarity
- Replace Emgu.CV with OpenCvSharp4 for cross-platform image processing; update .csproj with platform-specific runtime packages - Refactor DI setup to use IFaceDetectorModelFactory and IFaceDetectorModelProvider for improved abstraction - Update namespaces to feature-based structure for better modularity - Refactor GenerateService to use IFaceDetectorModelProvider, simplifying face detector management - Update ROI calculation logic to use feature-based calculator pattern - Add configuration and IDE support files (.gitignore, .name, betterCommentsSettings.xml, etc.) - Update architecture.md to clarify feature-based backend and shared contracts usage
- Move backend projects from src/ to backend/ root; update solution, Dockerfile, launch configs, and build scripts - Refactor architecture to feature-based layout, moving away from strict Clean Architecture rings - Rewrite and clarify backend documentation (architecture, config, job system, usage) in both English and Vietnamese - Implement stdio-based JSON-RPC 2.0 API: health, job management, slide/sheet scan, config endpoints - Redesign job system: Book/Sheet/Row hierarchy, SQLite persistence, crash recovery, concurrency control - Add feature-based services for job orchestration, YAML config management, slide generation, remote image download, scan operations - Integrate Elsa workflows for job snapshot persistence - Update CI/CD pipeline to auto-detect and conditionally run backend/frontend tests - Update project files and dependencies for .NET 10.0 - Remove unused/legacy configs and files; update .gitmodules for new submodule path - Reformat code and add XML documentation for maintainability - Update Vietnamese docs to match new backend structure and workflow
- Split codebase into Application and Domain layers for clarity - Move configs, download, and job/task models to Domain - Move scanning logic and models to Application - Update IPC endpoints to use new models and enums - Replace old Services/Features projects with new structure - Add Elsa workflow activities for presentation/workbook handling - Improve configuration and download systems for robustness - Remove obsolete code and update solution/project references - Enhance code organization, naming, and documentation
- Replace all references from electronAPI to desktopAPI in the source code (for Tauri compatibility). - Update configuration files and add necessary files for Tauri. - Add Tauri functions to handle requests such as opening files, saving files, and window management.
- Changed launch configurations to use `node-terminal` and simplified commands. - Added new tasks for frontend and backend development in `tasks.json`. - Updated `Taskfile.yml` to define new development tasks for frontend and backend. - Modified `.env.example` to use `localhost` instead of `127.0.0.1`. - Updated version numbers in `package.json` and `package-lock.json` to `2.0.0`. - Adjusted `tauri.conf.json` to use the new build command. - Changed placeholder text in `ServerTab.tsx` from `127.0.0.1` to `localhost`. - Updated backend URL normalization logic to use `localhost`. - Added a new method in `RpcChannelClient` for backend requests. - Created a `.gitkeep` file in the backend resources directory.
There was a problem hiding this comment.
Pull request overview
This PR migrates the desktop runtime from Electron/SignalR to a Tauri v2 + stdio JSON-RPC architecture, updating frontend bridge calls and introducing a new backend IPC host.
Changes:
- Replaces
window.electronAPIusage withwindow.desktopAPIand updates related UI defaults (e.g.,localhosthost placeholder). - Adds Tauri v2 runtime scaffolding (
src-tauri) and updates build/release automation to package via Tauri. - Reworks backend entrypoint to a
StreamJsonRpcstdio host (SlideGenerator.Ipc) and removes the old ASP.NET/SignalR-oriented structure.
Reviewed changes
Copilot reviewed 285 out of 330 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/features/settings/utils/index.ts | Adjusts backend URL normalization behavior for localhost. |
| frontend/src/features/settings/hooks/useSettings.ts | Switches restart/backend folder selection calls to window.desktopAPI. |
| frontend/src/features/results/hooks/useResults.ts | Switches open path calls to window.desktopAPI. |
| frontend/src/features/process/hooks/useProcess.ts | Switches base URL import to rpc client and uses window.desktopAPI for external open. |
| frontend/src/app/App.tsx | Uses window.desktopAPI.onNavigate for menu navigation events. |
| frontend/src-tauri/tauri.conf.json | Introduces Tauri v2 config (schema v2) and CSP/bundling settings. |
| backend/SlideGenerator.Ipc/Program.cs | Adds JSON-RPC stdio host and wires DI services for backend runtime. |
| backend/SlideGenerator.Ipc/Endpoints/RpcEndpoint*.cs | Adds JSON-RPC methods for health/scan/jobs/configs plus jobs.updated notification. |
| backend/SlideGenerator.Domain/Configs/Services/ConfigManager.cs | Adds YAML-backed config manager for new backend structure. |
| backend/SlideGenerator.Domain/Tasks/Models/OutputExtension.cs | Adds output extension mapping helpers for PPTX/POTX. |
| backend/SlideGenerator.Application/Scanning/Services/ScanService.cs | Adds PPTX/Excel scanning helpers for RPC endpoints. |
| frontend/.env.example | Adds example env vars (currently still SignalR hub paths). |
| frontend/docs/** | Updates docs to reference Tauri v2 and JSON-RPC instead of Electron/SignalR. |
| .github/workflows/release.yml | Switches CI release packaging from Electron build to Tauri action. |
Files not reviewed (9)
- backend/.idea/.idea.SlideGenerator/.idea/.gitignore: Language not supported
- backend/.idea/.idea.SlideGenerator/.idea/.name: Language not supported
- backend/.idea/.idea.SlideGenerator/.idea/betterCommentsSettings.xml: Language not supported
- backend/.idea/.idea.SlideGenerator/.idea/copilot.data.migration.agent.xml: Language not supported
- backend/.idea/.idea.SlideGenerator/.idea/copilot.data.migration.ask.xml: Language not supported
- backend/.idea/.idea.SlideGenerator/.idea/copilot.data.migration.ask2agent.xml: Language not supported
- backend/.idea/.idea.SlideGenerator/.idea/discord.xml: Language not supported
- backend/.idea/.idea.SlideGenerator/.idea/encodings.xml: Language not supported
- backend/.idea/.idea.SlideGenerator/.idea/indexLayout.xml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| serviceProvider.GetRequiredService<ConfigManager>()); | ||
| services.AddSingleton<JobSnapshotWorkflowDispatcher>(); | ||
| services.AddSingleton<DownloadService>(); | ||
| services.AddSingleton<IFaceDetectorModelFactory, IFaceDetectorModelFactory>(); |
There was a problem hiding this comment.
AddSingleton<IFaceDetectorModelFactory, IFaceDetectorModelFactory>() attempts to register an interface as its own implementation and will fail at runtime (and likely at build-time). Register the correct concrete factory type (e.g., FaceDetectorModelFactory) as the implementation, or use an explicit factory lambda if the concrete type needs construction parameters.
| services.AddSingleton<IFaceDetectorModelFactory, IFaceDetectorModelFactory>(); | |
| services.AddSingleton<IFaceDetectorModelFactory, FaceDetectorModelFactory>(); |
| using SlideGenerator.Domain.Configs.Contracts; | ||
| using SlideGenerator.Domain.Configs.Entities; | ||
| using SlideGenerator.Domain.Configs.Services; |
There was a problem hiding this comment.
These namespaces don’t match the newly added Domain config surface in this PR (SlideGenerator.Domain.Configs.Contacts and SlideGenerator.Domain.Configs.Models). As written, this is very likely a compile break (wrong namespace/type locations). Update the using statements (and any referenced types like Config) to the actual namespaces introduced by the new Domain files.
| using SlideGenerator.Domain.Configs.Contracts; | |
| using SlideGenerator.Domain.Configs.Entities; | |
| using SlideGenerator.Domain.Configs.Services; | |
| using SlideGenerator.Domain.Configs.Contacts; | |
| using SlideGenerator.Domain.Configs.Models; |
| { | ||
| try | ||
| { | ||
| Directory.CreateDirectory(Path.GetPathRoot(ConfigFilePath)!); |
There was a problem hiding this comment.
Path.GetPathRoot(ConfigFilePath) returns only the drive/root (e.g., C:\ or /), not the directory containing the config file, so it won’t create the expected folder structure (and is effectively a no-op in many cases). Use Path.GetDirectoryName(ConfigFilePath) (and guard null) to ensure the config file’s parent directory exists before writing.
| Directory.CreateDirectory(Path.GetPathRoot(ConfigFilePath)!); | |
| var directory = Path.GetDirectoryName(ConfigFilePath); | |
| if (!string.IsNullOrEmpty(directory)) | |
| { | |
| Directory.CreateDirectory(directory); | |
| } |
| public static class OutputExtensionExtensions | ||
| { | ||
| extension(OutputExtension extension) | ||
| { | ||
| public string ToFileExtension() | ||
| { | ||
| return extension switch | ||
| { | ||
| OutputExtension.Potx => ".potx", | ||
| OutputExtension.Pptx => ".pptx", | ||
| _ => throw new ArgumentOutOfRangeException(nameof(extension), extension, null) | ||
| }; | ||
| } | ||
|
|
||
| public PresentationDocumentType ToPresentationDocumentType() | ||
| { | ||
| return extension switch | ||
| { | ||
| OutputExtension.Potx => PresentationDocumentType.Template, | ||
| OutputExtension.Pptx => PresentationDocumentType.Presentation, | ||
| _ => throw new ArgumentOutOfRangeException(nameof(extension), extension, null) | ||
| }; | ||
| } | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
This uses the new extension(...) { ... } syntax, which requires a very recent/preview C# language feature and will not compile under typical C# 12/13 configurations unless explicitly enabled. Convert these to standard extension methods (public static string ToFileExtension(this OutputExtension extension)) to keep compilation stable.
| .ToList(); | ||
| var imageIds = slidePart.GetImageShapeIds().ToList(); | ||
|
|
||
| result.Add(new SlideInfo(result.Count, mustaches, imageIds)); |
There was a problem hiding this comment.
SlideInfo.Index is documented as 1-based, but result.Count is 0-based at insertion time, so the first slide will be indexed as 0. Use result.Count + 1 (or maintain an explicit 1-based counter) to match the contract.
| result.Add(new SlideInfo(result.Count, mustaches, imageIds)); | |
| result.Add(new SlideInfo(result.Count + 1, mustaches, imageIds)); |
|
|
||
| private static async Task<bool> CheckImageUri(Uri uri, HttpClient httpClient) | ||
| { | ||
| var response = await httpClient.GetAsync(uri, HttpCompletionOption.ResponseHeadersRead); |
There was a problem hiding this comment.
The HttpResponseMessage is not disposed, which can leak sockets/resources under load. Ensure the response is disposed (e.g., using var response = ...) and consider using a HEAD request if only headers are needed.
| var response = await httpClient.GetAsync(uri, HttpCompletionOption.ResponseHeadersRead); | |
| using var response = await httpClient.GetAsync(uri, HttpCompletionOption.ResponseHeadersRead); |
| "plugins": { | ||
| "updater": { | ||
| "active": false, | ||
| "endpoints": [], | ||
| "pubkey": "" | ||
| } | ||
| } |
There was a problem hiding this comment.
Multiple docs files updated in this PR state the app now uses the Tauri updater plugin, but the shipped config disables it (active: false) and has empty endpoints/pubkey. Either enable/configure the updater to match the documentation, or update the documentation to reflect that updater support is not yet active.
| VITE_SHEET_HUB_PATH=/hubs/sheet | ||
| VITE_JOB_HUB_PATH=/hubs/job | ||
| VITE_CONFIG_HUB_PATH=/hubs/config |
There was a problem hiding this comment.
This .env.example still documents SignalR hub path variables (*_HUB_PATH), but the PR’s README now documents JSON-RPC channel variables (VITE_*_RPC_CHANNEL). Update .env.example to match the new runtime configuration surface to prevent misconfiguration for new contributors.
| VITE_SHEET_HUB_PATH=/hubs/sheet | |
| VITE_JOB_HUB_PATH=/hubs/job | |
| VITE_CONFIG_HUB_PATH=/hubs/config | |
| VITE_SHEET_RPC_CHANNEL=sheet | |
| VITE_JOB_RPC_CHANNEL=job | |
| VITE_CONFIG_RPC_CHANNEL=config |
| const hasScheme = /^https?:\/\//i.test(trimmedHost); | ||
| const base = hasScheme ? trimmedHost : `http://${trimmedHost}`; | ||
| const normalizedHost = base.replace(/^(https?:\/\/)localhost(?=[:/]|$)/i, '$1127.0.0.1'); | ||
| const normalizedHost = base.replace(/^(https?:\/\/)localhost(?=[:/]|$)/i, '$1localhost'); |
There was a problem hiding this comment.
The replace(..., '$1localhost') is a no-op (it rewrites localhost to localhost). If the old intent was to normalize localhost to 127.0.0.1 (or similar), restore that behavior; otherwise remove the redundant replacement to simplify the function and avoid confusion.
| - Read and follow [Constructon](../construction.md) before making architectural decisions. | ||
| - If `copilot-instructions.md` and `Constructon` differ, prefer `Constructon` for project-specific architecture/runtime rules. |
There was a problem hiding this comment.
Correct spelling of 'Constructon' to 'construction' for the document name/reference.
| - Read and follow [Constructon](../construction.md) before making architectural decisions. | |
| - If `copilot-instructions.md` and `Constructon` differ, prefer `Constructon` for project-specific architecture/runtime rules. | |
| - Read and follow [Construction](../construction.md) before making architectural decisions. | |
| - If `copilot-instructions.md` and `construction.md` differ, prefer `construction.md` for project-specific architecture/runtime rules. |
No description provided.