Conversation
The file is loaded in full at the start of every session, so anything a session can reconstruct by reading the repo is a per-session cost with no payoff. Removed: - the developer-install script, duplicated from docs/How-to/installation.md, which the surrounding text already tells the reader to prefer - standard pytest, coverage, and ruff invocations, restated up to four times across the file; the non-guessable PR filter expression stays - the directory layout, config-file list, and CLI entry points, all visible from ls and proteus --help - the style rules ruff enforces from pyproject.toml; the two conventions it does not enforce stay - the code-change, new-module, and test-debugging walkthroughs, which restate rules stated once already - the Key Dependencies list, a third copy of the ecosystem module table - one of three copies of the coverage-ceiling values Kept every gotcha, prohibition, and piece of design rationale: the FWL_DATA/RAD_DIR and PETSc traps, the conda-worktree hardlink trap, the SOCRATES build flags, the plot standards, the voice rule, and the oxygen-accounting contract. Also fixed a pointer to a memory file that does not exist; the canary recipe it accompanied is stated inline and stays. 637 lines to 387; 40,057 characters to 31,707.
Both blocks matter only while working on one subsystem, but sat in a file that loads in full on every session. They move to .github/.claude/rules/, alongside the test and review rules, and the root file keeps a short trigger paragraph naming when to go read them. Nothing is lost: the portable-flags rationale, the bit-reproducibility recipe, the four O_mode definitions, the D1A design note, and both runtime guards are reproduced in full in the new files, with the aggregation-site list cross-linked to proteus-code-review.md instead of restated. 387 lines to 377; 31,707 characters to 30,396. The two blocks are 4,300 characters; the trigger paragraphs and the new entries in the rule-file list give about a quarter of that back.
Ten subsections of Testing Standards were reproduced almost verbatim in both copilot-instructions.md and proteus-tests.md, under a stated contract to keep the two in sync by hand. The root file loads into every session in full while the deep-dive is read on demand, so the duplication was paid for on every session including those that never touch a test. The root file now keeps the part a reader needs before knowing whether tests are in scope: the structure rule, the marker table with timeout budgets, the physics-invariant tiers, the anti-happy-path rules and forbidden patterns, and the certification markers. A mapping table sends the rest to the deep-dive section that states it: float comparison, discrimination guards, mocking discipline, importorskip, the monkeypatch trap, seeding and wall-time budgets, per-test documentation, the review trigger, tooling, and the coverage gates. The voice rule stays in the root file. It governs every commit message and pull-request body rather than only test-touching work, so it has to be present whether or not the deep-dive was read. proteus-tests.md now describes the split instead of claiming full sync: clauses it alone states can change there alone, clauses the root file also states change in both. 377 lines to 335; 30,396 characters to 24,926.
The path named was src/tests/, which does not exist in this repository.
egpbos
changed the title
egpbos
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #798 +/- ##
========================================
Coverage 94.71% 94.71%
========================================
Files 119 119
Lines 19165 19165
Branches 3281 3440 +159
========================================
+ Hits 18152 18153 +1
+ Misses 1013 1012 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Looks good, I really do hope it works as they say with the new version. All duplications in these files were hard-won lessons where Claude would ignore rules and so the rules and gotchas were duplicated high-up to actually make it in production. I presume we have to try it out to see if it works in reality; it may be we have to build it back a bit if it's now too stripped down. |
|
Yeah, it's a bit of a gamble, for sure and we'll have to see how it works out. But the repeated instruction thing you mention is something that they also explicitly mention is no longer necessary (see the blog post here: https://claude.com/blog/the-new-rules-of-context-engineering-for-claude-5-generation-models#:~:text=Then:%20Repeat%20yourself,the%20system%20prompt.). Btw, one other worry might be that this could make the instructions less performant on older models if people still use those, or for Copilot (I still sometimes use it for quick things inside vscode for instance; I should probably just switch there too). Ideally, someone with a lot of experience with the existing setup would test-drive this for a week or two. I personally don't think I would be able to tell the difference right now, so maybe someone wants to volunteer? The "robust" approach would be if we make a full PROTEUS-LLM-benchmark suite of tests, but probably overkill :) |
|
This reads well and the split looks right to me. One thing I would like to add, and it belongs in the "Two conventions ruff does NOT enforce" list you kept at Code Quality rather than anywhere new: a cap on inline comment length, at most 5 lines and ideally 1 to 2. The reason is agent-written comments specifically. Scanning One line would do it, in the same shape as the others: "Inline comments: at most 5 lines, ideally 1 to 2. Longer explanation belongs in the docstring, the commit message, or the PR body." I am raising it here rather than opening a competing PR because this is your file to shape and it would only collide. I am aware it is an addition to a change whose whole point is subtraction, so please drop it without ceremony if you would rather keep this purely subtractive; I can open it separately once this merges. |
|
The split is the right call, and the two new subsystem files carry their content faithfully. I checked the moved test clauses against the sections the mapping table points at, and they are there. Two things to fix before this comes out of draft. The branch is behind main, and the one change main has made to The sync contract now says two different things. One small one: the mapping table sends wall-time budgets to section 10, which has seeding and |
The oxygen accounting paragraph from main, with the vapourise relaxation of the M_atm <= M_planet check, goes into the moved rule file .github/.claude/rules/proteus-oxygen-accounting.md.
The ecosystem repos share part of their agent instructions. tools/agents/sync_core.py writes the canonical text into each repo's AGENTS.md files between fwl-<name> markers and records a hash of the text in the begin marker; --check reports repos that lag behind without writing. tools/agents/check_agents_md.py is copied unchanged into every repo and runs offline: it fails when a shared block no longer matches its hash, when the root AGENTS.md exceeds 16,000 B or a nested one 12,000 B, when CLAUDE.md is anything other than a regular file holding @AGENTS.md, and when .github/copilot-instructions.md exceeds 60 lines. The byte caps keep every file below the size at which agent tools truncate instruction files.
tools/agents/core.md holds the rules every ecosystem repo shares: what each module does in PROTEUS, why an interface change must be checked against the PROTEUS call sites, the unit conventions that differ between modules, the physics-test and tolerance rules, and the branch and pull-request rules. tools/agents/tests-core.md holds the test rules shared by the repos that use the tier markers and check_test_quality.py. tools/agents/voice-neutral.md is the rule for commit messages and public text. sync_core.py writes these texts into the fwl-core, fwl-tests-core and fwl-voice blocks of each repo's AGENTS.md files.
AGENTS.md is the instruction file that coding agents read by default. CLAUDE.md now only imports it, because an agent that reads CLAUDE.md ignores AGENTS.md when both exist. The first lines state what a first edit needs: where tests go, what must be conserved, and the commands that decide whether a change is ready. The repository section carries the environment traps, how to run and resume PROTEUS, the physics and coupling contract, the review points and the code organisation rules. .github/copilot-instructions.md is cut to the commands and the review checklist, for the chat and review surfaces that read only that file. Corrections against the code: O_mode defaults to ic_chemistry; installation.md describes the installer and manual_installation.md the manual steps; the pre-commit hook runs ruff check but not ruff format, which CI checks; the resume conditions are stated without line numbers.
The test rules load when an agent works in tests/, instead of depending on a pointer in the root file. The shared block states the tier markers, the rules check_test_quality.py applies and the physics markers; the PROTEUS part covers the physics modules, discriminating values, mocks, fixtures, coverage gates and failure patterns. Corrections against the code: validate_test_structure.sh checks directories, not the one-to-one file rule; conftest.py has no config_earth fixture; reference-pinned pages are per source file under docs/Validation/<module>/<file>.md.
The code-review, oxygen-accounting and SOCRATES-build notes move from .github/.claude/rules/ to .github/agent-rules/ and are linked from AGENTS.md; proteus-tests.md is replaced by tests/AGENTS.md. References in pyproject.toml, check_test_quality.py and the test docstrings point to the new files and name sections instead of numbers.
check_agents_md.py replaces the line-count hook on copilot-instructions.md: it checks the shared-block hashes, the byte caps of the AGENTS.md files, the CLAUDE.md import and the length of copilot-instructions.md, and runs in the PR lint job.
A nightly job clones the FormingWorlds module repositories and runs sync_core.py --check against them, so a module that has not taken the current shared text shows up as a failed job. The job is not part of the nightly status, so a lagging module does not mark the science nightly red.
hf_row holds pressure in bar and time in years (GetHelpfileKeys), and config keys carry their own units, planet mass in M_earth among them. The element aggregation sites all include oxygen but do not share one element set: the mass sites sum vol_element_list + noble_gases and leave rock vapour out on purpose, while escape and structure sum element_list; the rules now say so instead of asking for identical sets. assert_mass_conservation runs after the outgassing step and its species sum covers the volatile species only. O_mode ppmw scales O_budget by the volatile reservoir mass, and check_ic_oxygen_budget runs only for fO2_source = user_constant with a user oxygen budget. The Aragog entropy solver reads P-S tables, and its P-T tables are phase-specific when PALEOS-2phase tables exist. The hf_row override example restores every overridden key. The escape bound compares the mass lost in one step with the atmosphere. SOCRATES uses OMPARG only for the corr_k tool, so the reproducible-build recipe drops the OMPARG edit and rebuilds the AGNI wrappers that get_socrates.sh removes. The shared blocks name the config values in lowercase with their config keys, include the boundary option, describe PROTEUS from outside so they read correctly in PROTEUS itself, and require one tier marker per test. The test-quality lint is described as a count comparison against the baseline.
An agent that reads CLAUDE.md, with one at the repository root, loads a nested AGENTS.md only through a CLAUDE.md in the same directory, so tests/CLAUDE.md imports it. check_agents_md.py now requires that import next to every AGENTS.md, requires a root AGENTS.md, also checks untracked files, skips files deleted from the working tree, handles paths with spaces, counts block markers only at line starts, and fails on a path that is not a directory; sync_core.py fails on such a path too. The PR lint job also runs sync_core.py --check, so PROTEUS's own blocks cannot drift from tools/agents/.
The comments that now reference tests/AGENTS.md keep their content in at most three lines.
hf_row units are stated per key in GetHelpfileKeys: atmospheric pressures in bar and model time in years, but interior pressures such as P_cmb in Pa and orbital periods in s, so the rules point there instead of stating one convention. check_desiccation tests the volatile elements against its threshold and sums element_list for the escape balance, matching the M_vol_initial baseline. Escape per step is bounded by limit_escape_step at ESCAPE_STEP_MAX_FRAC of its reservoir. Resume needs a complete interior and atmosphere snapshot pair named by simulation time. Integration and slow tests live in tests/integration/. FWL_DATA_DIR is frozen at import in four modules, not one. The check_test_quality float rule exempts exact zero. In the SOCRATES notes, a changed flag string requires an update of the rewrite in get_socrates.sh, and the AGNI wrappers must be rebuilt after every direct call of that script.
…terion Integration tests must sit in tests/integration/, which the nightly integration tier runs; a slow test file can sit in any test directory and runs when a shard of the slow-tier matrix lists it. The short Copilot file now carries the same hf_row units and escape bound as AGENTS.md. check_test_quality.py --check fails against the committed baseline on main, so the ready criterion is that no rule's count rises above the same run on origin/main. The review notes name the desiccation threshold list, the full path of limit_escape_step and O_kg_total in the FeO mode, and drop an example that is not a config input.
An inline comment block stays at 2 lines or fewer and never exceeds 4, because a long block drifts from the code it describes; a longer explanation goes in the docstring, the commit message or the pull-request description. The rule sits in tools/agents/core.md, so every repository gets it through sync_core.py.
check_agents_md.py fails when the root AGENTS.md has no fwl-core block or tests/AGENTS.md has no fwl-tests-core block, so deleting a shared block no longer passes. New tests pin the block hash to its full 16-digit prefix and check that sync_core.py rewrites a block whose body was edited under the canonical hash. The agents-md-sync job now counts in the nightly status, so a module that lags behind tools/agents/ shows as a failed nightly, and the job also checks Obliqua. Module repositories keep the vendored hash, so an edit of the core in PROTEUS does not turn their pull-request checks red.
The installer is bash install.sh. Resume needs no atmosphere snapshot when atmos_clim.module is dummy. Proteus.start() sets config.params.resume and offline, and the Zalmoxis structure call changes config.orbit.module only inside try and restores it in finally, so the rule describes that pattern. PR CI runs the four docs generators with --check, and a coupled interface change bumps the module pin in pyproject.toml; both are stated. A documented exception is tested together with the check that no side effect ran. The oxygen notes merge into .github/agent-rules/code-review.md without the internal design label, and that file keeps only what AGENTS.md does not say. The Review section keeps the physics bounds, the EOS tables and validator liveness, and names copilot-instructions.md as its copy. The hf_row units are stated once, in the shared block; the coverage table and details found in seconds in the docs are dropped, the fail_under rule stays. The shared voice block covers skip reasons, parametrize ids, shipped log strings and CI job names, and excludes internal plan labels and em or en dashes. The shared test block no longer points to itself.
The resume rule notes that the dummy and boundary interiors write no interior snapshot. The oxygen statements distinguish fO2_source 'user_constant', where fO2 is buffered at outgas.fO2_shift_IW, from 'from_O_budget', where the O budget sets fO2. The core bullet names M_core, the hf_row key both Zalmoxis and SPIDER write, and how SPIDER keeps it consistent. tests/AGENTS.md gives the full proteus.* module paths that read FWL_DATA_DIR. The Copilot checklist names integration tests in tests/integration/ and says which parts of AGENTS.md it repeats. The checker's sync error names the sync_core.py repository argument.
A new test uses a marker hash that matches the body hash in its first 8 hex digits only; the checker and the sync must both report it. The pin rule names [project.optional-dependencies] for fwl-vulcan and atmodeller, the config rule says the override is restored in a finally block, and code-review.md keeps only the oxygen and mass-check details that AGENTS.md does not state.
timlichtenberg
changed the title
The checker passed a repository whose tests/AGENTS.md was missing, because it only checks the AGENTS.md files it finds. It now reports the missing file whenever tests/ exists, and a test covers it.
|
@egpbos could you review all the changes on this branch, including the ones I added on top of yours? After my merge of main (1ff4e0b), the agent instructions move to ZEPHYRUS already has the same structure (FormingWorlds/ZEPHYRUS#38, merged today), so this PR is the one that makes the canonical block exist. The only red check was codecov/project, from a nightly coverage flag carried over from July; a nightly on this branch is running now to refresh it. If it looks good to you, feel free to mark it ready for review. |
This branch has not been deployed
2 participants
Description
This builds on Patrick's trim of the agent instructions and moves them to the
AGENTS.mdformat, which most coding agents read. The rootAGENTS.mdholds the rules for the repository andtests/AGENTS.mdthe rules for tests;CLAUDE.mdandtests/CLAUDE.mdimport them, and.github/copilot-instructions.mdkeeps a short checklist that points toAGENTS.md. Longer review notes are in.github/agent-rules/code-review.md, and.github/.claude/rules/is gone.The rules that every PROTEUS module shares (units at module boundaries, physics tests that fail for the most plausible wrong formula, commit and pull-request text, inline comments of at most 4 lines) have one canonical copy in
tools/agents/core.mdandtools/agents/tests-core.md.tools/agents/sync_core.pywrites them into a repository'sAGENTS.mdfiles with their sha256, andtools/agents/check_agents_md.pyfails when a shared block no longer matches its hash or is missing, when a file exceeds its size limit (16,000 B at the root, 12,000 B intests/), or when aCLAUDE.mddoes not import itsAGENTS.md. The check runs in pre-commit and in the PR checks and replacestools/check_file_sizes.sh; a nightly job compares every module's copy with the canonical one and reports innightly-status. ZEPHYRUS follows in FormingWorlds/ZEPHYRUS#38, the other modules after this.The PROTEUS text was checked against the code and corrected where it was out of date, for example the installer path, the resume requirements, the
hf_rowunits, and the oxygen and mass-conservation rules. A few test docstrings and markers change so that the files follow the test rules they describe, and the marker descriptions inpyproject.tomlandtools/check_test_quality.pypoint totests/AGENTS.md.Validation of changes
python tools/agents/check_agents_md.py .andpython tools/agents/sync_core.py --check .pass;tests/tools/test_agents_md.py(21 tests) fails for a missing or edited shared block, a hash that differs after the eighth digit, a size above the limit, and a missing import.ruff checkandruff format --checkare clean, andbash tools/validate_test_structure.shpasses.Checklist