Skip to content

Conversation

@jasonleenaylor
Copy link
Contributor

@jasonleenaylor jasonleenaylor commented Feb 9, 2026

Replace the rough-draft AGENTS.md with a comprehensive guide covering critical rules, the code generation pipeline, model schema, persistence infrastructure, and data migration system. Add separate step-by-step task documents in docs/agents/ for common operations: adding properties, writing migrations, adding classes, adding virtual properties, and writing tests.


This change is Reviewable

Replace the rough-draft AGENTS.md with a comprehensive guide covering critical rules, the code generation pipeline, model schema, persistence infrastructure, and data migration system. Add separate step-by-step task documents in docs/agents/ for common operations: adding properties, writing migrations, adding classes, adding virtual properties, and writing tests.

Co-authored-by: Cursor <cursoragent@cursor.com>
@johnml1135
Copy link
Contributor

AGENTS.md line 30 at r1 (raw file):

### Prerequisites
- .NET SDK 8.x

So, this can be compiled in any of the 3 frameworks? What version of mono? Why 3? One sentence would be helpful here to understand.

@johnml1135
Copy link
Contributor

AGENTS.md line 21 at r1 (raw file):

2. **Model changes require a version bump and migration.** Almost every change to `MasterLCModel.xml` requires incrementing the `version` attribute and writing a data migration class. The ONLY exceptions are: editing `<comment>`/`<notes>` elements, editing XML comments, or adding attributes that only affect the code generator. Read the warnings at the top of `MasterLCModel.xml` carefully.

3. **All data changes must occur within a UnitOfWork.** Use `UndoableUnitOfWorkHelper` for user actions or `NonUndoableUnitOfWorkHelper` for system operations. Changes outside a UOW will throw or silently fail.

Can this be enforced? Can the write commands only be accessible through these two commands?

@johnml1135
Copy link
Contributor

AGENTS.md line 23 at r1 (raw file):

3. **All data changes must occur within a UnitOfWork.** Use `UndoableUnitOfWorkHelper` for user actions or `NonUndoableUnitOfWorkHelper` for system operations. Changes outside a UOW will throw or silently fail.

4. **No references to `System.Windows.Forms`.** The build enforces this via the `CheckWinForms` target.

Why would Windows Forms be in here at all? Why reference it if it is not in the repo? Does this need one more sentence or clause of explanation?

@johnml1135
Copy link
Contributor

AGENTS.md line 33 at r1 (raw file):

- Windows: .NET Framework 4.6.1 targeting pack
- Linux: mono-devel, icu-fw packages
- Full git history (GitVersion.MsBuild requires `git fetch --unshallow` or `fetch-depth: 0`)

Is this something specific to this repo? Why is it best recorded here?

@johnml1135
Copy link
Contributor

AGENTS.md line 37 at r1 (raw file):

### CI Commands (GitHub Actions, `.github/workflows/ci-cd.yml`)

dotnet build --configuration Release

Does this section add value? We probably need the test script called out (if there is one). Or make a testing.instructions.md file.

@johnml1135
Copy link
Contributor

AGENTS.md line 48 at r1 (raw file):

### Known Issues
- GitVersion.MsBuild requires full git metadata. Shallow clones will fail.

Can we fix this programmatically? When we start a build (and need it), can we ensure that we are not in a shallow clone in a quick way? A fix is better than a known issue...

@johnml1135
Copy link
Contributor

AGENTS.md line 49 at r1 (raw file):

### Known Issues
- GitVersion.MsBuild requires full git metadata. Shallow clones will fail.
- `NU1701` warnings are expected; treat as warnings unless the build breaks.

This should probably be in a comment in a project file rather than here. These should just be ignored programmatically with a rationale (if needed).

@johnml1135
Copy link
Contributor

AGENTS.md line 61 at r1 (raw file):

3. Processes NVelocity templates (`LcmGenerate/*.vm.cs`) to produce the 9 generated C# files

The generated code provides: class ID/field ID constants, interfaces, concrete implementations, factory interfaces and implementations, repository interfaces and implementations, the backend provider's `ModelVersion` constant, and StructureMap DI bootstrapping.

Can one more sentence be added as to why this architecture was chosen? It is unobvious to me. Is there an article that explains this paradigm? Do newer versions of DotNet make this irrelevant, but we want to still support mono? Is this a good long term strategy? If you feel we need a few paragraphs to explain, can you reference a document or wiki, etc?

@johnml1135
Copy link
Contributor

AGENTS.md line 173 at r1 (raw file):

When performing any of the following tasks, read the linked guide first:

- **Adding a property to an existing class**: [docs/agents/adding-a-property.md](docs/agents/adding-a-property.md)

Is it best practice to put these here? Should they be grouped with the actual relevant data?

@johnml1135
Copy link
Contributor

docs/agents/adding-a-property.md line 2 at r1 (raw file):

# Task: Adding a Property to an Existing Class

Are these files generated from a user guide that was fed into Claude? How have they been validated? I don't know enough to know if they are right.

@johnml1135
Copy link
Contributor

docs/agents/adding-a-property.md line 75 at r1 (raw file):

Create: `src/SIL.LCModel/DomainServices/DataMigration/DataMigration7000073.cs`

**For new optional properties with safe defaults (most common case)**, existing data doesn't need modification. But you still need the migration class:

Is this a duplication of data migration? You can ask Claude "Check for duplicated code here" "Refactor to have an appropriate amount of context for a broad scope of work".

@johnml1135
Copy link
Contributor

docs/agents/adding-a-new-class.md line 1 at r1 (raw file):

# Task: Adding a New Class to the Model

Do these show up in VSCode Copilot extension? If we want new agents, they should show up for GitHub and for claude. We should follow "https://docs.github.com/en/copilot/reference/custom-agents-configuration". These are custom agents (specific general tasks that need to follow a workflow and have certain things they can an cannot do. They may also need a bit of reformatting (head matter, etc.).

@johnml1135
Copy link
Contributor

docs/agents/adding-a-new-class.md line 3 at r1 (raw file):

# Task: Adding a New Class to the Model

This guide covers adding an entirely new class to `MasterLCModel.xml`. This is less common than adding properties and more involved -- it touches the model, requires a migration, may need an owner property on an existing class, and may need hand-written partial class logic.

You may also want a impact-assessment subagent (or similar). Sending off sub-agents to research the codebase and then come back is often effective. YOu can create a custom agents and then call it out in other agents to use as appropriate.

@johnml1135
Copy link
Contributor

docs/agents/writing-a-data-migration.md line 3 at r1 (raw file):

# Task: Writing a Data Migration

Data migrations transform existing persisted data when the model version changes. They operate on raw XML via `DomainObjectDTO` objects -- live `CmObject` instances are NOT available during migration.

This sounds very formal. Will this become the formal documentation or is it written somewhere else? Can we reference it or can this become it?

@johnml1135
Copy link
Contributor

docs/agents/writing-tests.md line 2 at r1 (raw file):

# Task: Writing Tests

Writing tests are better as skills. Skills are reusable. We should also have a debugging skill (with specific debugging paradigms for this repo).

You can have a custom agents be for writing tests, but I would put most of the logic in a skill.

Migrations should also be a skill - it's kind of cross cutting concerns.

Does the distinction make sense?

https://github.com/orgs/community/discussions/183962
https://danielmiessler.com/blog/when-to-use-skills-vs-commands-vs-agents

Copy link
Contributor

@johnml1135 johnml1135 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@johnml1135 reviewed 6 files and all commit messages.
Reviewable status: all files reviewed, 15 unresolved discussions (waiting on @jasonleenaylor).

Copy link
Contributor Author

@jasonleenaylor jasonleenaylor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jasonleenaylor made 7 comments.
Reviewable status: all files reviewed, 15 unresolved discussions (waiting on @johnml1135).


AGENTS.md line 21 at r1 (raw file):

Previously, johnml1135 (John Lambert) wrote…

Can this be enforced? Can the write commands only be accessible through these two commands?

It is enforced actually, this is just making sure the agent understands that. Actually there is better guidance I can give here. The silently fail is not accurate.


AGENTS.md line 23 at r1 (raw file):

Previously, johnml1135 (John Lambert) wrote…

Why would Windows Forms be in here at all? Why reference it if it is not in the repo? Does this need one more sentence or clause of explanation?

We specifically guard against developers who tend to add things like message boxes or other things from System.Windows.Forms (there is a build target that guards against it). The AI picked up on that, which seems fine.


AGENTS.md line 48 at r1 (raw file):

Previously, johnml1135 (John Lambert) wrote…

Can we fix this programmatically? When we start a build (and need it), can we ensure that we are not in a shallow clone in a quick way? A fix is better than a known issue...

It is a known GitVersion issue I believe, not an issue of ours.


AGENTS.md line 61 at r1 (raw file):

Previously, johnml1135 (John Lambert) wrote…

Can one more sentence be added as to why this architecture was chosen? It is unobvious to me. Is there an article that explains this paradigm? Do newer versions of DotNet make this irrelevant, but we want to still support mono? Is this a good long term strategy? If you feel we need a few paragraphs to explain, can you reference a document or wiki, etc?

If you are referring to the class id, field id constants that was to provide fast reflection and support data migration capabilities which didn't require instantiating C# classes. It has nothing to do with mono.
This markdown is specific to the agents, so I don't know that adding paragraphs to explain it is valuable.


AGENTS.md line 173 at r1 (raw file):

Previously, johnml1135 (John Lambert) wrote…

Is it best practice to put these here? Should they be grouped with the actual relevant data?

I'm not sure if this is best practice. this is what I got when I asked it to keep the context which an agent starts with as small as reasonable but to have instructions for specific common tasks with best practices. I was wondering about this and thinking they should really be 'skills' that live somewhere. I'm not sure on the best practice if there is one for vendor unlocked skills files.


docs/agents/adding-a-property.md line 2 at r1 (raw file):

Previously, johnml1135 (John Lambert) wrote…

Are these files generated from a user guide that was fed into Claude? How have they been validated? I don't know enough to know if they are right.

Yes, these files came from a combination of a model description pdf and specific code file guidance that I gave. I've mostly validated them but I was actually executing a test model change to validate some of this.


docs/agents/writing-a-data-migration.md line 3 at r1 (raw file):

Previously, johnml1135 (John Lambert) wrote…

This sounds very formal. Will this become the formal documentation or is it written somewhere else? Can we reference it or can this become it?

This is a description of how the datamigration code works and it is giving the agent instructions on how to successfully add one (I'm currently testing if these instructions actually result in successful agent action)

jasonleenaylor and others added 2 commits February 11, 2026 16:52
Replace the rough-draft AGENTS.md with a comprehensive guide covering critical rules, the code generation pipeline, model schema, persistence infrastructure, and data migration system. Add separate step-by-step task documents in docs/agents/ for common operations: adding properties, writing migrations, adding classes, adding virtual properties, and writing tests.

Co-authored-by: Cursor <cursoragent@cursor.com>
* Move migration details to skill
* Improve handling of icu tests to remove the need for
  bootstrapping by agents
* Remove the obsolete build commands

Co-authored-by: Cursor <cursoragent@cursor.com>
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.

2 participants