Lexbox API to create empty FLEx project

[rmunn]

This is 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. Due to our using the LfMergeBridge code (which refuses to create a new Mercurial branch), projects created with this code end up with three initial commits in Mercurial, rather than the two initial commits that projects created in FLEx end up with. But one of them is an empty commit that simply changes branches to 750000.7000072 (and creates the branch in the process), and that doesn't cause any problems for Chorus or FLExBridge.

Self-review done, have identified a few issues to work on. Mostly comment editing, but a couple unit tests I want to add (and one method I want to get rid of).

Fixes #2589.

[coderabbitai]

📝 Walkthrough

Walkthrough

Changes

Template Project Creation

Layer / File(s)	Summary
Template assets and writing-system construction backend/FwHeadless/FwHeadless.csproj, backend/FwHeadless/FwHeadlessConfig.cs, backend/FwHeadless/FwHeadlessKernel.cs, backend/FwLite/...	Packages templates and supports creating projects with multiple analysis, vernacular, and UI writing systems.
Headless repository creation workflow backend/FwHeadless/Services/*, backend/Testing/FwHeadless/*	Adds repository setup, branch and commit operations, template generation, version checks, cleanup, and creation/sync reservation handling.
Headless project creation endpoint backend/FwHeadless/Program.cs, backend/FwHeadless/Routes/ProjectRoutes.cs	Registers and validates the internal project-creation endpoint before invoking the creation service.
LexBox API orchestration and validation backend/LexBoxApi/*, backend/LexCore/Entities/*, backend/Testing/LexBoxApi/*, backend/Testing/SyncReverseProxy/*	Adds the admin API flow, HTTP client contract, compensating cleanup, timeout configuration, and unit/integration coverage.

Estimated code review effort: 4 (Complex) | ~60 minutes

Suggested reviewers: hahn-kev

Poem

A rabbit packed templates neat,
With writing systems, rows complete.
Mercurial branches hop in line,
APIs bloom by carrot-sign.
Tests thump softly: “All is right!”

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 19.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title identifies the LexBox API and FLEx project creation, which are central changes. However, “empty” is imprecise because the API creates a repository populated from a FLEx project template.
Description check	✅ Passed	The description directly explains the new LexBox API, template-populated repository flow, Mercurial branch behavior, and related issue.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/create-empty-fwdata-project

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[rmunn]

Ah, I see: the security risk is forging new log entries via inserting newlines (or CRLF). We should be sanitizing the project code anyway, at an earlier point than this.

[rmunn]

Yes, we are sanitizing the project code, and can't hit this code path with unsanitized user data. False positive.

[argos-ci]

The latest updates on your projects. Learn more about Argos notifications ↗︎

Build	Status	Details	Updated (UTC)
default (Inspect)	⚠️ Changes detected (Review)	1 changed	Aug 24, 2026, 5:25 AM
e2e (Inspect)	✅ No changes detected	-	Aug 24, 2026, 5:32 AM

[rmunn]

Rebasing on top of develop before I push my new work from today.

[rmunn]

@coderabbitai fullreview

[coderabbitai]

@rmunn: Starting a complete re-review of PR #2439, including the rebased changes and current draft implementation.

✅ Action performed

Full review finished.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (3)

backend/LexBoxApi/Controllers/ProjectController.cs (1)

53-56: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Dead code is null check.

code is a non-nullable string bound 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, making code is null unreachable. 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 win

Parameter order flips between CreateFromTemplate, BuildFromTemplate, and NewProject.

CreateFromTemplate takes (vernacular, analysis, ui), BuildFromTemplate also takes (vernacular, analysis, ui), but NewProject takes (analysis, vernacular, ui). All three currently thread the right list to the right slot (verified by parameter-name binding), but with two identically-typed IReadOnlyList<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 win

Mock doesn't override the new list-based NewProject overload.

MockFwProjectLoader only overrides the single-pair overload. The new NewProject(project, IReadOnlyList<string>, IReadOnlyList<string>, string) overload is virtual on the base ProjectLoader but 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

📥 Commits

Reviewing files that changed from the base of the PR and between 17d693e and b32667b.

📒 Files selected for processing (21)

• backend/FwHeadless/FwHeadless.csproj
• backend/FwHeadless/FwHeadlessConfig.cs
• backend/FwHeadless/FwHeadlessKernel.cs
• backend/FwHeadless/Program.cs
• backend/FwHeadless/Routes/ProjectRoutes.cs
• backend/FwHeadless/Services/ISendReceiveService.cs
• backend/FwHeadless/Services/ProjectCreationService.cs
• backend/FwHeadless/Services/SendReceiveHelpers.cs
• backend/FwHeadless/Services/SendReceiveService.cs
• backend/FwHeadless/Services/SyncHostedService.cs
• backend/FwLite/FwDataMiniLcmBridge.Tests/Fixtures/MockFwProjectLoader.cs
• backend/FwLite/FwDataMiniLcmBridge/LcmUtils/ProjectLoader.cs
• backend/FwLite/FwLiteProjectSync.Tests/ProjectTemplateTests.cs
• backend/LexBoxApi/Controllers/ProjectController.cs
• backend/LexBoxApi/LexBoxKernel.cs
• backend/LexBoxApi/Services/FwHeadlessClient.cs
• backend/LexBoxApi/Services/ProjectService.cs
• backend/LexCore/Entities/CreateProjectFromTemplateInput.cs
• backend/Testing/FwHeadless/SyncHostedServiceCreationReservationTests.cs
• backend/Testing/LexBoxApi/CreateFromTemplateValidationTests.cs
• backend/Testing/SyncReverseProxy/CreateProjectFromTemplateTests.cs

[rmunn]

Ready for review, but not yet ready to merge: there's a subtle writing-system-related bug I'm trying to pin down (see comment below).

[rmunn]

This is causing a bug: when a writing system ID would be canonicalized to a different value (e.g., one test project had qaa-x-qaa-v which SIL.Core canonicalizes to qaa-x-v), and then LfMerge produced the error "Unable to set writing system 'qaa-x-v' because this id already exists." I'm still working on a solution.

[rmunn]

Solution implemented in an LfMerge PR; I'll merge that first, then this won't be a problem any more.

[rmunn]

The Claude-generated code in 02c41f5 makes ProjectService.CreateProject and ProjectService.CreateDraftProject inconsistent with each other, and also with the GraphQL CreateProject mutation. Claude only added the optional projectOrigin parameter to ProjectService.CreateProject, and didn't add it into CreateProjectInput.

Is that what we want? Pro: no change to graphQL inputs (apart from the new enum value), so no need for the GraphQL mutation handlers to implement the "admin-only" rule on setting ProjectOrigin in a new project. Con: inconsistency, no way for this to be set via GraphQL.

I'm inclined to modify the CreateProjectInput to include that enum as an optional parameter (i.e. a nullable enum), so that the GraphQL mutations are consistent with the ProjectService methods. But that does mean a wider area where we need to get the admin checks right.

EDIT: No, wait, that doesn't make sense. Our rule is that only admins can set ProjectOrigin, and admins will never be creating draft projects, so anyone creating a draft project would never be allowed to set ProjectOrigin anyway, so it makes no sense for CreateDraftProject to take that parameter. The inconsistency beteen CreateProject and CreateDraftProject is just fine here.

[rmunn]

Did a self-review. Lots of places where a comment needs to be edited or corrected, and a couple minor code changes. But this is close to being ready to merge. I do want to merge an LfMerge bugfix first, though.

[rmunn]

This URL scheme is inconsistent with the one from the LexBox API, where it's /api/project/initFwDataProject. I'll probably update this one to initFwDataProject instead, to stay consistent.

[rmunn]

This error message will become inconsistent with the API shape once we rename it (in fact it already is inconsistent, since it starts with a capital letter). I'll just edit it to not have the API endpoint in the message, since when you get the error message you already know what API endpoint you called.

[rmunn]

Might allow null wsUi values, which will then be passed in to libLcm's CreateNewLangProj (which will in turn select "en" as the default). We have code in ProjectController selecting "en" as the default, but we should perhaps leave the default decision up to libLcm instead, and just pass a null straight through.

[rmunn]

I'm uncertain if adding a CommitEmpty method to the interface is the right approach; it's only used in one place, and I think I'd rather fold this into the SetBranch method instead (renaming it to SwitchBranch, so that it will switch the branch and commit — and it could take a boolean, default true, for whether to run hg commit after switching branches). That would reduce the surface area being added to ISendReceiveService here.

The other two methods, InitRepo and SetBranch, do make sense to get added to the interface. But I have never liked CommitEmpty, and now I know what to do about it.

[rmunn]

I believe this part of the comment is wrong; I'll double check the actual steps, but I think the minimal CustomProperties file gets created first and then the .fwdata file gets built.

[rmunn]

I should also make a note in this comment about how Chorus wants commit 0 to be on Mercurial's default branch, not on a numbered branch. Because Chorus uses commit 0 to track a repo's history, i.e. do these two copies of a repo actually share an ancestor and come from the same FW project or not? My first attempt created the project without a default branch, and caused Chorus to complain. So that's why we commit a file first before switching branches; I'm not sure that's covered in the comments yet.

[rmunn]

Add the LanguageForgeNonSR case here, since we'll actually be using that one

[rmunn]

Should those helper methods even be public? Do we actually need unit tests to test that the ?? operator works? Really unsure of the value of these tests. At least they're fast.

[rmunn]

Let's make these six or seven items long, and test actual equivalence including order, because we want to ensure that the writing systems lists coming from the API will be created in the order they're asked for. (There are old Language Forge projects that we will be migrating with multiple writing systems, and it would be nice to ensure that their order is preserved).

[rmunn]

We can avoid the mentioned issue with "en" by simply ensuring that "en" is part of the analysis writing systems the test expects. Then we can check the order, which can matter in the case of projects being migrated from Language Forge,

[rmunn]

Might also want to add a test that uses a NON-admin API tester, and ensures that the projectOrigin param will be rejected. That test will currently fail because the API is [AdminRequired], but it will guard against regression when we eventually remove the [AdminRequired] attribute. (We can verify that the test does what it should by temporarily removing [AdminRequired] and doing a rebuild and local test run, then ensuring that the API call continues to fail as it should).