Begin modeling pictures in MiniLcm#2300
Merged
Merged
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds picture modeling and CRUD support for ChangesPicture support on Senses
Estimated code review effort🎯 5 (Critical) | ⏱️ ~90+ minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
github-actions
Bot
added
the
💻 FW Lite
Contributor
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
github-actions
Bot
added
the
📦 Lexbox
Contributor
C# Unit Tests165 tests 165 ✅ 21s ⏱️ Results for commit 0891da0. ♻️ This comment has been updated with latest results. |
Contributor
Author
|
@CodeRabbit - Please examine how MediaUri is handled in audio files, then suggest how to handle the MediaUri property in the Picture class. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
echo "=== Search for MediaUri in the codebase ==="
rg -n "MediaUri" --type cs -C 3
echo ""
echo "=== Search for audio-related model files ==="
fd -e cs -i "audio" --exec echo {}
echo ""
echo "=== Search for Picture model files ==="
fd -e cs -i "picture" --exec echo {}
echo ""
echo "=== Search for ExampleSentence or audio model definitions ==="
rg -n "class.*Audio\|AudioFile\|Sound" --type cs -C 5Length of output: 45536 🏁 Script executed: #!/bin/bash
echo "=== Find MiniLcm model files ==="
fd -e cs . --search-path "MiniLcm" | head -60
echo ""
echo "=== Search for all MediaUri occurrences ==="
rg -rn "MediaUri" -C 5Length of output: 57201 🏁 Script executed: #!/bin/bash
echo "=== Search for PictureFileRA (LCM picture file property) ==="
rg -n "PictureFileRA\|InternalPath\|PictureFolder\|Pictures folder\|TypeToLinkedFolder" --type cs -C 3
echo ""
echo "=== Look at UpdatePictureProxy in full ==="
cat backend/FwLite/FwDataMiniLcmBridge/Api/UpdateProxy/UpdatePictureProxy.cs
echo ""
echo "=== Look at CreatePictureChange in full ==="
cat backend/FwLite/LcmCrdt/Changes/CreatePictureChange.cs
echo ""
echo "=== Look at PictureSync in full ==="
cat backend/FwLite/MiniLcm/SyncHelpers/PictureSync.cs
echo ""
echo "=== Search for TypeToLinkedFolder and AudioVisualFolder constant ==="
rg -n "TypeToLinkedFolder\|AudioVisualFolder\|PictureFolder\|Pictures\b" --type cs -C 2Length of output: 5543 🏁 Script executed: #!/bin/bash
echo "=== Find AudioVisualFolder and TypeToLinkedFolder in FwDataMiniLcmApi.cs ==="
grep -n "AudioVisual\|TypeToLinked\|PictureFile\|InternalPath\|Pictures\|LinkedFiles\|CmFile\|cmFile" \
backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs | head -60
echo ""
echo "=== View lines around the folder/type mapping function ==="
grep -n "TypeToLinkedFolder\|AudioVisualFolder\|const string\|private.*Folder" \
backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs
echo ""
echo "=== Grep for picture file path references in FW LCM ==="
rg -n "PictureFile\|CmFile\|InternalPath\|\.PictureFilePath\|picture\..*[Ff]ile\|\.ImageFile" \
backend/FwLite/FwDataMiniLcmBridge/ -C 2
echo ""
echo "=== Check what LCM picture interface looks like ==="
rg -rn "PictureFileRA\|ICmPicture\|CmPicture" --type cs -C 3 | head -80Length of output: 3346 🏁 Script executed: #!/bin/bash
echo "=== LcmToPicture method context ==="
grep -n "LcmToPicture\|private.*Picture\|FromLcmPicture\|ToMiniLcmPicture\|ICmPicture" \
backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs
echo ""
echo "=== View lines 795-830 of FwDataMiniLcmApi.cs (picture mapping) ==="
sed -n '795,835p' backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs
echo ""
echo "=== View lines 1940-1960 (TypeToLinkedFolder) ==="
sed -n '1940,1960p' backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs
echo ""
echo "=== Search for ICmPicture file-path related properties in LCM NuGet/dlls ==="
rg -rn "\.PictureFile\|picture\.\|ICmPicture\b" --type cs backend/FwLite/ -C 2 | head -60
echo ""
echo "=== Check the SIL LCModel ICmPicture or CmPicture in dlls/packages ==="
find . -name "*.cs" | xargs grep -l "ICmPicture" 2>/dev/null | head -10
echo ""
echo "=== View LfClassicMiniLcmApi picture mapping (line ~350) ==="
grep -n "picture\.\|Picture\|MediaUri\|CmFile\|InternalPath" backend/LfClassicData/LfClassicMiniLcmApi.cs | head -30Length of output: 4911 🏁 Script executed: #!/bin/bash
echo "=== Search for ICmFile usage anywhere ==="
rg -rn "ICmFile\|PictureFileRA\|InternalPath\|\.PicturePath\b" --type cs -C 3 | head -60
echo ""
echo "=== Look for how FW code sets picture file path - CreatePicture context ==="
sed -n '1810,1850p' backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs
echo ""
echo "=== Search for Pictures folder const or PictureFolder ==="
rg -rn "const.*[Pp]icture\|PictureFolder\b" --type cs -C 2
echo ""
echo "=== Look for nuget packages that contain LCModel ==="
find . -name "*.csproj" | xargs grep -l "LCModel\|liblcm\|SIL.LCModel" 2>/dev/null | head -5
echo ""
echo "=== Check any existing usages that access picture.PictureFile ==="
rg -rn "picture\." --type cs backend/FwLite/FwDataMiniLcmBridge/ -C 1 | head -40Length of output: 5205
How
|
| Step | Audio | Picture |
|---|---|---|
| FW storage | MultiString[audioWs] = relative filename |
ICmPicture.PictureFileRA.InternalPath = relative path |
| Subfolder | LinkedFilesRootDir/AudioVisual/ |
LinkedFilesRootDir/Pictures/ |
| Read (FW→MiniLcm) | mediaAdapter.MediaUriFromPath(fullPath) |
same |
| Write (MiniLcm→FW) | mediaAdapter.PathFromMediaUri() → update string |
same, but find/create ICmFile, set PictureFileRA |
| NotFound guard | UpdateDictionaryProxy.ShouldSet skips write |
SetLcmPictureFile returns early if MediaUri.NotFound |
Contributor
Author
|
Current code is partly working, but I haven't yet told Entity Framework how to map a Picture object into the DB. Which results in this exception when I run a sync: System.InvalidOperationException: The property 'Picture.MediaUri' could not be mapped because it is of type 'MediaUri', which is not a supported primitive type or a valid entity type. Either explicitly map this property, or ignore it using the '[NotMapped]' attribute or by using 'EntityTypeBuilder.Ignore' in 'OnModelCreating'.
at Microsoft.EntityFrameworkCore.Infrastructure.ModelValidator.ValidatePropertyMapping(IConventionTypeBase structuralType, IConventionModel model, IDiagnosticsLogger`1 logger)
at Microsoft.EntityFrameworkCore.Infrastructure.ModelValidator.ValidatePropertyMapping(IModel model, IDiagnosticsLogger`1 logger)
at Microsoft.EntityFrameworkCore.Infrastructure.ModelValidator.Validate(IModel model, IDiagnosticsLogger`1 logger)
at Microsoft.EntityFrameworkCore.Infrastructure.RelationalModelValidator.Validate(IModel model, IDiagnosticsLogger`1 logger)
at Microsoft.EntityFrameworkCore.Sqlite.Infrastructure.Internal.SqliteModelValidator.Validate(IModel model, IDiagnosticsLogger`1 logger)
at Microsoft.EntityFrameworkCore.Infrastructure.ModelRuntimeInitializer.Initialize(IModel model, Boolean designTime, IDiagnosticsLogger`1 validationLogger)
at Microsoft.EntityFrameworkCore.Infrastructure.ModelSource.CreateModel(DbContext context, ModelCreationDependencies modelCreationDependencies, Boolean designTime)
at Microsoft.EntityFrameworkCore.Infrastructure.ModelSource.GetModel(DbContext context, ModelCreationDependencies modelCreationDependencies, Boolean designTime)
at Microsoft.EntityFrameworkCore.Internal.DbContextServices.CreateModel(Boolean designTime)
at Microsoft.EntityFrameworkCore.Internal.DbContextServices.get_Model()
at Microsoft.EntityFrameworkCore.Infrastructure.EntityFrameworkServicesBuilder.<>c.<TryAddCoreServices>b__8_4(IServiceProvider p)
at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitCache(ServiceCallSite callSite, RuntimeResolverContext context, ServiceProviderEngineScope serviceProviderEngine, RuntimeResolverLock lockType)
at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSiteMain(ServiceCallSite callSite, TArgument argument)
at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitCache(ServiceCallSite callSite, RuntimeResolverContext context, ServiceProviderEngineScope serviceProviderEngine, RuntimeResolverLock lockType)
at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSiteMain(ServiceCallSite callSite, TArgument argument)
at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitCache(ServiceCallSite callSite, RuntimeResolverContext context, ServiceProviderEngineScope serviceProviderEngine, RuntimeResolverLock lockType)
at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSiteMain(ServiceCallSite callSite, TArgument argument)
at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitCache(ServiceCallSite callSite, RuntimeResolverContext context, ServiceProviderEngineScope serviceProviderEngine, RuntimeResolverLock lockType)
at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSiteMain(ServiceCallSite callSite, TArgument argument)
at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitCache(ServiceCallSite callSite, RuntimeResolverContext context, ServiceProviderEngineScope serviceProviderEngine, RuntimeResolverLock lockType)
at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSiteMain(ServiceCallSite callSite, TArgument argument)
at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitCache(ServiceCallSite callSite, RuntimeResolverContext context, ServiceProviderEngineScope serviceProviderEngine, RuntimeResolverLock lockType)
at Microsoft.Extensions.DependencyInjection.ServiceLookup.DynamicServiceProviderEngine.<>c__DisplayClass2_0.<RealizeService>b__0(ServiceProviderEngineScope scope)
at Microsoft.Extensions.DependencyInjection.ServiceProviderServiceExtensions.GetRequiredService[T](IServiceProvider provider)
at Microsoft.EntityFrameworkCore.Infrastructure.EntityFrameworkRelationalServicesBuilder.<>c.<TryAddCoreServices>b__3_3(IServiceProvider p)
at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitCache(ServiceCallSite callSite, RuntimeResolverContext context, ServiceProviderEngineScope serviceProviderEngine, RuntimeResolverLock lockType)
at Microsoft.Extensions.DependencyInjection.ServiceLookup.DynamicServiceProviderEngine.<>c__DisplayClass2_0.<RealizeService>b__0(ServiceProviderEngineScope scope)
at Microsoft.Extensions.DependencyInjection.ServiceLookup.ServiceProviderEngineScope.GetService(Type serviceType)
at Microsoft.EntityFrameworkCore.Infrastructure.Internal.InfrastructureExtensions.GetService(IInfrastructure`1 accessor, Type serviceType)
at Microsoft.EntityFrameworkCore.Infrastructure.Internal.InfrastructureExtensions.GetService[TService](IInfrastructure`1 accessor)
at Microsoft.EntityFrameworkCore.Infrastructure.AccessorExtensions.GetService[TService](IInfrastructure`1 accessor)
at Microsoft.EntityFrameworkCore.Infrastructure.DatabaseFacade.get_Dependencies()
at Microsoft.EntityFrameworkCore.Infrastructure.DatabaseFacade.Microsoft.EntityFrameworkCore.Storage.IDatabaseFacadeDependenciesAccessor.get_Dependencies()
at Microsoft.EntityFrameworkCore.RelationalDatabaseFacadeExtensions.GetFacadeDependencies(DatabaseFacade databaseFacade)
at Microsoft.EntityFrameworkCore.RelationalDatabaseFacadeExtensions.CloseConnectionAsync(DatabaseFacade databaseFacade)
at LcmCrdt.CrdtProjectsService.CreateProject(CreateProjectRequest request) in /src/backend/FwLite/LcmCrdt/CrdtProjectsService.cs:line 181
at LcmCrdt.CrdtProjectsService.CreateProject(CreateProjectRequest request) in /src/backend/FwLite/LcmCrdt/CrdtProjectsService.cs:line 194
at LcmCrdt.CrdtProjectsService.CreateProject(CreateProjectRequest request) in /src/backend/FwLite/LcmCrdt/CrdtProjectsService.cs:line 194
at FwHeadless.Services.SyncWorker.SetupCrdtProject(String crdtFile, IProjectLookupService projectLookupService, Guid projectId, CrdtProjectsService projectsService, String projectFolder, Guid fwProjectId, String lexboxUrl) in /src/backend/FwHeadless/Services/SyncHostedService.cs:line 371
at FwHeadless.Services.SyncWorker.ExecuteSync(CancellationToken stoppingToken, Boolean onlyHarmony) in /src/backend/FwHeadless/Services/SyncHostedService.cs:line 188
at FwHeadless.Services.SyncWorker.ExecuteSync(CancellationToken stoppingToken, Boolean onlyHarmony) in /src/backend/FwHeadless/Services/SyncHostedService.cs:line 276
at FwHeadless.Services.SyncHostedService.ExecuteAsync(CancellationToken stoppingToken) in /src/backend/FwHeadless/Services/SyncHostedService.cs:line 33 |
Contributor
Author
|
@CodeRabbit - Look at the exception in #2300 (comment) and look at the Entity Framework Core documentation. Summarize how to map the MediaUri property, and provide links to the documentation explaining how to map properties in EF Core. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
echo "=== Find MediaUri type definition ==="
rg -n "class MediaUri\|struct MediaUri\|record MediaUri" --type cs -C 5
echo ""
echo "=== Find existing EF Core value converters in LcmCrdt ==="
rg -n "ValueConverter\|HasConversion\|OnModelCreating\|EntityTypeBuilder\|modelBuilder" --type cs backend/FwLite/LcmCrdt/ -C 3 | head -100
echo ""
echo "=== Find DbContext in LcmCrdt ==="
fd -e cs . backend/FwLite/LcmCrdt/ | xargs grep -l "DbContext\|OnModelCreating" 2>/dev/null
echo ""
echo "=== Check if MediaUri implements any interface or has implicit conversion ==="
rg -n "implicit\|explicit\|operator\|ToString\|Parse\|TryParse" --type cs -l | xargs grep -l "MediaUri" 2>/dev/nullLength of output: 3199 🌐 Web query:
Length of output: 20413 🏁 Script executed: #!/bin/bash
echo "=== MediaUri definition - search all file types ==="
rg -rn "MediaUri" --type cs | grep -i "class\|record\|struct" | head -20
echo ""
echo "=== Find the file that defines MediaUri ==="
rg -rn "MediaUri" --type cs -l
echo ""
echo "=== Read LcmCrdtDbContext.cs ==="
cat backend/FwLite/LcmCrdt/LcmCrdtDbContext.cs
echo ""
echo "=== Read MiniLcm MediaUri source file ==="
fd -e cs "MediaUri" | head -5Length of output: 6757 🏁 Script executed: #!/bin/bash
echo "=== Read MediaUri.cs ==="
cat backend/FwLite/MiniLcm/Media/MediaUri.csLength of output: 1864
The fixAdd a converter class and register it in // In LcmCrdtDbContext.ConfigureConventions:
protected override void ConfigureConventions(ModelConfigurationBuilder builder)
{
builder.Properties<MultiString>()
.HaveColumnType("jsonb")
.HaveConversion<MultiStringDbConverter>();
builder.Properties<RichString?>()
.HaveColumnType("jsonb")
.HaveConversion<RichStringDbConverter>();
builder.Properties<RichMultiString>()
.HaveColumnType("jsonb")
.HaveConversion<RichMultiStringDbConverter>();
builder.Properties<WritingSystemId>()
.HaveConversion<WritingSystemIdConverter>();
// Add this:
builder.Properties<MediaUri>()
.HaveConversion<MediaUriConverter>();
}
// Add this converter class alongside the others:
private class MediaUriConverter() : ValueConverter<MediaUri, string>(
uri => uri.ToString(),
str => new MediaUri(str));
Relevant EF Core documentation
After adding the converter, you'll also need to create a new EF migration ( |
Contributor
Author
|
@CodeRabbit - When I run Please analyze the port-forwarding setup for the |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
echo "=== Find Taskfile for fw-lite-web ==="
fd -i "Taskfile" | head -20
echo ""
echo "=== Search for fw-lite-web in Taskfiles ==="
rg -n "fw-lite-web\|fw_lite_web" --type yaml -C 5
echo ""
echo "=== Search for port 3050 references ==="
rg -rn "3050" -C 3 | head -60
echo ""
echo "=== Search for port forwarding or devtunnel setup ==="
rg -rn "devtunnel\|port.forward\|portforward\|forward.*3050\|3050.*forward" -C 5 | head -60Length of output: 28033 🏁 Script executed: #!/bin/bash
echo "=== Search for OIDC/auth configuration with localhost ==="
rg -rn "localhost:3050\|localhost.*3050\|3050.*localhost" -C 3 | head -80
echo ""
echo "=== Search for OidcAuthority or Authority config ==="
rg -rn "Authority\|OidcAuthority\|OpenIdConnect\|oidc" --type cs -C 3 | head -80
echo ""
echo "=== Search for auth login endpoint ==="
rg -rn "api/auth/login\|AuthController\|login.*controller" --type cs -C 5 | head -60Length of output: 637 🏁 Script executed: #!/bin/bash
echo "=== Root Taskfile.yml - search for fw-lite-web or related ==="
grep -n "fw-lite-web\|fw_lite_web\|fw-lite\|fwlite\|5137\|3050\|https-proxy\|devtunnel\|forward" Taskfile.yml | head -60
echo ""
echo "=== backend/FwLite/Taskfile.yml - full content ==="
cat backend/FwLite/Taskfile.yml
echo ""
echo "=== frontend/https-proxy/vite.config.ts - full content ==="
cat frontend/https-proxy/vite.config.ts
echo ""
echo "=== backend/Taskfile.yml - search for fw-lite or 3050 or 5137 ==="
grep -n "fw-lite\|3050\|5137\|https-proxy\|devtunnel\|forward" backend/Taskfile.yml | head -40Length of output: 10948 🏁 Script executed: #!/bin/bash
echo "=== Search for 5137 in all files ==="
rg -rn "5137" -C 2 | head -60
echo ""
echo "=== Search for lexbox server URL config in appsettings or launchSettings ==="
fd -e json "appsettings\|launchSettings" backend/FwLite/ | head -20
fd -e json "appsettings\|launchSettings" backend/FwLite/ | xargs grep -l "localhost\|lexbox\|3050\|Authority\|oidc\|auth" 2>/dev/null | head -10
echo ""
echo "=== Search for auth login in FwLite web project ==="
rg -rn "login\|auth\|oidc\|Authority" backend/FwLite/FwLiteWeb/ -C 3 | head -80
echo ""
echo "=== Find FwLiteWeb project structure ==="
fd . backend/FwLite/FwLiteWeb/ | head -40Length of output: 14226 🏁 Script executed: #!/bin/bash
echo "=== AuthRoutes.cs ==="
cat backend/FwLite/FwLiteWeb/Routes/AuthRoutes.cs
echo ""
echo "=== Program.cs ==="
cat backend/FwLite/FwLiteWeb/Program.cs
echo ""
echo "=== appsettings.json files in FwLiteWeb ==="
fd -e json "appsettings" backend/FwLite/FwLiteWeb/ | xargs -I{} sh -c 'echo "--- {} ---"; cat {}'
echo ""
echo "=== frontend Taskfile.yml - fw-lite-infr task ==="
grep -n "fw-lite-infr\|https-proxy\|3050\|lexbox" frontend/Taskfile.yml -A 10 | head -60
echo ""
echo "=== FwLiteWebConfig.cs ==="
cat backend/FwLite/FwLiteWeb/FwLiteWebConfig.cs
echo ""
echo "=== ServerRedirectUrlProvider.cs ==="
cat backend/FwLite/FwLiteWeb/Services/ServerRedirectUrlProvider.csLength of output: 9618 🏁 Script executed: #!/bin/bash
echo "=== Find AuthService and AuthConfig ==="
fd -e cs "AuthService\|AuthConfig\|OAuthService" backend/FwLite/ | head -10
echo ""
echo "=== AuthConfig.cs ==="
fd -e cs "AuthConfig" backend/FwLite/ | xargs cat
echo ""
echo "=== AuthService.cs ==="
fd -e cs "AuthService" backend/FwLite/ | xargs cat
echo ""
echo "=== OAuthService.cs ==="
fd -e cs "OAuthService" backend/FwLite/ | xargs cat
echo ""
echo "=== UrlContext.cs ==="
cat backend/FwLite/FwLiteWeb/UrlContext.cs
echo ""
echo "=== Search for ValidateAuthority or ValidIssuers or MSAL config ==="
rg -rn "ValidateAuthority\|ValidIssuers\|PublicClientApplicationBuilder\|WithAuthority\|OidcWellKnown\|openid-configuration" --type cs -C 5 | head -80
echo ""
echo "=== Root Taskfile lines 113-120 (fw-lite-web task definition) ==="
sed -n '100,130p' Taskfile.ymlLength of output: 22755 🏁 Script executed: #!/bin/bash
echo "=== Find OAuthClientFactory and OAuthClient ==="
fd -e cs "OAuthClient" backend/FwLite/ | head -10
echo ""
echo "=== OAuthClientFactory.cs ==="
fd -e cs "OAuthClientFactory" backend/FwLite/ | xargs cat
echo ""
echo "=== OAuthClient.cs ==="
fd -e cs -x echo {} backend/FwLite/ | xargs grep -l "OAuthClient" | grep -v "Factory\|Service\|Test" | head -5 | xargs cat
echo ""
echo "=== Search for PublicClientApplication or WithAuthority in FwLiteShared ==="
rg -rn "PublicClientApplication\|WithAuthority\|BuildApplication\|IPublicClientApplication" --type cs backend/FwLite/ -C 5 | head -80
echo ""
echo "=== Search for ValidateIssuer or authority in appsettings for the local lexbox ==="
fd -e json "appsettings" backend/ | xargs grep -l "localhost\|3050\|3000\|authority\|Authority" 2>/dev/null | head -5
echo ""
echo "=== Check if local lexbox server config has ASPNETCORE_FORWARDEDHEADERS ==="
fd -i "docker-compose" . | xargs grep -l "lexbox\|3000\|3050" 2>/dev/null | head -5
fd "docker-compose" . | head -10Length of output: 60547
Root CauseThe data flow is: The chain of events that causes the mismatch:
Fix OptionsOption A — Fix in the vite proxy (self-contained, no server changes needed)Intercept the OIDC discovery response and rewrite import { gunzipSync } from 'zlib';
// Inside the lexboxServer ProxyOptions configure callback, after existing code:
proxy.on('proxyRes', (proxyRes, req, res) => {
if (!req.url?.includes('openid-configuration')) return;
const chunks: Buffer[] = [];
const originalWrite = res.write.bind(res);
const originalEnd = res.end.bind(res);
// Intercept the streamed response
proxyRes.on('data', (chunk: Buffer) => chunks.push(chunk));
proxyRes.on('end', () => {
const body = Buffer.concat(chunks).toString('utf-8');
const rewritten = body.replaceAll(
/"issuer"\s*:\s*"http:\/\//g,
'"issuer":"https://'
);
const newBody = Buffer.from(rewritten, 'utf-8');
res.setHeader('Content-Length', newBody.length);
originalWrite(newBody);
originalEnd();
});
});
A simpler alternative using vite's built-in transformer hook (if supported by your vite version): // In vite.config.ts server.proxy options:
'/': {
...lexboxServer,
// Add a response rewriter for OIDC discovery only
rewrite: (path) => path,
}Option B — Fix in the local lexbox server (correct architectural fix)Enable environment:
ASPNETCORE_FORWARDEDHEADERS_ENABLED: "true"This makes ASP.NET Core automatically enable forwarded headers, so the lexbox server generates Or in {
"ForwardedHeaders": {
"ForwardedHeaders": "XForwardedFor, XForwardedProto"
}
}Option C — Fix in
|
Contributor
C# FwHeadless Unit Tests48 tests 48 ✅ 15s ⏱️ Results for commit 0891da0. ♻️ This comment has been updated with latest results. |
Contributor
Author
|
@CodeRabbit - Syncing pictures using commit 3a729a7 produced the following exception: System.NotSupportedException: type JsonPatchChange`1 does not support NewEntity, because it inherits from EditChange, this means it must be called with a from an existing entity, not a newly generated one
at SIL.Harmony.Changes.EditChange`1.NewEntity(Commit commit, IChangeContext context) in backend/harmony/src/SIL.Harmony/Changes/EditChange.cs:line 8
at SIL.Harmony.Changes.Change`1.SIL.Harmony.Changes.IChange.NewEntity(Commit commit, IChangeContext context) in backend/harmony/src/SIL.Harmony/Changes/Change.cs:line 51
at SIL.Harmony.SnapshotWorker.ApplyCommitChanges(SortedSet`1 commits) in backend/harmony/src/SIL.Harmony/SnapshotWorker.cs:line 78
at SIL.Harmony.SnapshotWorker.UpdateSnapshots(SortedSet`1 commits) in backend/harmony/src/SIL.Harmony/SnapshotWorker.cs:line 54
at SIL.Harmony.DataModel.UpdateSnapshots(CrdtRepository repo, SortedSet`1 commitsToApply) in backend/harmony/src/SIL.Harmony/DataModel.cs:line 217
at SIL.Harmony.DataModel.SIL.Harmony.ISyncable.AddRangeFromSync(IEnumerable`1 commits) in backend/harmony/src/SIL.Harmony/DataModel.cs:line 159
at SIL.Harmony.DataModel.SIL.Harmony.ISyncable.AddRangeFromSync(IEnumerable`1 commits) in backend/harmony/src/SIL.Harmony/DataModel.cs:line 161
at SIL.Harmony.DataModel.SIL.Harmony.ISyncable.AddRangeFromSync(IEnumerable`1 commits) in backend/harmony/src/SIL.Harmony/DataModel.cs:line 161
at SIL.Harmony.SyncHelper.SyncWith(ISyncable localModel, ISyncable remoteModel, JsonSerializerOptions serializerOptions) in backend/harmony/src/SIL.Harmony/SyncHelper.cs:line 42
at SIL.Harmony.DataModel.SyncWith(ISyncable remoteModel) in backend/harmony/src/SIL.Harmony/DataModel.cs:line 377
at FwLiteShared.Sync.SyncService.ExecuteSync(Boolean skipNotifications) in backend/FwLite/FwLiteShared/Sync/SyncService.cs:line 89
at FwLiteShared.Projects.CombinedProjectsService.<>c.<<DownloadProject>b__16_1>d.MoveNext() in backend/FwLite/FwLiteShared/Projects/CombinedProjectsService.cs:line 202
--- End of stack trace from previous location ---
at LcmCrdt.CrdtProjectsService.CreateProject(CreateProjectRequest request) in backend/FwLite/LcmCrdt/CrdtProjectsService.cs:line 175
at LcmCrdt.CrdtProjectsService.CreateProject(CreateProjectRequest request) in backend/FwLite/LcmCrdt/CrdtProjectsService.cs:line 191
at LcmCrdt.CrdtProjectsService.CreateProject(CreateProjectRequest request) in backend/FwLite/LcmCrdt/CrdtProjectsService.cs:line 194
at LcmCrdt.CrdtProjectsService.CreateProject(CreateProjectRequest request) in backend/FwLite/LcmCrdt/CrdtProjectsService.cs:line 194
at FwLiteShared.Projects.CombinedProjectsService.<>c__DisplayClass16_0.<<DownloadProject>b__0>d.MoveNext() in backend/FwLite/FwLiteShared/Projects/CombinedProjectsService.cs:line 196
--- End of stack trace from previous location ---
at FwLiteShared.Projects.CombinedProjectsService.DownloadProject(ProjectModel project) in backend/FwLite/FwLiteShared/Projects/CombinedProjectsService.cs:line 196If possible, analyse why NewEntity is being called on a |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
echo "=== PictureSync.cs ==="
fd -e cs "PictureSync" | xargs cat
echo ""
echo "=== CreatePictureChange.cs ==="
fd -e cs "CreatePictureChange" | xargs cat
echo ""
echo "=== Search for where pictures are synced from FW to CRDT ==="
rg -n "CreatePicture\|AddPicture\|picture.*sync\|sync.*picture" --type cs -i backend/FwLite/ -C 3 | head -80
echo ""
echo "=== How other entity syncs look - e.g. EntrySync ==="
fd -e cs "EntrySync" backend/FwLite/ | xargs cat 2>/dev/null | head -100
echo ""
echo "=== How other entity syncs look - e.g. SenseSync ==="
fd -e cs "SenseSync" backend/FwLite/ | xargs cat 2>/dev/null | head -100Length of output: 10820 🏁 Script executed: #!/bin/bash
echo "=== Find CrdtMiniLcmApi picture-related methods ==="
fd -e cs "CrdtMiniLcmApi" backend/FwLite/ | xargs grep -n "Picture\|picture" | head -60
echo ""
echo "=== Find CreatePicture and UpdatePicture implementations in LcmCrdt ==="
rg -n "CreatePicture\|UpdatePicture\|DeletePicture\|MovePicture" --type cs backend/FwLite/LcmCrdt/ -C 5
echo ""
echo "=== Find how JsonPatchChange is created/used ==="
rg -n "JsonPatchChange\b" --type cs backend/FwLite/ -C 3
echo ""
echo "=== CrdtFwdataProjectSyncService - picture related sync code ==="
rg -n "Picture\|picture" --type cs backend/FwLite/FwLiteProjectSync/CrdtFwdataProjectSyncService.cs -C 3
echo ""
echo "=== Check UpdatePicture in LcmCrdt and what change type it creates ==="
rg -n "async.*UpdatePicture\|Task.*UpdatePicture" --type cs backend/FwLite/ -C 10Length of output: 14639
Root cause
// Overload 1 — takes an UpdateObjectInput, creates JsonPatchChange<Picture>
public async Task<Picture> UpdatePicture(Guid entryId, Guid senseId, Guid pictureId, UpdateObjectInput<Picture> update)
{
var patchChange = new JsonPatchChange<Picture>(pictureId, jsonPatch); // ← EditChange, no NewEntity support
await AddChange(patchChange);
...
}
// Overload 2 — takes before/after Pictures, calls PictureSync.Sync (single-picture version)
public async Task<Picture> UpdatePicture(Guid entryId, Guid senseId, Picture before, Picture after, ...)
{
await PictureSync.Sync(entryId, senseId, before, after, api ?? this); // ← ONLY calls UpdatePicture, NEVER CreatePicture
...
}The single-picture The sync code on the FW headless server is taking a path that calls the "before/after Picture" Where to lookThe FW headless sync must have new code (in this PR) in either // CORRECT — uses DiffOrderable, calls CreatePicture for new pictures
await PictureSync.Sync(entryId, senseId, beforeSense.Pictures, afterSense.Pictures, api);…it's going through something like: // WRONG — single-picture overload, always emits JsonPatchChange, never CreatePicture
foreach (var (before, after) in matchedPictures)
await api.UpdatePicture(entryId, senseId, before, after);
// or
await PictureSync.Sync(entryId, senseId, beforePicture, afterPicture, api);Suggested fixOption A — In // In SenseSync.Sync, alongside the ExampleSentenceSync call:
changes += await PictureSync.Sync(entryId,
beforeSense.Id,
beforeSense.Pictures,
afterSense.Pictures,
api);Option B — If the FW sync has a custom loop in The key invariant to enforce: 🧠 Learnings used |
Compiles, but some logic (most notably MediaUri handling) is incomplete
Next up will be writing tests to make sure we can properly two-way sync a picture from FW Classic, i.e. that the MediaUri handling is correct.
Contributor
Author
|
Rebased on top of current |
Contributor
Author
|
Huh. Look commit 2ce7ec8 and, specifically, at the DB migration. Why did it add the HomographNumber property? I might have had a local DB that's not up-to-date. I might have to delete that migration and create a new one. |
Contributor
Author
|
Force-pushed to get rid of commit 2ce7ec8 so the migration won't add a duplicate HomographNumber column. |
MiniLcmApiWriteNormalizationWrapper is new on develop branch since I started this PR, so I didn't have the Picture CRUD API calls for it.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/FwLite/LcmCrdt/Changes/CreateSenseChange.cs (1)
12-21: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDon't half-wire the new
Picturespayload.
CreateSenseChangenow capturessense.Pictures, butNewEntity()still hardcodesPictures = new(). That makes the new field unreachable on replay and can silently drop picture data unless some other path recreates it. Either materialize the pictures here or remove the field/assignment from this change type.Based on the separate picture-change flow in
backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs, this payload should be either fully applied or not serialized here at all.Also applies to: 38-53
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/FwLite/LcmCrdt/Changes/CreateSenseChange.cs` around lines 12 - 21, CreateSenseChange is only partially wiring the new Pictures payload: the constructor captures sense.Pictures, but NewEntity still initializes Pictures to an empty collection, so replay can drop picture data. Update CreateSenseChange/NewEntity to either fully persist and restore Pictures from the change payload (using the existing Pictures field) or remove the Pictures assignment from this change type entirely so it is not serialized here.
♻️ Duplicate comments (1)
backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs (1)
1828-1833: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
GetPicturedereferenceslcmPicturebefore the null check.
ValidateOwnershipaccesseslcmPicture.Owner, butTryGetObjectleaveslcmPicturenull for unknown IDs. The null check on Line 1832 runs too late, so an unknownidthrows aNullReferenceExceptioninstead of returningnull. This was previously raised and appears unaddressed.Suggested fix
public Task<Picture?> GetPicture(Guid entryId, Guid senseId, Guid id) { PictureRepository.TryGetObject(id, out var lcmPicture); - ValidateOwnership(lcmPicture, entryId, senseId); - return Task.FromResult(lcmPicture is null ? null : FromLcmPicture(senseId, lcmPicture)); + if (lcmPicture is null) return Task.FromResult<Picture?>(null); + ValidateOwnership(lcmPicture, entryId, senseId); + return Task.FromResult<Picture?>(FromLcmPicture(senseId, lcmPicture)); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs` around lines 1828 - 1833, GetPicture is calling ValidateOwnership before checking whether PictureRepository.TryGetObject returned a picture, which can dereference a null lcmPicture. Update GetPicture in FwDataMiniLcmApi to return null immediately when TryGetObject fails, and only call ValidateOwnership and FromLcmPicture after confirming lcmPicture is non-null; use the GetPicture, TryGetObject, and ValidateOwnership symbols to place the fix.
🧹 Nitpick comments (3)
backend/FwLite/FwLiteProjectSync/DryRunMiniLcmApi.cs (1)
323-327: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMisleading parameter name in
MovePicture.The picture id parameter is named
exampleId(copy-paste fromMoveExampleSentence), which also surfaces in the dry-run description text. Rename topictureIdfor clarity.Proposed fix
- public Task MovePicture(Guid entryId, Guid senseId, Guid exampleId, BetweenPosition between) + public Task MovePicture(Guid entryId, Guid senseId, Guid pictureId, BetweenPosition between) { - DryRunRecords.Add(new DryRunRecord(nameof(MovePicture), $"Move picture {exampleId} between {between.Previous} and {between.Next}")); + DryRunRecords.Add(new DryRunRecord(nameof(MovePicture), $"Move picture {pictureId} between {between.Previous} and {between.Next}")); return Task.CompletedTask; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/FwLite/FwLiteProjectSync/DryRunMiniLcmApi.cs` around lines 323 - 327, The MovePicture method is using a misleading parameter name copied from MoveExampleSentence, so update the signature and any related dry-run message to use pictureId instead of exampleId. Make the change in DryRunMiniLcmApi’s MovePicture implementation and ensure the DryRunRecord description clearly refers to the picture identifier for consistency and readability.backend/FwLite/LcmCrdt/OrderPicker.cs (1)
41-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffConsider extracting the shared ordering switch to remove duplication.
The
(previous, next)resolution plus the order-selectionswitchis copied verbatim from theIQueryableoverload (Lines 21-37). Since the divergence is only howitems/previous/nextare materialized, the shared tail could be a private helper taking the resolved list and neighbors, keeping both overloads in sync if the ordering heuristic changes later.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/FwLite/LcmCrdt/OrderPicker.cs` around lines 41 - 63, The ordering heuristic in OrderPicker.PickOrder is duplicated between the List<T> overload and the IQueryable overload, so update the shared neighbor-to-order logic to live in one private helper. Keep the existing PickOrder methods responsible only for resolving previous/next from items and then delegate the common switch-based order selection to the helper. This will keep both overloads in sync if the ordering rules change later and avoid copy-paste drift.backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs (1)
929-932: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the unused
repoinCreatePicture.The
repoopened at line 929 is never used — the readback goes throughGetPicture, which opens its own repo. This is dead code that misleads readers into thinking the surrounding logic shares aDbContext.♻️ Proposed cleanup
public async Task<Picture> CreatePicture(Guid entryId, Guid senseId, Picture picture, BetweenPosition? between = null) { - await using var repo = await repoFactory.CreateRepoAsync(); var change = new CreateSensePictureChange(picture, senseId, between); await AddChange(change); return await GetPicture(entryId, senseId, change.PictureId) ?? throw NotFoundException.ForType<Picture>(change.PictureId); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs` around lines 929 - 932, Remove the unused repo acquisition in CreatePicture: the await using var repo = await repoFactory.CreateRepoAsync() in CrdtMiniLcmApi.CreatePicture is dead code because AddChange and GetPicture do not use it, so delete that local and keep the method flow unchanged with CreateSensePictureChange, AddChange, and GetPicture.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/FwLite/MiniLcm/Normalization/MiniLcmApiWriteNormalizationWrapper.cs`:
- Around line 564-600: NormalizePicturePatch is mutating the caller’s
UpdateObjectInput by removing MediaUri operations from update.Patch.Operations.
Change the removal/reinsertion flow to work on a copied operations list or a
cloned patch inside NormalizePicturePatch, and pass that working copy into
NormalizePatch so the original update remains unchanged. Keep the existing
MediaUri-preserving logic, but ensure all RemoveAt/Insert operations are applied
only to the local copy, not the input object.
---
Outside diff comments:
In `@backend/FwLite/LcmCrdt/Changes/CreateSenseChange.cs`:
- Around line 12-21: CreateSenseChange is only partially wiring the new Pictures
payload: the constructor captures sense.Pictures, but NewEntity still
initializes Pictures to an empty collection, so replay can drop picture data.
Update CreateSenseChange/NewEntity to either fully persist and restore Pictures
from the change payload (using the existing Pictures field) or remove the
Pictures assignment from this change type entirely so it is not serialized here.
---
Duplicate comments:
In `@backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs`:
- Around line 1828-1833: GetPicture is calling ValidateOwnership before checking
whether PictureRepository.TryGetObject returned a picture, which can dereference
a null lcmPicture. Update GetPicture in FwDataMiniLcmApi to return null
immediately when TryGetObject fails, and only call ValidateOwnership and
FromLcmPicture after confirming lcmPicture is non-null; use the GetPicture,
TryGetObject, and ValidateOwnership symbols to place the fix.
---
Nitpick comments:
In `@backend/FwLite/FwLiteProjectSync/DryRunMiniLcmApi.cs`:
- Around line 323-327: The MovePicture method is using a misleading parameter
name copied from MoveExampleSentence, so update the signature and any related
dry-run message to use pictureId instead of exampleId. Make the change in
DryRunMiniLcmApi’s MovePicture implementation and ensure the DryRunRecord
description clearly refers to the picture identifier for consistency and
readability.
In `@backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs`:
- Around line 929-932: Remove the unused repo acquisition in CreatePicture: the
await using var repo = await repoFactory.CreateRepoAsync() in
CrdtMiniLcmApi.CreatePicture is dead code because AddChange and GetPicture do
not use it, so delete that local and keep the method flow unchanged with
CreateSensePictureChange, AddChange, and GetPicture.
In `@backend/FwLite/LcmCrdt/OrderPicker.cs`:
- Around line 41-63: The ordering heuristic in OrderPicker.PickOrder is
duplicated between the List<T> overload and the IQueryable overload, so update
the shared neighbor-to-order logic to live in one private helper. Keep the
existing PickOrder methods responsible only for resolving previous/next from
items and then delegate the common switch-based order selection to the helper.
This will keep both overloads in sync if the ordering rules change later and
avoid copy-paste drift.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6bb4c0fe-ae82-4313-9773-10df85b0e890
📒 Files selected for processing (53)
backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.csbackend/FwLite/FwDataMiniLcmBridge/Api/UpdateProxy/UpdatePictureProxy.csbackend/FwLite/FwDataMiniLcmBridge/Api/UpdateProxy/UpdateSenseProxy.csbackend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.csbackend/FwLite/FwLiteProjectSync.Tests/Snapshots/sena-3_snapshot.2026-04-09.verified.txtbackend/FwLite/FwLiteProjectSync.Tests/SyncTests.csbackend/FwLite/FwLiteProjectSync.Tests/UpdateDiffTests.csbackend/FwLite/FwLiteProjectSync.Tests/sena-3-live_snapshot.verified.txtbackend/FwLite/FwLiteProjectSync/DryRunMiniLcmApi.csbackend/FwLite/FwLiteShared/TypeGen/ReinforcedFwLiteTypingConfig.csbackend/FwLite/LcmCrdt.Tests/Changes/ChangeDeserializationRegressionData.latest.verified.txtbackend/FwLite/LcmCrdt.Tests/Changes/ChangeDeserializationRegressionData.legacy.verified.txtbackend/FwLite/LcmCrdt.Tests/Changes/UseChangesTests.csbackend/FwLite/LcmCrdt.Tests/Data/MigrationTests_FromScriptedDb.v1.ChangeEntities.verified.txtbackend/FwLite/LcmCrdt.Tests/Data/MigrationTests_FromScriptedDb.v1.ProjectSnapshot.verified.txtbackend/FwLite/LcmCrdt.Tests/Data/MigrationTests_FromScriptedDb.v1.Snapshots.verified.txtbackend/FwLite/LcmCrdt.Tests/Data/MigrationTests_FromScriptedDb.v2.ChangeEntities.verified.txtbackend/FwLite/LcmCrdt.Tests/Data/MigrationTests_FromScriptedDb.v2.ProjectSnapshot.verified.txtbackend/FwLite/LcmCrdt.Tests/Data/MigrationTests_FromScriptedDb.v2.Snapshots.verified.txtbackend/FwLite/LcmCrdt.Tests/Data/SnapshotDeserializationRegressionData.latest.verified.txtbackend/FwLite/LcmCrdt.Tests/Data/SnapshotDeserializationRegressionData.legacy.verified.txtbackend/FwLite/LcmCrdt.Tests/Data/VerifyRegeneratedSnapshotsAfterMigrationFromScriptedDb.v1.verified.jsonbackend/FwLite/LcmCrdt.Tests/Data/VerifyRegeneratedSnapshotsAfterMigrationFromScriptedDb.v2.verified.jsonbackend/FwLite/LcmCrdt.Tests/DataModelSnapshotTests.VerifyChangeModels.verified.txtbackend/FwLite/LcmCrdt.Tests/DataModelSnapshotTests.VerifyDbModel.verified.txtbackend/FwLite/LcmCrdt/Changes/CreateSenseChange.csbackend/FwLite/LcmCrdt/Changes/CreateSensePictureChange.csbackend/FwLite/LcmCrdt/Changes/RemoveSensePictureChange.csbackend/FwLite/LcmCrdt/Changes/ReorderSensePictureChange.csbackend/FwLite/LcmCrdt/Changes/UpdateSensePictureChange.csbackend/FwLite/LcmCrdt/CrdtMiniLcmApi.csbackend/FwLite/LcmCrdt/LcmCrdtDbContext.csbackend/FwLite/LcmCrdt/LcmCrdtKernel.csbackend/FwLite/LcmCrdt/Migrations/20260619022528_AddPicturesColumnToSenses.Designer.csbackend/FwLite/LcmCrdt/Migrations/20260619022528_AddPicturesColumnToSenses.csbackend/FwLite/LcmCrdt/Migrations/LcmCrdtDbContextModelSnapshot.csbackend/FwLite/LcmCrdt/OrderPicker.csbackend/FwLite/MiniLcm.Tests/AutoFakerHelpers/AutoFakerDefault.csbackend/FwLite/MiniLcm.Tests/AutoFakerHelpers/EntryFakerHelper.csbackend/FwLite/MiniLcm/IMiniLcmReadApi.csbackend/FwLite/MiniLcm/IMiniLcmWriteApi.csbackend/FwLite/MiniLcm/Models/Picture.csbackend/FwLite/MiniLcm/Models/Sense.csbackend/FwLite/MiniLcm/Normalization/MiniLcmApiWriteNormalizationWrapper.csbackend/FwLite/MiniLcm/SyncHelpers/PictureSync.csbackend/FwLite/MiniLcm/SyncHelpers/SenseSync.csbackend/LfClassicData/Entities/Entry.csbackend/LfClassicData/LfClassicMiniLcmApi.csfrontend/viewer/src/lib/dotnet-types/generated-types/MiniLcm/Models/IPicture.tsfrontend/viewer/src/lib/dotnet-types/generated-types/MiniLcm/Models/ISense.tsfrontend/viewer/src/lib/sandbox/Sandbox.sveltefrontend/viewer/src/lib/utils.tsfrontend/viewer/src/project/demo/demo-entry-data.ts
hahn-kev
reviewed
Jun 26, 2026
hahn-kev
reviewed
Jun 26, 2026
hahn-kev
reviewed
Jun 26, 2026
hahn-kev
reviewed
Jun 26, 2026
hahn-kev
reviewed
Jun 26, 2026
hahn-kev
reviewed
Jun 26, 2026
hahn-kev
reviewed
Jun 26, 2026
hahn-kev
reviewed
Jun 26, 2026
hahn-kev
reviewed
Jun 26, 2026
hahn-kev
reviewed
Jun 26, 2026
hahn-kev
reviewed
Jun 26, 2026
hahn-kev
reviewed
Jun 26, 2026
hahn-kev
reviewed
Jun 26, 2026
hahn-kev
reviewed
Jun 26, 2026
If LinkedFiles directory was missing, the LocalMediaAdapter code was throwing an exception. Instead, we should just say "no files found" and not call Directory.EnumerateFiles() if the root path doesn't exist.
Contributor
Author
|
@hahn-kev - I believe all review comments are addressed now. I've left the conversations open so you can more easily track which ones I've addressed. Please feel free to resolve any that you don't need to discuss further. |
hahn-kev
approved these changes
Jun 30, 2026
hahn-kev
left a comment
hahn-kev
left a comment
Collaborator
There was a problem hiding this comment.
great work! I'm really happy with the PR now, just get the tests passing and it should be good to go.
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
2 participants
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.
Fixes #2232.
Fixes #2233.
UI will be in a different PR.
Pictures now sync between FwData and Crdt, and unit tests verify that the sync is correct. Until the UI PR is ready, this one can be manually tested by checking the "Entry as JSON" data: the Pictures list is a child of each Sense.
All unit tests and integration tests are passing.
NOTE: Should probably incorporate the changes from #2367 into the PictureSync code as well, i.e. have the sync helpers fire off SubmitUpdate instead of Update (and so on) for pictures as well.