Lexbox API to create empty FLEx project - #2439
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 Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughChangesTemplate Project Creation
Estimated code review effort: 4 (Complex) | ~60 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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 |
| } | ||
| catch (Exception ex) | ||
| { | ||
| logger.LogError(ex, "Failed to roll back project {ProjectId} ({Code}) after a failed template creation", projectId, code); |
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
Shared by LexBoxApi (query param) and FwHeadless (creation handler); None is the default. Values: None, Standard, Enhanced. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Builds a new FieldWorks project from the SIL.LCModel template and pushes it into the empty hg repo LexBox already created: clone the empty remote → build fw.fwdata (first vernacular + first analysis WS) → add the remaining writing systems via the existing CreateWritingSystem surface (dedup + save-on-dispose, then release LCM file locks) → hg add/commit → push. Guards the fdoDataModelVersion, cleans up the local folder on failure, and stubs anthropologyCategories (// TODO: Implement). Adds a per-project creation reservation to SyncHostedService so a sync can't be queued for a project mid-creation (and two creations can't race). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
POST /api/project/create-from-template (projectId query param + JSON body of vernacular/analysis writing systems and anthropologyCategories). Validates the WS tags with IetfLanguageTag at the boundary, resolves the code via IProjectLookupService, and delegates to ProjectCreationService. Registers the service and maps the route. Threads AnthropologyCategories through to the stub. Internal endpoint (cluster-only, no auth) — LexBoxApi does the admin check and the project/repo creation before calling this. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Posts the vernacular/analysis writing systems and anthropologyCategories to the FwHeadless create-from-template endpoint for a given project id. Runs inline; returns null on success or the error body on failure (for the controller's saga rollback). Threads a CancellationToken like the sibling methods. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
POST /api/project/createFromTemplate (admin only): validates the code and that at least one wsVernacular is given (400), rejects a taken code (409), defaults wsAnalysis to ["en"], then creates the FLEx project (Postgres row + empty repo) and asks FwHeadless to populate the repo with the template .fwdata. On FwHeadless failure it compensates by deleting the repo + project row; on success it refreshes LastCommit and returns the new project id. wsVernacular/wsAnalysis are repeatable query params; anthropologyCategories (enhanced|standard|none) defaults to none. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Unit tests (no infrastructure): ProjectController.CreateFromTemplate rejects a missing vernacular WS and an invalid code with 400; SyncHostedService's creation reservation blocks a second concurrent creation and refuses to queue a sync while a project is being created, and is reusable after release. The end-to-end (template build + empty-repo first push) is integration-level and runs in CI. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Close a TOCTOU race between TryStartProjectCreation and QueueJob (they checked each other's dictionary without a shared lock, so a sync could be queued for a project mid-creation and race on the repo). Both check-then-add paths now run under one lock. - AssertModelVersionMatches now fails closed: an unreadable version is treated as a mismatch rather than silently allowing the push. - Log a warning when a not-yet-implemented anthropologyCategories value is requested so the no-op stub isn't silent. - Route: null-safe writing-system checks and reject an empty analysis list with 400 (rather than a 500 from the service guard). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Propagate FwHeadless's status code: an invalid writing-system tag now surfaces as 400 instead of being flattened to 500 (FwHeadlessClient returns the status + body). - Give the FwHeadless HTTP client a 5-minute timeout so inline creation (cold LCM load + push) doesn't hit the 100s default. - Route the failure compensation through a new ProjectService.CleanupFailedProjectCreation so repo+row deletion and cache invalidation stay in one place (the controller no longer deletes the row directly and skip cache invalidation). - Extract the analysis-default (["en"]) into a testable helper. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Assert the ProblemDetails.Detail so each validation test proves its own branch (the two 400s were previously indistinguishable). - Add the mirror race case: a queued sync blocks a creation. - Cover the analysis-default helper (null/empty -> ["en"], supplied kept). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
End-to-end test (Category=Integration, CI-only — needs the lexbox stack): calls the admin createFromTemplate endpoint, then asserts the empty repo received the first commit (GetProjectLastCommit non-null; hgweb tip is no longer the all-zero hash) and that the pushed fw.fwdata carries the requested vernacular + analysis writing systems. Also asserts a missing vernacular is rejected with 400. Cleans up via softDeleteProject. This exercises the one path that can't be unit-tested: LfMergeBridge/Chorus pushing a locally-committed brand-new project into an empty remote repo. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ProjectCreationService needs NewLangProj.fwdata (and its sibling template files) to build a new project, but FwDataBridgeConfig.TemplatesFolder defaults to the FieldWorks user-data dir (~/.local/share/fieldworks/Templates), which doesn't exist in the container — creation failed with DirectoryNotFoundException on NewLangProj.fwdata. Copy the SIL.LCModel package's contentFiles/Templates/* into FwHeadless's output and point TemplatesFolder at that shipped folder (AppContext.BaseDirectory/Templates) unless overridden in config. Mirrors how FwLiteWeb and the bridge tests ship the templates. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Creating a project from a template pushes into a repo LexBox has only `hg init`'d server-side, so it has zero changesets. Cloning that empty remote is a no-op that returns Success=true but leaves no local `.hg`, so the subsequent `hg add` failed with "no repository found ... (.hg not found)". Replace the clone with LfMerge's from-scratch genesis recipe (verified against the lfmerge reference implementation): `hg init` the local fw/ folder, set the branch to the FDO model version BEFORE the first commit, build the .fwdata, then commit + push. Chorus's Language_Forge_Send_Receive refuses to make the first-ever commit itself, so the manual CommitFile stays. The branch step is the data-correctness fix: FLEx clients clone looking for a branch named after the model version, so a first commit landing on `default` would be invisible to them. Numeric branch names work via the fixutf8 hg extension already wired into Mercurial/mercurial.ini. Adds InitRepo and SetBranch primitives to SendReceiveHelpers / ISendReceiveService. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The template project can be built with an explicit UI writing system. Thread a new `wsUi` query parameter (default "en") from the admin endpoint through FwHeadless to ProjectLoader.NewProject, which passes it to LcmCache.CreateNewLangProj as the trailing user-interface writing-system string (after the analysis/vernacular definitions). Like wsVernacular/wsAnalysis it must be a valid writing-system tag; that check lives in ProjectRoutes.CreateProjectFromTemplate alongside the existing ones. NewProject defaults uiWs to "en" so existing callers are unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Previously the template was built with only the first vernacular + analysis writing system, then FwDataFactory was reopened to add the rest one by one. LcmCache.CreateNewLangProj accepts two more parameters after the uiWs string — a HashSet<CoreWritingSystemDefinition> of additional analysis writing systems and one of additional vernacular writing systems — so let it create the whole set in one pass instead. ProjectLoader gains a list-based NewProject overload that builds those sets by turning each list into a hash set and removing the default (the first item, which is passed separately). ProjectCreationService.BuildFromTemplate now takes the two lists and no longer needs FwDataFactory or the AddWritingSystems post-step. Adds a test creating a project with two analysis and two vernacular writing systems and asserting each lands on the correct list (also guards the analysis-then-vernacular argument order into CreateNewLangProj). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Chorus discovers file-type handlers via DirectoryCatalog(".", "*-ChorusPlugin.dll"),
which scans the current working directory. WebApplication.CreateBuilder does not set
CWD, so when FwHeadless launches from a dir other than the one holding the co-published
LibFLExBridge-ChorusPlugin.dll (e.g. `dotnet run`), that plugin is never loaded. Without
it, FieldWorks extensions like `.list` fall back to Chorus's 1 MB LargeFileFilter default,
so the >1 MB SemanticDomainList.list is silently filtered out of a new project's first
push, leaving projects with no semantic domains.
Set Environment.CurrentDirectory to AppContext.BaseDirectory before building the host so
the plugin is always discovered (which gives FieldWorks files their intended unlimited
size and restores the full merge/diff handler set). Safe here: hg runs with explicit
working dirs and TemplatesFolder already resolves against AppContext.BaseDirectory.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ion) New project creation set the genesis Mercurial branch to the bare FDO model version (7000072), which LfMergeBridge only accepts via a backwards-compatibility fallback. The modern convention is to name the branch by joining the FlexBridgeDataVersion, a dot, and the model version (7500002.7000072) -- exactly what LfMergeBridge derives from the model version via FlexUpdateBranchHelperStrategy.GetBranchNameFromModelVersion. Add FlexBridgeDataVersion (default 7500002, mirroring FLExBridge's internal constant we can't reference) and a SendReceiveBranchName config property, and set the genesis branch from it so the first commit lands on the branch send/receive expects with no fallback. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Match how FieldWorks bootstraps a new repo: the first commit is a minimal FLExProject.CustomProperties file (an empty <AdditionalFields /> list) on hg's default branch, and the .fwdata is no longer committed directly. Send/Receive then splits the .fwdata into the nested files Mercurial actually tracks (the .fwdata is excluded from tracking), landing them on the model-version branch. The genesis file is written with SIL's CanonicalXmlSettings so its bytes match exactly what the splitter regenerates, leaving no spurious diff on the first Send/Receive. Reorder so the genesis commit lands on 'default', then set the model-version branch, then Send/Receive. Add a CommitEmpty helper used as a temporary workaround for a bug in LanguageForgeSendReceiveActionHandler, which refuses to commit when doing so could create a new branch and so won't establish the model-version branch itself; the empty commit records the branch change and creates the head Send/Receive needs. Remove once that bug is fixed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The template project no longer commits fw.fwdata to the repo, so fetching raw-file/tip/fw.fwdata always 404s. Instead fetch General/LanguageProject.langproj (one of the nested files Send/Receive produces), parse it as XML, and assert the requested writing systems are current: each analysis code appears in CurAnalysisWss/Uni and each vernacular code in CurVernWss/Uni. Check presence rather than exact equality because FieldWorks adds its own defaults (e.g. "en" as an analysis WS), so the stored list is a superset of what was requested. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
liblcm's CreateNewLangProj builds LinkedFilesRootDir with Path.Combine, which on this Linux host yields '%proj%/LinkedFiles' (forward slash). FieldWorks on Windows compares this value with an exact string match against '%proj%\LinkedFiles' (backslash), so it prints a warning on its first Send/Receive of a project we created. After building the template .fwdata, rewrite just that one value to the backslash form. Rather than parse the potentially multi-MB fwdata as XML, stream it once: find the line beginning with <LinkedFilesRootDir> and replace the forward slash on the following <Uni> line, copying every other line through unchanged. The edit is length-preserving (exactly one byte changes) and anchored to the LinkedFilesRootDir element so no other path is touched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Populating a new project's anthropology categories requires template files that live only in FieldWorks, not in liblcm, so the feature can't be implemented on the FwHeadless side. Remove the AnthropologyCategories enum and every parameter, DTO field, and no-op handler that referenced it: the createFromTemplate REST param, CreateProjectFromTemplateInput, FwHeadlessClient, the FwHeadless route, and ProjectCreationService (including the ApplyAnthropologyCategories stub). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Rebasing on top of develop before I push my new work from today. |
11e808d to
707f704
Compare
|
@coderabbitai fullreview |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
backend/LexBoxApi/Controllers/ProjectController.cs (1)
53-56: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDead
code is nullcheck.
codeis a non-nullablestringbound implicitly from the query string. With nullable reference types enabled, ASP.NET Core's[ApiController]model binding treats such parameters as required and returns 400 before the action runs, makingcode is nullunreachable. Based on learnings, "avoid adding manual string.IsNullOrWhiteSpace guards for missing values on non-nullable query params."🤖 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/LexBoxApi/Controllers/ProjectController.cs` around lines 53 - 56, Remove the unreachable `code is null` condition from the validation in `ProjectController`, while preserving the minimum-length and `Project.ProjectCodeRegex` checks and existing 400 response.Source: Learnings
backend/FwHeadless/Services/ProjectCreationService.cs (1)
64-64: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winParameter order flips between
CreateFromTemplate,BuildFromTemplate, andNewProject.
CreateFromTemplatetakes(vernacular, analysis, ui),BuildFromTemplatealso takes(vernacular, analysis, ui), butNewProjecttakes(analysis, vernacular, ui). All three currently thread the right list to the right slot (verified by parameter-name binding), but with two identically-typedIReadOnlyList<string>positional arguments and no compiler check, this is an easy spot for a future edit to silently swap analysis/vernacular writing systems in every newly-created project.♻️ Suggested consistency fix
- private void BuildFromTemplate( - FwDataProject fwDataProject, - IReadOnlyList<string> vernacularWritingSystems, - IReadOnlyList<string> analysisWritingSystems, - string uiWs) + private void BuildFromTemplate( + FwDataProject fwDataProject, + IReadOnlyList<string> analysisWritingSystems, + IReadOnlyList<string> vernacularWritingSystems, + string uiWs) { - using var cache = projectLoader.NewProject(fwDataProject, analysisWritingSystems, vernacularWritingSystems, uiWs); + using var cache = projectLoader.NewProject(fwDataProject, analysisWritingSystems, vernacularWritingSystems, uiWs); }and update the call site to
BuildFromTemplate(fwDataProject, analysisWritingSystems, vernacularWritingSystems, uiWritingSystem)to match.Also applies to: 95-105
🤖 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/FwHeadless/Services/ProjectCreationService.cs` at line 64, Standardize the writing-system parameter order across CreateFromTemplate, BuildFromTemplate, and NewProject to use vernacular, analysis, then UI. Update NewProject’s signature and internal binding as needed, and change the BuildFromTemplate call site to pass vernacularWritingSystems before analysisWritingSystems while preserving each list’s intended destination.backend/FwLite/FwDataMiniLcmBridge.Tests/Fixtures/MockFwProjectLoader.cs (1)
22-38: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMock doesn't override the new list-based
NewProjectoverload.
MockFwProjectLoaderonly overrides the single-pair overload. The newNewProject(project, IReadOnlyList<string>, IReadOnlyList<string>, string)overload is virtual on the baseProjectLoaderbut unoverridden here, so any test/consumer that calls it through this mock will silently fall through to the real liblcm-backed implementation instead of the lightweight in-memory blank project — defeating the point of using the mock.🤖 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.Tests/Fixtures/MockFwProjectLoader.cs` around lines 22 - 38, The MockFwProjectLoader must override the new list-based NewProject overload so calls using IReadOnlyList<string> work through the lightweight in-memory path. Add the override alongside the existing NewProject method, preserving the same initialization, cache creation, project registration, and return behavior while adapting the list-based workspace arguments as required by the base API.
🤖 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/LexBoxApi/Services/ProjectService.cs`:
- Around line 241-254: Update CleanupFailedProjectCreation to invoke the
FwHeadless deletion/guard operation before DeleteRepoIfExists and any database
removal, mirroring the ordering in DeleteProjectPermanently. Ensure an
in-progress sync raises ProjectSyncInProgressException and prevents cleanup from
deleting an active or completed creation; preserve the existing repository,
database, and cache cleanup after the guard succeeds.
---
Nitpick comments:
In `@backend/FwHeadless/Services/ProjectCreationService.cs`:
- Line 64: Standardize the writing-system parameter order across
CreateFromTemplate, BuildFromTemplate, and NewProject to use vernacular,
analysis, then UI. Update NewProject’s signature and internal binding as needed,
and change the BuildFromTemplate call site to pass vernacularWritingSystems
before analysisWritingSystems while preserving each list’s intended destination.
In `@backend/FwLite/FwDataMiniLcmBridge.Tests/Fixtures/MockFwProjectLoader.cs`:
- Around line 22-38: The MockFwProjectLoader must override the new list-based
NewProject overload so calls using IReadOnlyList<string> work through the
lightweight in-memory path. Add the override alongside the existing NewProject
method, preserving the same initialization, cache creation, project
registration, and return behavior while adapting the list-based workspace
arguments as required by the base API.
In `@backend/LexBoxApi/Controllers/ProjectController.cs`:
- Around line 53-56: Remove the unreachable `code is null` condition from the
validation in `ProjectController`, while preserving the minimum-length and
`Project.ProjectCodeRegex` checks and existing 400 response.
🪄 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 Plus
Run ID: e44e80e4-8af0-4b09-bccb-de1a96b9adf5
📒 Files selected for processing (21)
backend/FwHeadless/FwHeadless.csprojbackend/FwHeadless/FwHeadlessConfig.csbackend/FwHeadless/FwHeadlessKernel.csbackend/FwHeadless/Program.csbackend/FwHeadless/Routes/ProjectRoutes.csbackend/FwHeadless/Services/ISendReceiveService.csbackend/FwHeadless/Services/ProjectCreationService.csbackend/FwHeadless/Services/SendReceiveHelpers.csbackend/FwHeadless/Services/SendReceiveService.csbackend/FwHeadless/Services/SyncHostedService.csbackend/FwLite/FwDataMiniLcmBridge.Tests/Fixtures/MockFwProjectLoader.csbackend/FwLite/FwDataMiniLcmBridge/LcmUtils/ProjectLoader.csbackend/FwLite/FwLiteProjectSync.Tests/ProjectTemplateTests.csbackend/LexBoxApi/Controllers/ProjectController.csbackend/LexBoxApi/LexBoxKernel.csbackend/LexBoxApi/Services/FwHeadlessClient.csbackend/LexBoxApi/Services/ProjectService.csbackend/LexCore/Entities/CreateProjectFromTemplateInput.csbackend/Testing/FwHeadless/SyncHostedServiceCreationReservationTests.csbackend/Testing/LexBoxApi/CreateFromTemplateValidationTests.csbackend/Testing/SyncReverseProxy/CreateProjectFromTemplateTests.cs
CleanupFailedProjectCreation deleted the hg repo and project row unconditionally, without asking FwHeadless to delete its local project state. Call fwHeadless.DeleteProject first, mirroring DeleteProjectPermanently: it cleans FwHeadless's local state and throws ProjectSyncInProgressException if a creation or sync is still in flight, so we don't tear down an active or already-completed creation. The repo, database, and cache cleanup are preserved after the guard succeeds, with the repo deletion moved after the database removal. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CreateFromTemplate_PopulatesTheEmptyRepoWithTheRequestedWritingSystems failed in CI with 409 "already exists". The shared ApiTestBase HttpClient retries transient failures and timeouts, but createFromTemplate is a long-running, non-idempotent operation: the first attempt creates the project row, then (now that the semantic domains actually get committed and pushed) runs long enough to time out or transiently fail, and the retried POST hits the just-created project and returns 409. Give NewHttpClient a retryTransientFailures flag and have the test issue the creation POST on a non-retrying client with a longer timeout, authenticated as the same admin via Bearer token. A single attempt now surfaces the real result instead of a misleading conflict. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The previous fix gave the creation POST a dedicated non-retrying client but authenticated it with _adminApiTester.CurrJwt as a Bearer token. That JWT is captured once at login and goes stale (the shared client stays authenticated via its refreshed cookie, not that value), so the request failed with 401. Have the dedicated client log in itself via JwtHelper.ExecuteLogin, which sets a fresh auth cookie on it - matching how the shared tester authenticates - while keeping the no-retry behavior and longer timeout. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Self-review not yet done because I'm too close to the code right now; I'll look at it tomorrow with fresh eyes, and also edit the (outdated) description below. But this is working, and in a pretty good state. The only real design decision remaining is whether to integrate this into the new-project UI flow, allowing people to choose to make a FLEx project or leave the repo empty.
Everything below this line is outdated and I will edit it tomorrow when I'm fresh.
WIP. Not yet ready for review; Claude has made several mistakes that I haven't fixed yet, though the created project does successfully clone in FLEx so the remaining mistakes aren't too serious. Opening draft PR so that this code doesn't only live on my machine.
This is the beginnings of a new Lexbox API to create a new repo populated with a FLEx project, rather than requiring users to create an empty repo and then push into it from FW Classic. So far it is almost entirely vibe-coded and I haven't looked at the code yet, so don't assume that the way it's written is good. There may be Claude-isms that I haven't fixed yet.
Mistakes Claude has made that I know about but haven't fixed yet:
7000072, but FLExBridge has shifted to using750000.7000072instead. (And that model number might change in the future).Design decisions we will need to make:
Design decisions already made:
CanDelete == false— yes,== falserather than!CanDelete). If we ever need to implement similar logic in FW Lite for identifying the main publication, we should not rely on its GUID as that might theoretically be changed in the future.Probably others, I'm sure I'm missing something, but I'm out of time for today.