Conversation
Pure Rust shared library with 155 questions across 15 categories (EULUMDAT, IES, symmetry, coordinates, calculations, BUG rating, UGR/glare, color science, horticultural, BIM, modern formats, validation, units, diagrams, standards). FFI-safe types for uniffi/PyO3 cross-platform use. Optional serde feature.
…g category - Add eulumdat-tui-quiz crate: ratatui terminal quiz with braille polar diagrams, half-block heatmap renderer, config/quiz/results screens, language picker (F2), clap CLI with --lang/--help - Add eulumdat-wasm-quiz crate: Leptos web quiz with SVG diagrams, dark/light theme, per-question diagram routing for DiagramReading - Add DiagramReading category (20 questions): polar diagram interpretation (12) and heatmap reading (8) with per-question diagram selection - Add pitfalls category, quiz i18n with 195 questions in 8 languages - Add heatmap UI string to QuizUiStrings (optional, backward-compatible) - Add compare module, BIM/schema validation, template improvements - Update eulumdat-i18n with compare/validation/BIM translation keys
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR introduces comprehensive photometric comparison, BIM data visualization, and schema validation across Swift and Rust platforms. New diagram types (Isolux, Isocandela, Floodlight) with C-plane rotation support are added. Localization is expanded with structured validation codes and comparison metrics. Multiple template resources and a new quiz module are included. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CompareUI as Compare UI<br/>(Swift/egui)
participant FileLoader as File Loader
participant PhotometricEngine as Photometric<br/>Comparison Engine
participant SVGRenderer as SVG Diagram<br/>Renderer
participant MetricsCalculator as Metrics &<br/>Similarity Calc.
User->>CompareUI: Load File A (LDT/IES)
User->>CompareUI: Load File B (LDT/IES/XML)
CompareUI->>FileLoader: Parse File A & B
FileLoader-->>CompareUI: Eulumdat objects
User->>CompareUI: Select Compare Mode (Overlay/Side-by-Side)
User->>CompareUI: Select C-Plane (optional)
CompareUI->>PhotometricEngine: compare_photometric(A, B, mode)
PhotometricEngine->>MetricsCalculator: Calculate metrics<br/>(flux, LOR, BUG, etc.)
MetricsCalculator-->>PhotometricEngine: Metrics & similarity
PhotometricEngine-->>CompareUI: ComparisonResult
CompareUI->>SVGRenderer: generate_overlay_svg(A, B, mode)
SVGRenderer-->>CompareUI: SVG string
CompareUI-->>User: Render diagrams +<br/>metrics table
User->>CompareUI: Export CSV/PDF
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds localized validation/comparison support and new tooling/UI capabilities (comparison, BIM, schema validation, new diagram types, and conversion rotation), plus accompanying locale and template updates.
Changes:
- Expanded i18n locale structures to support code-keyed validation messages and comparison metric translations.
- Added new UniFFI exports for localized validation, photometric comparison, BIM parameter extraction, and schema validation.
- Introduced new diagrams (isolux/isocandela/floodlight), compare UI surface areas, CLI C-plane rotation option, and new app templates/resources.
Reviewed changes
Copilot reviewed 52 out of 141 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/eulumdat-quiz/Cargo.toml | Adds a new eulumdat-quiz crate manifest (quiz engine scaffold). |
| crates/eulumdat-i18n/src/lib.rs | Extends locale model (comparison + validation messages) and adds lookup/templating helpers. |
| crates/eulumdat-i18n/locales/zh.json | Updates validation messages to keyed codes; adds compare/report UI strings and comparison metrics. |
| crates/eulumdat-i18n/locales/ru.json | Updates validation messages to keyed codes; adds compare/report UI strings and comparison metrics. |
| crates/eulumdat-i18n/locales/pt-BR.json | Updates validation messages to keyed codes; adds compare/report UI strings and comparison metrics. |
| crates/eulumdat-i18n/locales/it.json | Updates validation messages to keyed codes; adds compare/report UI strings and comparison metrics. |
| crates/eulumdat-i18n/locales/fr.json | Updates validation messages to keyed codes; adds compare/report UI strings and comparison metrics. |
| crates/eulumdat-i18n/locales/es.json | Updates validation messages to keyed codes; adds compare/report UI strings and comparison metrics. |
| crates/eulumdat-i18n/locales/en.json | Updates validation messages to keyed codes; adds compare/report UI strings and comparison metrics. |
| crates/eulumdat-i18n/locales/de.json | Updates validation messages to keyed codes; adds compare/report UI strings and comparison metrics. |
| crates/eulumdat-ffi/src/validation.rs | Adds localized validation/warnings/errors FFI entrypoints. |
| crates/eulumdat-ffi/src/schema_validation.rs | Adds schema validation FFI surface (S001/TM33/TM32). |
| crates/eulumdat-ffi/src/lib.rs | Re-exports new FFI modules and adds new diagram exports. |
| crates/eulumdat-ffi/src/diagram.rs | Adds isolux/isocandela/floodlight + per-plane and overlay SVG generation functions. |
| crates/eulumdat-ffi/src/compare.rs | Adds photometric comparison result types + localized comparison export. |
| crates/eulumdat-ffi/src/bim.rs | Adds BIM extraction/availability APIs and data records for FFI consumers. |
| crates/eulumdat-egui/src/ui/tabs.rs | Adds Compare main tab and new subtabs (isolux/isocandela/floodlight/BIM/compare panel). |
| crates/eulumdat-egui/src/ui/diagram_panel.rs | Adds new diagram types & params and refines diagram centering/layout behavior. |
| crates/eulumdat-cli/src/main.rs | Wires rotate into convert command execution. |
| crates/eulumdat-cli/src/commands.rs | Implements optional C-plane rotation during conversion (import/export). |
| crates/eulumdat-cli/src/cli.rs | Adds --rotate flag to conversion CLI. |
| EulumdatApp/EulumdatApp/Templates.swift | Adds IES template support and many new templates; improves bundle reading encoding handling. |
| EulumdatApp/EulumdatApp/SchemaValidationView.swift | Adds a new schema validation + localized LDT/IES validation UI. |
| EulumdatApp/EulumdatApp/Room3DView.swift | Optimizes 3D scene rebuilds by introducing a scene-change key. |
| EulumdatApp/EulumdatApp/Resources/Templates/tm-33-23_with_custom_data.xml | Adds TM-33-23 custom data template resource. |
| EulumdatApp/EulumdatApp/Resources/Templates/tm-33-23_uv_supplemental.xml | Adds TM-33-23 UV supplemental template resource. |
| EulumdatApp/EulumdatApp/Resources/Templates/tm-33-23_seedling_propagation.xml | Adds TM-33-23 seedling/clone template resource. |
| EulumdatApp/EulumdatApp/Resources/Templates/tm-33-23_minimal.xml | Adds minimal TM-33-23 template resource. |
| EulumdatApp/EulumdatApp/Resources/Templates/tm-33-23_horticultural_led.xml | Adds TM-33-23 horticultural LED template resource. |
| EulumdatApp/EulumdatApp/Resources/Templates/tm-33-23_far_red_supplemental.xml | Adds TM-33-23 far-red supplemental template resource. |
| EulumdatApp/EulumdatApp/Resources/Templates/tm-32-24_office_downlight_bim.xml | Adds TM-32-24 BIM parameters sample template resource. |
| EulumdatApp/EulumdatApp/Resources/Templates/4058075580695_FL_MAX_LUM_1200W_757_ASYM_50X110WAL.ldt | Adds floodlight LDT template resource. |
| EulumdatApp/EulumdatApp/Resources/Templates/4058075580688_FL_MAX_LUM_1200W_757_SYM_60_WAL.ldt | Adds floodlight LDT template resource. |
| EulumdatApp/EulumdatApp/Resources/Templates/4058075580671_FL_MAX_LUM_1200W_757_SYM_30_WAL.ldt | Adds floodlight LDT template resource. |
| EulumdatApp/EulumdatApp/Resources/Templates/4058075580657_FL_MAX_LUM_900W_757_ASYM_50X110_WAL.ldt | Adds floodlight LDT template resource. |
| EulumdatApp/EulumdatApp/Resources/Templates/4058075580640_FL_MAX_LUM_900W_757_SYM_60_WAL.ldt | Adds floodlight LDT template resource. |
| EulumdatApp/EulumdatApp/Resources/Templates/4058075580602_FL_MAX_LUM_600W_757_SYM_60_WAL.ldt | Adds floodlight LDT template resource. |
| EulumdatApp/EulumdatApp/EulumdatApp.swift | Adds compare viewer window and shared compare window model. |
| EulumdatApp/EulumdatApp/BimPanelView.swift | Adds BIM panel UI with export/share actions. |
| Cargo.toml | Adds workspace deps (rfd, toml) to support new features. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+479
to
+535
| /// Validation message translations keyed by code (W001–W046, E001–E006). | ||
| /// Messages may contain `{0}`, `{1}`, … placeholders for dynamic values. | ||
| #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] | ||
| pub struct ValidationMessages { | ||
| pub missing_manufacturer: String, | ||
| pub missing_flux: String, | ||
| pub invalid_angles: String, | ||
| pub intensity_mismatch: String, | ||
| pub symmetry_violation: String, | ||
| pub struct ValidationMessageLocale { | ||
| pub w001: String, | ||
| pub w002: String, | ||
| pub w003: String, | ||
| pub w004: String, | ||
| pub w005: String, | ||
| pub w006: String, | ||
| pub w007: String, | ||
| pub w008: String, | ||
| pub w009: String, | ||
| pub w010: String, | ||
| pub w011: String, | ||
| pub w012: String, | ||
| pub w013: String, | ||
| pub w014: String, | ||
| pub w015: String, | ||
| pub w016: String, | ||
| pub w017: String, | ||
| pub w018: String, | ||
| pub w019: String, | ||
| pub w020: String, | ||
| pub w021: String, | ||
| pub w022: String, | ||
| pub w023: String, | ||
| pub w024: String, | ||
| pub w025: String, | ||
| pub w026: String, | ||
| pub w027: String, | ||
| pub w028: String, | ||
| pub w029: String, | ||
| pub w030: String, | ||
| pub w031: String, | ||
| pub w032: String, | ||
| pub w033: String, | ||
| pub w034: String, | ||
| pub w035: String, | ||
| pub w036: String, | ||
| pub w037: String, | ||
| pub w038: String, | ||
| pub w039: String, | ||
| pub w040: String, | ||
| pub w041: String, | ||
| pub w042: String, | ||
| pub w043: String, | ||
| pub w044: String, | ||
| pub w045: String, | ||
| pub w046: String, | ||
| pub w047: String, | ||
| pub e001: String, | ||
| pub e002: String, | ||
| pub e003: String, | ||
| pub e004: String, | ||
| pub e005: String, | ||
| pub e006: String, |
There was a problem hiding this comment.
The doc comment says validation warnings are keyed W001–W046, but the struct includes w047 (and Locale::validation_message matches W047). Update the comment to reflect the actual supported range (likely W001–W047).
Comment on lines
+42
to
+51
| let populated_count = rows.len() as u32; | ||
| let summary = format!("{} parameters populated", populated_count); | ||
|
|
||
| BimData { | ||
| populated_count, | ||
| summary, | ||
| rows, | ||
| csv: bim.to_csv(), | ||
| text_report: bim.to_text_report(), | ||
| } |
There was a problem hiding this comment.
BimData already includes populated_count, but the FFI also returns an English summary string derived from it. This makes localization hard for app/UI consumers and duplicates data. Consider removing summary from the FFI record (or making it optional) and letting clients format a localized summary from populated_count.
Comment on lines
+105
to
+112
| LabeledContent("Luminaire", value: ldt.luminaireName) | ||
| LabeledContent("Manufacturer", value: ldt.identification) | ||
| LabeledContent("Type", value: typeDescription) | ||
| LabeledContent("Symmetry", value: symmetryDescription) | ||
| LabeledContent("C-Planes", value: "\(ldt.numCPlanes)") | ||
| LabeledContent("G-Angles", value: "\(ldt.numGPlanes)") | ||
| LabeledContent("Max Intensity", value: String(format: "%.1f cd/klm", ldt.maxIntensity)) | ||
| LabeledContent("Total Flux", value: String(format: "%.0f lm", ldt.totalLuminousFlux)) |
There was a problem hiding this comment.
This view introduces several user-visible hardcoded English strings while the rest of the UI uses L10n.string(...) (e.g., \"Luminaire\", \"Manufacturer\", \"C-Planes\", etc.). These should be moved into the localization system to keep language switching consistent (also check other new literals in this file like status text and disclosure titles).
Suggested change
| LabeledContent("Luminaire", value: ldt.luminaireName) | |
| LabeledContent("Manufacturer", value: ldt.identification) | |
| LabeledContent("Type", value: typeDescription) | |
| LabeledContent("Symmetry", value: symmetryDescription) | |
| LabeledContent("C-Planes", value: "\(ldt.numCPlanes)") | |
| LabeledContent("G-Angles", value: "\(ldt.numGPlanes)") | |
| LabeledContent("Max Intensity", value: String(format: "%.1f cd/klm", ldt.maxIntensity)) | |
| LabeledContent("Total Flux", value: String(format: "%.0f lm", ldt.totalLuminousFlux)) | |
| LabeledContent(L10n.string("schema.fileInfo.luminaire", language: appLanguage), value: ldt.luminaireName) | |
| LabeledContent(L10n.string("schema.fileInfo.manufacturer", language: appLanguage), value: ldt.identification) | |
| LabeledContent(L10n.string("schema.fileInfo.type", language: appLanguage), value: typeDescription) | |
| LabeledContent(L10n.string("schema.fileInfo.symmetry", language: appLanguage), value: symmetryDescription) | |
| LabeledContent(L10n.string("schema.fileInfo.cPlanes", language: appLanguage), value: "\(ldt.numCPlanes)") | |
| LabeledContent(L10n.string("schema.fileInfo.gAngles", language: appLanguage), value: "\(ldt.numGPlanes)") | |
| LabeledContent(L10n.string("schema.fileInfo.maxIntensity", language: appLanguage), value: String(format: "%.1f cd/klm", ldt.maxIntensity)) | |
| LabeledContent(L10n.string("schema.fileInfo.totalFlux", language: appLanguage), value: String(format: "%.0f lm", ldt.totalLuminousFlux)) |
Comment on lines
296
to
+306
| /// File extension for this template | ||
| var fileExtension: String { | ||
| format == .atlaXml ? "xml" : "ldt" | ||
| switch self { | ||
| case .aecItalo, .aecMaxwellCie: return "IES" | ||
| case .aecMaxwellIesna: return "ies" | ||
| default: | ||
| switch format { | ||
| case .atlaXml: return "xml" | ||
| case .ies: return "ies" | ||
| case .ldt: return "ldt" | ||
| } |
There was a problem hiding this comment.
Returning mixed-case extensions ("IES" vs "ies") is inconsistent and can break resource lookup on case-sensitive filesystems/bundles. Prefer normalizing to lowercase for all cases (and keep the actual bundled filenames/extensions consistent).
…icts Accept our branch changes, deduplicate workspace deps (toml, rfd).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
All tests passed, as well i tried online and in the tui
Summary by CodeRabbit
New Features
Documentation