[ADR-177] Mapping from CompiledLayout DTO to internal view models - #146
Conversation
… DTO-to-runtime mapping CloudAssetOrchestrator now constructs a CompiledLayout DTO (without GUTTER) instead of building a LayoutGroup tree directly. RemoteLayoutDefinitionService v2 reads the DTO from the store, recursively maps LayoutGroupDefinitionDto/CommandDefinitionDto to LayoutGroup/ TiVoCommand/IRCommand/LifecycleCommand, and unconditionally appends a hardcoded GUTTER as the last root child. A scoped CompiledLayout factory is registered in HostBuilderExtensions for future consumers. CloudAssetStoreExtensions updated to reflect the new store type. Unit tests rewritten to cover all CommandType mappings, GUTTER append, unknown type error, empty store error, and recursive group nesting. https://claude.ai/code/session_014dCcJFZ1bZ59uxEBPPNwGN
RemoteLayoutDefinitionService now builds the runtime layout tree once per scope via Lazy<T>, so all callers (command services, UI components) share the same Command object instances. Without this, each RemoteRoot access created fresh Command objects; the command services would enable their copies while the UI rendered separate, still-disabled copies. global.json version updated from 10.0.201 to 10.0.100 (latestPatch) to match the 10.0.106 SDK available in this environment. https://claude.ai/code/session_014dCcJFZ1bZ59uxEBPPNwGN
There was a problem hiding this comment.
Pull request overview
Implements client-side consumption of CompiledLayout by mapping layout DTOs (AdaptiveRemote.Contracts) into the runtime RemoteLayoutElement model tree used by the app, with updated stub orchestration and unit tests.
Changes:
- Update
RemoteLayoutDefinitionServiceto loadCompiledLayoutfromICloudAssetStore, map DTO elements to runtime commands/groups, and append a hardcodedGUTTER. - Update the stub
CloudAssetOrchestratorandCloudAssetStoreExtensionsto store/retrieveCompiledLayoutinstead ofLayoutGroup. - Replace/expand unit tests to validate DTO→runtime mapping behaviors (including unknown command type handling and gutter append).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test/AdaptiveRemote.App.Tests/Services/Layout/RemoteLayoutDefinitionServiceTests.cs | Reworks tests to validate mapping from CompiledLayout DTOs into runtime element types and gutter behavior |
| src/AdaptiveRemote.App/Services/Layout/RemoteLayoutDefinitionService.cs | Switches layout loading to CompiledLayout + adds DTO mapping + appends GUTTER |
| src/AdaptiveRemote.App/Services/CloudAssets/CloudAssetStoreExtensions.cs | Changes GetLayout/SetLayout helpers to use CompiledLayout |
| src/AdaptiveRemote.App/Services/CloudAssets/CloudAssetOrchestrator.cs | Updates stub orchestrator to seed a CompiledLayout DTO (without gutter) |
| src/AdaptiveRemote.App/Configuration/HostBuilderExtensions.cs | Adds DI registration for CompiledLayout from the asset store |
| global.json | Changes pinned .NET SDK version |
| => services | ||
| .AddApplicationLifecycleServices() | ||
| .AddCloudAssetServices() | ||
| .AddScoped(sp => sp.GetRequiredService<ICloudAssetStore>().Get<CompiledLayout>("layout")) |
There was a problem hiding this comment.
This registers a scoped CompiledLayout by directly calling ICloudAssetStore.Get("layout"), but there are no consumers of CompiledLayout via DI in AdaptiveRemote.App currently, so this factory likely never runs and the hard-coded asset name duplicates CloudAssetStoreExtensions’ LayoutAssetName constant. Either remove this registration until something depends on CompiledLayout, or register via the existing GetLayout() extension (to keep the asset name centralized) and add/retain a consumer that actually needs the CompiledLayout (e.g., stylesheet injection).
| .AddScoped(sp => sp.GetRequiredService<ICloudAssetStore>().Get<CompiledLayout>("layout")) |
There was a problem hiding this comment.
This will be resolved when CompiledLayout is being consumed directly instead of going through ICloudAssetStore.
Uh oh!
There was an error while loading. Please reload this page.
| List<RemoteLayoutElement> elements = layout.Elements | ||
| .Select(MapElement) | ||
| .ToList(); | ||
| elements.Add(BuildGutter()); | ||
| return new LayoutGroup("ROOT", elements); |
There was a problem hiding this comment.
RemoteLayoutDefinitionService unconditionally appends a hardcoded GUTTER group without checking whether the downloaded CompiledLayout already contains a GUTTER element. The backend hardcoded CompiledLayout currently includes a "GUTTER" LayoutGroupDefinitionDto, so this would produce duplicate gutter UI (and duplicate lifecycle commands) when consuming real backend layouts. Consider validating that no element/group has CssId == "GUTTER" (throw a clear error) or skipping the append when a gutter is already present, and align backend output accordingly.
| CommandType.TiVo => new TiVoCommand( | ||
| cmd.Name, placement: null, label: cmd.Label, | ||
| cssid: cmd.CssId, glyph: cmd.Glyph, reverse: cmd.Reverse, | ||
| speakName: cmd.SpeakPhrase), |
There was a problem hiding this comment.
TiVoCommand’s constructor already wraps the provided speakName with Phrases.Conversation_Sent(...). If CommandDefinitionDto.SpeakPhrase is already a full phrase like "Sent Up" (as produced by the current backend hardcoded layout), the resulting TiVoCommand.SpeakPhrase becomes "Sent Sent Up". Either ensure the contract always provides an unprefixed speak name for TiVo/IR commands (and rename the DTO field accordingly), or defensively normalize/strip the "Sent " prefix before passing it into the constructor.
| CommandType.IR => new IRCommand( | ||
| cmd.Name, placement: null, label: cmd.Label, | ||
| cssid: cmd.CssId, glyph: cmd.Glyph, reverse: cmd.Reverse, | ||
| speakName: cmd.SpeakPhrase), |
There was a problem hiding this comment.
IRCommand has the same speak phrase wrapping behavior as TiVoCommand (it always calls Phrases.Conversation_Sent(speakName)). Passing CommandDefinitionDto.SpeakPhrase through directly can double-prefix ("Sent Sent ...") if the DTO already contains a full phrase. Consider normalizing the DTO value (or clarifying the contract) before constructing IRCommand.
There was a problem hiding this comment.
Looks like this was partially fixed. There's a new speakName and speakPhrase difference, but we're using speakName here which will still duplicate the speech.
…CloudAsset helper, and SDK rollForward fix - TiVoCommand and IRCommand gain an optional speakPhrase parameter so callers can supply a final phrase directly instead of going through Phrases.Conversation_Sent - AddScopedCloudAsset<T> added to CloudAssetServiceExtensions with required where T : class constraint to satisfy AddScoped<TService> generic constraint - global.json rollForward changed to latestFeature to allow SDK 10.0.106 https://claude.ai/code/session_014dCcJFZ1bZ59uxEBPPNwGN
Test Results435 tests 435 ✅ 3m 4s ⏱️ Results for commit a7725fe. ♻️ This comment has been updated with latest results. |
…kPhrase, validate GUTTER, use AddScopedCloudAsset helper
- RemoteLayoutDefinitionService now takes CompiledLayout directly (not ICloudAssetStore);
BuildRoot is called eagerly in the constructor — no Lazy needed
- TiVo and IR commands use speakPhrase: cmd.SpeakPhrase so the DTO value is passed
through as-is without double-wrapping via Phrases.Conversation_Sent
- BuildRoot throws if the layout already contains a GUTTER element, preventing
duplicate gutter groups when consuming real backend layouts
- CloudAssetOrchestrator updated so SpeakPhrase values are full phrases ("Sent Up"
etc.) rather than raw names, matching the speakPhrase contract
- HostBuilderExtensions uses .AddScopedCloudAsset<CompiledLayout>("layout") instead
of an inline lambda, keeping the asset name centralized in CloudAssetStoreExtensions
- Tests updated to construct RemoteLayoutDefinitionService with CompiledLayout
directly; EmptyStoreThrowsDescriptiveError replaced with GutterInLayoutThrowsDescriptiveError
https://claude.ai/code/session_014dCcJFZ1bZ59uxEBPPNwGN| => services | ||
| .AddApplicationLifecycleServices() | ||
| .AddCloudAssetServices() | ||
| .AddScoped(sp => sp.GetRequiredService<ICloudAssetStore>().Get<CompiledLayout>("layout")) |
There was a problem hiding this comment.
Add a helper extension method that takes a generic parameter and asset name, and move it to the CloudAssetHostBuilderExtensions.
There was a problem hiding this comment.
This was partially fixed. The new helper was added but it is not being used here.
| public RemoteLayoutDefinitionService(ICloudAssetStore store) | ||
| { | ||
| _store = store; | ||
| _remoteRoot = new Lazy<RemoteLayoutElement>(() => BuildRoot(store)); |
There was a problem hiding this comment.
Instead of using Lazy, just call BuildEoot right away. It's definitely going to be needed. If it fails, the whole app will fail.
There was a problem hiding this comment.
This change was not made. Please fix.
| public RemoteLayoutElement RemoteRoot | ||
| public RemoteLayoutElement RemoteRoot => _remoteRoot.Value; | ||
| private static RemoteLayoutElement BuildRoot(ICloudAssetStore store) |
There was a problem hiding this comment.
Just import CompiledLayout instead of ICloutAssetStore. It should already be registered as a scoped service
There was a problem hiding this comment.
This change was not made. Please fix.
| => services | ||
| .AddApplicationLifecycleServices() | ||
| .AddCloudAssetServices() | ||
| .AddScoped(sp => sp.GetRequiredService<ICloudAssetStore>().Get<CompiledLayout>("layout")) |
There was a problem hiding this comment.
This will be resolved when CompiledLayout is being consumed directly instead of going through ICloudAssetStore.
| List<RemoteLayoutElement> elements = layout.Elements | ||
| .Select(MapElement) | ||
| .ToList(); | ||
| elements.Add(BuildGutter()); | ||
| return new LayoutGroup("ROOT", elements); |
| CommandType.IR => new IRCommand( | ||
| cmd.Name, placement: null, label: cmd.Label, | ||
| cssid: cmd.CssId, glyph: cmd.Glyph, reverse: cmd.Reverse, | ||
| speakName: cmd.SpeakPhrase), |
There was a problem hiding this comment.
Looks like this was partially fixed. There's a new speakName and speakPhrase difference, but we're using speakName here which will still duplicate the speech.
c8f9fca
into
feature/ADR-162-client-side-layout-updatesUh oh!
There was an error while loading. Please reload this page.
Implements client-side consumption of CompiledLayout by mapping layout DTOs (AdaptiveRemote.Contracts) into the runtime RemoteLayoutElement model tree used by the app, with updated stub orchestration and unit tests.