Skip to content

Rewrite AGENTS.md as an architecture guide and add CLAUDE.md - #401

Open
jasonleenaylor wants to merge 5 commits into
piece3/agent-skillsfrom
piece2/agents-md
Open

jasonleenaylor wants to merge 5 commits into
piece3/agent-skillsfrom
piece2/agents-md

Conversation

@jasonleenaylor

@jasonleenaylor jasonleenaylor commented Aug 28, 2026 •

Copy link
Copy Markdown
Contributor

Rewrites AGENTS.md as an architecture guide and adds CLAUDE.md.

Stacked on #400, which is stacked on #364. Review those first; this PR targets #400's branch.

Why rewrite rather than amend

The old file was an onboarding log rather than guidance. It inventoried the repository root
and the top-level directories, which the repository already states, and it transcribed the
SDK version, the target frameworks and the CI command sequence, which then drifted from
global.json, the .csproj files and ci-cd.yml. The architecture — the generation
pipeline, the model schema, the persistence design — was not described at all.

What the file now carries that cannot be discovered cheaply

  • Editing a Generated*.cs file does not fail. Regeneration is skipped while the file is
    newer than MasterLCModel.xml, so the edit compiles into the local build and the tests
    pass, while CI generates from the XML into a fresh tree and builds different code. The files
    are gitignored, so the edit can never be committed. A fresh clone or worktree has none of
    them until the first build, and a stale one left by a branch switch is compiled as it stands.
  • Per-rule consequences instead of one blanket claim. Rules 2, 3 and 5 corrupt data;
    rule 4 breaks the build; rule 1 does neither, which is what makes it dangerous.
  • The FLEx Bridge metadata cache obligation from WARNING 4.
  • Removing a worktree needs the directory left first, since build output is untracked and
    plain git worktree remove refuses.
  • -p:ParallelizeAssembly=false is not optional: ICU and the writing system subsystems
    hold shared state.
  • Windows builds need the C++ tools whether or not the IDE is used, because code
    generation preprocesses the IDL with cl.exe, located via vswhere.
  • A root build compiles every project, so a stray .cs file from another branch produces
    errors that look like your change broke something.

Corrections

The IOC section described StructureMap. Since #393 the container is
Microsoft.Extensions.DependencyInjection, wrapped in MicrosoftServiceLocator so
GetInstance<T>() still works.

Facts now pointed at rather than copied

The SDK floor, target frameworks and CI sequence point at global.json, the .csproj files
and .github/workflows/ci-cd.yml. The transcriptions are what went stale; the definitions
cannot.

The -m:1 rule from #364 and the instruction to report failing instructions are carried
forward unchanged. The existing GitVersion guidance is also carried forward, with the two
checkouts that break it named alongside it.

Checked against source

The flid arithmetic (LexSenseTags.kflidDefinition is 5016005), LexDb atomic under
LangProject, Styles as an StStyle collection, all seven Overrides*.cs names, the nine
generated files, CheckWinForms in three projects, and every path in the layout tree.

CLAUDE.md imports AGENTS.md and points Claude-only procedures at .claude/skills/,
matching FieldWorks and interlinearizer-extension.

Docs only, so no build or test run is claimed.


This change is Reviewable

jasonleenaylor and others added 5 commits August 28, 2026 12:10
build.cmd, build.sh and build/LCM.proj wrapped MSBuild against LCM.sln. CI has
used dotnet build/test/pack directly for some time and nothing else in the repo
referenced these files, so they only offered a second, diverging way to build.
README.md and AGENTS.md now name dotnet build instead.

Drop the premature Wrapper.Init() call in InitializeIcuAttribute. It ran before
PATH held the lib directories and before ICU_DATA was set, so the first native
ICU load had nothing to find and the environment had to be set by hand.
CustomIcu.InitIcuDataDir, called a few lines later, does both. The catch message
named Wrapper.Init() while guarding InitIcuDataDir; corrected.

Relax global.json from rollForward latestFeature to latestMajor. The 8.0.100 pin
refuses any newer SDK, blocking work on machines with no 8.0.x SDK installed. CI
installs 8.0.x explicitly via setup-dotnet, so the pin was redundant there.

Verified: solution builds clean on SDK 10.0.201, and previously on 9.0.316.
SIL.LCModel.Core.Tests 792/792, SIL.LCModel.FixData.Tests 21/21 and
SIL.LCModel.Tests 1716/1716 pass on net462 and net8.0, with ICU_DATA unset, no
ICU entries on PATH, no Software\SIL\Icu70DataDir registry value and no
%ProgramData%\SIL\Icu70 directory.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The removed wrapper scripts called msbuild without /m, so they were
implicitly serial. dotnet build is parallel by default, and on a tree with
no generated sources yet the GenerateModel and GenerateKernelCs targets can
run concurrently against the same non-per-framework outputs, then fail as
LcmGenerate or IdlImp "returned false but did not log an error".

The failure is intermittent, so the error is easy to misread as a broken
change. README.md and AGENTS.md now name -m:1 for a cold-start build.

Correct the Windows prerequisite to Visual Studio 2022. net8.0 targets were
added in #385 and VS 2019 cannot build them. The C++ build tools are needed
for any Windows build rather than only for building in the IDE, because code
generation preprocesses the IDL with cl.exe.

Drop the AGENTS.md root-file inventory, which duplicated the repository root
and went stale with this change, and ask anyone whose run contradicts the
file to report it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds .claude/skills/ with step-by-step guides for adding a property, adding a
class, adding a virtual property, writing a data migration, and writing tests.
Replaces the onboarding log with guidance an agent can act on. The old file
recorded that dotnet build and dotnet test had FAILED and attributed both to
GitVersion, which sent readers down a dead end; it also inventoried the repo
root and top-level directories, which the repository already states.

What the file now carries that could not be discovered cheaply:

- Editing a Generated*.cs file does not fail. Regeneration is skipped while
  the file is newer than MasterLCModel.xml, so the edit compiles locally and
  the tests pass while CI builds different code from the XML. The files are
  gitignored, so it can never be committed. A fresh worktree has none of them
  until the first build, and a stale one left by a branch switch is compiled
  as it stands.
- Per-rule consequences instead of one blanket claim, since rule 1 neither
  breaks the build nor corrupts data.
- The FLEx Bridge metadata cache obligation from WARNING 4.
- Worktrees need a named branch, because GitVersion cannot version a detached
  HEAD, and removal needs the directory left first.
- ParallelizeAssembly=false is required because ICU and the writing system
  subsystems hold shared state.
- Windows builds need the C++ tools whether or not the IDE is used, because
  code generation preprocesses the IDL with cl.exe.

Corrects the IOC description: the container is
Microsoft.Extensions.DependencyInjection since #393, not StructureMap.

Drops the transcribed SDK version, target frameworks and CI command list in
favour of pointing at global.json, the csproj files and ci-cd.yml, which is
where they are actually defined. The -m:1 rule and the instruction to report
failing instructions are carried forward unchanged.

CLAUDE.md imports AGENTS.md and points Claude-only procedures at
.claude/skills/, matching FieldWorks and interlinearizer-extension.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The rewrite dropped five accurate statements about GitVersion.MsBuild: that
every project uses it, that it needs real git metadata, that CI checks out
with fetch-depth 0, that GitVersion.yml configures it, and the error string
it fails with. Those were correct and worth keeping.

Restores them as a Versioning section, with the part the original lacked:
the two checkouts that break it. A shallow clone reports that it cannot find
the commit and asks for git fetch --unshallow; a detached HEAD reports that
it cannot determine a version without a branch name. Both arrive through
MSBuild as MSB3073 with gitversion.dll exiting 1, so the error alone does
not distinguish them.

Verified by cloning this repository with --depth 1 and running gitversion
against it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jasonleenaylor
jasonleenaylor marked this pull request as ready for review August 31, 2026 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant