Skip to content

M0.P1: Repository hygiene and CI - #67

Merged
kashgummaraju merged 25 commits into
integration/1.20.1from
m0.p1/repo-hygiene
Oct 25, 2026
Merged

kashgummaraju merged 25 commits into
integration/1.20.1from
m0.p1/repo-hygiene

Conversation

@kashgummaraju

@kashgummaraju kashgummaraju commented Oct 25, 2026 •

Copy link
Copy Markdown
Member

Summary

This pull request cleans the repository and adds CI. It changes no gameplay.

  • The unit tests run with JUnit 5 on ./gradlew :common:test. The stand-in runner is gone. Each run prints the test count and fails if it finds no test.
  • The 16 GameTest bodies are in common. A Forge holder and a Fabric holder register them. The structures are in common resources.
  • A GitHub Actions workflow builds both loaders, runs the unit tests and uploads the two mod jars on every push and pull request.
  • The README tells a new contributor how to build, run, test and contribute. A pull request template carries the Definition of done.
  • docs/config-reference.md lists every TOML key. docs/performance-baseline.md sets the target player PCs and how every fps and tick-time gate is checked.
  • Template remnants, toml4j, @OnlyIn in common and the mergetool-api stub are gone. The ignore rules keep git status clean after a build.

Work package

M0.P1: task plan. Spec: engineering backlog, M0.P1.

Acceptance

Line pass/fail Evidence
CI is green on the integration branch for both loaders. pass on this branch Every run on this branch is green, for example 40213048639, 40214813775 and 40218845565. Each uploads one Forge jar and one Fabric jar. The first run on integration/1.20.1 after the merge is the acceptance run.
git status is clean after a build. pass After ./gradlew build :common:test, git status --short shows only the lead's local CLAUDE.md and .claude/.
The README answers "how do I run this" without the Gradle files. pass The README has a command table for each loader, the test commands, the GameTest names per loader, the EULA step and the branch to check out. Each command exists in ./gradlew tasks --all.
./gradlew :common:test runs every moved test and they pass. It is the only unit test runner. docs/performance-baseline.md exists. pass CI log: Unit tests: 390 run, 390 passed, 0 failed, 0 skipped. tools/junit-standin/ is deleted. The baseline page exists, with Mojang's two Java tiers as the target PCs.
No build file declares toml4j. pass grep -rn toml4j --include=*.gradle outside .claude/ finds 0 lines.
The GameTests compile from common and run on each loader through its holder. pass Headless Forge and Fabric GameTest servers ran all 16 tests through the holders. No template was missing. Machine tests pass 3 of 3 on both. Herd results: see the findings below.

Runs performed

  • Forge client (headed, lead, 2026-10-25, head a7da2f7): a new creative world loads, and the mod's creative tab is present. /test run sequencercorereceivesenergy reports passed! (2046ms). The game saves and stops cleanly. The startup log has the four asset errors that earlier playtests showed (GeckoLib cannot parse the swim and getup animations, and the carnivore_feeder_one item model fails to load). This pull request changes no assets.
  • Fabric client: reaches the title screen. Entering a world crashes, because Fabric has no client entry point yet (known, M0.P2).
  • Dedicated server: ./gradlew :forge:runServer starts and stops at the EULA, as vanilla does. The README says how to accept it.
  • GameTest servers (headless, pass/fail record only):
    • Forge: 16 tests ran. 8 required tests failed, all herd tests. Machine 3 of 3 pass. The count varies from run to run (9 in the run before).
    • Fabric: 16 tests ran. 13 required tests failed, all herd tests (finding F1). Machine 3 of 3 pass.

Findings

  • F1. Fabric registers no data reload listener. Forge registers four in CommonForgeEvents (gene, behaviour, diet, fossil). All 13 herd GameTests fail on Fabric with a null behaviour entry. M0.P2 owns the fix (reload listeners in common through a platform service).
  • stackedmembersseparate passes in some Forge runs and fails in others. Its body and template did not change.
  • The Fabric herd templates are proven only by inference. Each of the 13 bodies ran up to the F1 failure, and vanilla fails a test before its body when the template is missing.
  • The build prints about 400 doclint warnings that were there before this pull request. setup-java@v5 turns them into 10 GitHub annotations per run. A backlog item could clear them.
  • The test runtime classpath holds duplicate jars with mixed mappings. No test touches them today. A later test that does fails loudly.
  • CI saves the Gradle cache only on pushes to integration/1.20.1. The first push after this merge saves it. Later runs restore it, because integration/1.20.1 is now the default branch. Until then every run is cold (8 to 16 minutes).

Decisions

Nothing in this section blocks the merge.

Your answers in this review (2026-10-25)

  • D284. Target PCs. The speed targets are Mojang's minimum and recommended Java PCs of 2026-07-21, never your own PC. The 120 fps gates are now 30 fps on the minimum PC (Fast) and 60 fps on the recommended PC (Fancy). A developer checks a gate on their own PC against vanilla. The package that owns a gate states the allowed difference in its task plan.
  • D285. Tick time. Tick time comes from the F3 line Integrated server @ N ms ticks, as the median of three readings. A gate that F3 cannot show gets its method in the plan of the package that owns it.

Choices this pull request made

These are made. Change one only if it is wrong.

  • Metadata. mod_author=Dumb-Code, a one-line description, the credits list, the GitHub URLs and pack_format 15. No logo exists, so the logo lines are gone.
  • Dependencies. Forge and Fabric declare what the 1.20.1 build ships, with Botarium, not Balm. Declaring Balm on 1.20.1 would stop the mod from loading. Balm replaces Botarium in M0.P2.
  • GameTest holders. Each loader has one holder with one line per test. package-info.java gives the rule for adding a test.
  • CI scope. CI builds and runs the unit tests. It does not run the GameTest server. M0.P2 decides that.
  • Local files. graphify-out/ is ignored. CLAUDE.md and .claude/ at the root stay yours, not ignored and not committed.
  • Unit tests with Minecraft classes. HerdSpacingTest uses Vec3 and QualityTest loads ChatFormatting. Neither needs a running game, so both stay in :common:test, although the backlog calls the set "pure Java".

Notes

  • The plan changed two backlog lines that named the deleted stand-in runner. The In-scope line "Remove tools/junit-standin/run.sh …" now says "Run all of them with ./gradlew :common:test". The acceptance line "run.sh is gone" now says "It is the only unit test runner".
  • The default branch is integration/1.20.1 since 2026-10-25. New clones and new pull requests start there.
  • For the M0.P2 planner: backlog lines 134 and 439 still name "the five distmarker imports", and none remain. Line 444 lists the Fabric dependency declarations as missing, and this pull request adds them. minecraft_version_range is [1.20.1,1.22) on Forge, while Fabric pins 1.20.1 exactly.
  • Two wording notes from the last review: README line 33 tells contributors not to fix the known GameTest failures, and the baseline page says Load B is profiled on a dedicated server, which backlog line 5846 does not say.

Definition of done

  • Every In scope item is implemented, or the PR names the cut item and the reason. (Balm is not declared, see Dependencies.)
  • Every Acceptance item passes. The PR lists each one with "pass" or "fail" and the evidence.
  • It compiles and runs on Fabric and NeoForge. The feature was exercised in both clients. (1.20.1 has Forge, not NeoForge. Both loaders compile in CI. The Forge client enters a world. The Fabric client crashes entering a world until M0.P2.)
  • Unit tests for its pure logic pass in ./gradlew :common:test.
  • No debug output, no TODO without an issue number, no dead code added.
  • Config keys are in docs/config-reference.md with defaults and the section 11 reference.
  • en_us.json has every new key. Datagen ran again if a provider changed. (No new key, no provider change.)
  • Every Art list row in its Art line is "Done" (no Placeholder, no To check). (No art.)
  • Every row in its "Recipe rows" line ships with the ingredients of the recipe table. (No recipes.)
  • For a machine: the PR shows each screen and animation next to the video frame at the cited time. (No machine change.)
  • The PR links this plan and the work package id. It names the open decisions that it worked around.

🤖 Generated with Claude Code

kashgummaraju and others added 21 commits October 24, 2026 21:08
…ader, Fabric holder constructor, config defaults from code

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… rules for logs, data runs and caches

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…pi stub it needed

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…and-in runner

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t of the source tree

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…uctures move to common resources

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…plate with the definition of done

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e reference machine

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… D285 for the dedicated-server method

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…test; D285 asks for their method

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…request

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… is found

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…sing jar

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…est times and known failures

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…es F3 cannot test; D285 names them

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ames, comment fixes

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Graphify reviewed this change.

Code graph out of date: Health delta baseline: last indexed commit 67bf1b5 (diverged from this PR's base — delta is approximate). Impact and hotspot numbers below may be wrong.

No verdict: the code graph is out of date. Static checks found no blocking issues, but they compared against an outdated graph. The next push after the repository re-indexes gets a full review.

Not checked: tests were not run; no formal proof of the changed code.


Graphify review — findings

Adds a contributor on-ramp for the Beta: a PR template carrying the definition-of-done checklist, a CI workflow that builds both loaders, runs :common:test and uploads the Forge and Fabric jars (the Gradle cache is saved only from integration/1.20.1), and a README rewritten around build, run, test and contribute steps. The README's GameTest section has contributors run one test with /test run <name> using the lower-cased method name, and it lists the known Forge and Fabric herd-test failures so nobody chases them. Also lands the engineering backlog and codebase-analysis docs, broadens the .gitignore patterns, and drops the unused mergetool-api and toml4j dependencies from common.

Review partial — this diff was larger than one review pass covers, so later files were not reviewed; some findings may be missing.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 991 functions depend on the 939 functions this change touches.

Health — this change adds coupling hotspots:

  • new: write() — 4 callers, 5 callees
  • new: natural_forest() — 1 callers, 6 callees

Verification — 991 functions in the blast radius were not formally verified this run (proofs are advisory here).

Health delta baseline: last indexed commit 67bf1b5 (diverged from this PR's base — delta is approximate).

Gate & verification

graphify gate

PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.

Advisory (not blocking):

  • verification_scope: 991 function(s) in the blast radius were not formally verified this run

· 2 more finding(s) on lines outside this diff (see the check run).

kashgummaraju and others added 4 commits October 25, 2026 11:32
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ce via pull_request

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… against vanilla; tick time from F3 (lead answers D284 and D285)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…efault branch

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kashgummaraju
kashgummaraju merged commit 0838bd4 into integration/1.20.1 Oct 25, 2026
2 checks passed
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