|
| 1 | +<!-- cspell:ignore callees pathofbuildingcommunity pathofbuilding --> |
1 | 2 | <!-- code-review-graph MCP tools --> |
2 | 3 | ## MCP Tools: code-review-graph |
3 | 4 |
|
4 | | -**IMPORTANT: This project has a knowledge graph. ALWAYS use the |
5 | | -code-review-graph MCP tools BEFORE using Grep/Glob/Read to explore |
6 | | -the codebase.** The graph is faster, cheaper (fewer tokens), and gives |
7 | | -you structural context (callers, dependents, test coverage) that file |
| 5 | +This project may have a knowledge-graph MCP server (`code-review-graph`) |
| 6 | +configured. **When its tools are available in the session, prefer them over |
| 7 | +Grep/Glob/Read for exploration** — they are faster, cheaper (fewer tokens), |
| 8 | +and give structural context (callers, dependents, test coverage) that file |
8 | 9 | scanning cannot. |
9 | 10 |
|
10 | | -### When to use graph tools FIRST |
| 11 | +### When to use graph tools first |
11 | 12 |
|
12 | 13 | - **Exploring code**: `semantic_search_nodes` or `query_graph` instead of Grep |
13 | 14 | - **Understanding impact**: `get_impact_radius` instead of manually tracing imports |
14 | 15 | - **Code review**: `detect_changes` + `get_review_context` instead of reading entire files |
15 | 16 | - **Finding relationships**: `query_graph` with callers_of/callees_of/imports_of/tests_for |
16 | 17 | - **Architecture questions**: `get_architecture_overview` + `list_communities` |
| 18 | +- **Planning refactors**: `refactor_tool` for renames and dead code |
17 | 19 |
|
18 | | -Fall back to Grep/Glob/Read **only** when the graph doesn't cover what you need. |
19 | | - |
20 | | -### Key Tools |
21 | | - |
22 | | -| Tool | Use when | |
23 | | -|------|----------| |
24 | | -| `detect_changes` | Reviewing code changes — gives risk-scored analysis | |
25 | | -| `get_review_context` | Need source snippets for review — token-efficient | |
26 | | -| `get_impact_radius` | Understanding blast radius of a change | |
27 | | -| `get_affected_flows` | Finding which execution paths are impacted | |
28 | | -| `query_graph` | Tracing callers, callees, imports, tests, dependencies | |
29 | | -| `semantic_search_nodes` | Finding functions/classes by name or keyword | |
30 | | -| `get_architecture_overview` | Understanding high-level codebase structure | |
31 | | -| `refactor_tool` | Planning renames, finding dead code | |
32 | | - |
33 | | -### Workflow |
34 | | - |
35 | | -1. The graph auto-updates on file changes (via hooks). |
36 | | -2. Use `detect_changes` for code review. |
37 | | -3. Use `get_affected_flows` to understand impact. |
38 | | -4. Use `query_graph` pattern="tests_for" to check coverage. |
| 20 | +If these tools are **not** available in the current session, use the standard |
| 21 | +Grep/Glob/Read tools as normal. |
39 | 22 |
|
40 | 23 | --- |
41 | 24 |
|
42 | 25 | # Path of Building 2 — Agent Instructions |
43 | 26 |
|
44 | 27 | You are working on a fork of PathOfBuildingCommunity/PathOfBuilding-PoE2, an |
45 | | -offline build planner for Path of Exile 2. The codebase is ~100% Lua (5.1 / LuaJIT). |
| 28 | +offline build planner for Path of Exile 2. The codebase is ~100% Lua |
| 29 | +(5.1 / LuaJIT). The project's core value is correctly modelling PoE2's stat |
| 30 | +math; correctness of numeric calculations outranks everything else. |
46 | 31 |
|
47 | 32 | ## Critical rules |
48 | 33 |
|
49 | | -1. **Never break numeric calculations.** PoB's value is in correctly modelling |
50 | | - PoE2's stat math. If you touch anything in `src/Modules/Calcs/` or `src/Modules/CalcSections.lua`, |
51 | | - you MUST add or update tests in `spec/System/` that assert specific stat values |
52 | | - within tolerance (±0.01 for DPS, exact for hit point pools). |
| 34 | +1. **Never break numeric calculations.** If you touch any |
| 35 | + `src/Modules/Calc*.lua` file, you MUST add or update tests in |
| 36 | + `spec/System/` that assert specific stat values within tolerance |
| 37 | + (±0.01 for DPS, exact for hit point pools). |
53 | 38 |
|
54 | | -2. **Modifier parsing is fragile.** `src/Modules/ModParser.lua` is the single most |
55 | | - load-bearing file. Any change requires: |
56 | | - - Adding test cases for the new modifier strings in `spec/System/TestModParser.lua` |
57 | | - - Verifying existing test suite still passes (`docker compose run --rm tests`) |
| 39 | +2. **Modifier parsing is fragile.** `src/Modules/ModParser.lua` is the single |
| 40 | + most load-bearing file. Any change requires: |
| 41 | + - Test cases for the new modifier strings in `spec/System/TestModParser_spec.lua` |
| 42 | + - A passing full suite (`docker compose run --rm busted-tests`) |
58 | 43 |
|
59 | | -3. **Data file changes need validation.** Game data lives in `src/Data/`. After |
60 | | - modifying any data file, run the data validation tests (`docker compose run --rm tests --tags data`). |
| 44 | +3. **Data file changes need validation.** Game data lives in `src/Data/`. |
| 45 | + After modifying any data file, run |
| 46 | + `docker compose run --rm busted-tests --tags data`. |
61 | 47 |
|
62 | | -4. **Lua nil-safety is your responsibility.** Lua does not have a type system. |
63 | | - When you call `someTable.field.subfield`, guard with `someTable and someTable.field and someTable.field.subfield` |
64 | | - or use `rawget`. Nil reference crashes are the #1 source of bug reports in PoB. |
| 48 | +4. **Lua nil-safety is your responsibility.** Lua has no type system. Guard |
| 49 | + chained accesses (`t and t.field and t.field.sub`). Nil reference crashes |
| 50 | + are the #1 source of bug reports in PoB. |
65 | 51 |
|
66 | 52 | 5. **Performance matters in calculation hot loops.** Avoid table allocations |
67 | | - inside `Calc:Build*` functions; reuse tables and use local variable caching |
68 | | - for frequently-accessed table fields. |
| 53 | + inside per-pass calc code; cache frequently-accessed table fields in locals. |
69 | 54 |
|
70 | 55 | ## How to test changes |
71 | 56 |
|
| 57 | +Tests use Busted with LuaJIT inside a pre-built container |
| 58 | +(`ghcr.io/pathofbuildingcommunity/pathofbuilding-tests`). Only Docker (or |
| 59 | +Podman) is needed — no local Lua toolchain. |
| 60 | + |
72 | 61 | ```bash |
73 | | -docker compose run --rm tests # full Busted suite |
74 | | -docker compose run --rm tests --tags builds # build snapshot tests (slow) |
75 | | -docker compose run --rm tests --tags data # data file validation only |
76 | | -docker compose run --rm tests --coverage # with luacov |
| 62 | +docker compose run --rm busted-tests # full suite |
| 63 | +docker compose run --rm busted-tests --exclude-tags builds,data # unit tests only (what CI calls "unit") |
| 64 | +docker compose run --rm busted-tests --tags builds # build snapshot tests (slow) |
| 65 | +docker compose run --rm busted-tests --tags data # data file validation only |
| 66 | +docker compose run --rm busted-tests --filter="pattern" # single test by describe/it name (Lua pattern) |
77 | 67 | ``` |
78 | 68 |
|
| 69 | +Notes: |
| 70 | +- The compose service is named `busted-tests` (not `tests`). |
| 71 | +- The compose volume is mounted read-only, so `--coverage` cannot write |
| 72 | + `luacov.report.out` through compose. CI uses a plain `docker run` that |
| 73 | + bind-mounts only the report file writable (see `.github/workflows/ci.yml`). |
| 74 | +- Busted config is in `.busted`: it runs from `src/` with |
| 75 | + `HeadlessWrapper.lua` as the helper and discovers specs in `spec/`. |
| 76 | +- Baseline on `dev`: **370 successes**, plus two known failures in |
| 77 | + `spec/System/TestWard_spec.lua` (Ward regen/bypass not yet implemented). |
| 78 | + Don't "fix" those by weakening assertions. |
| 79 | + |
79 | 80 | ## How to validate before opening a PR |
80 | 81 |
|
81 | | -1. All tests pass: `docker compose run --rm tests` |
82 | | -2. Coverage did not drop: compare `luacov.report.out` against main |
83 | | -3. No new luac warnings: `luac -p src/**/*.lua` |
84 | | -4. The commit message follows Conventional Commits (`feat:`, `fix:`, `refactor:`, etc.) |
| 82 | +1. Full suite passes: `docker compose run --rm busted-tests` |
| 83 | +2. Coverage did not drop: compare `luacov.report.out` against the base branch |
| 84 | +3. Commit messages follow Conventional Commits (`feat:`, `fix:`, `refactor:`, …) |
85 | 85 |
|
86 | 86 | ## Project structure |
87 | 87 |
|
88 | | -- `src/` — main Lua source. Entrypoint is `src/Launch.lua`. |
89 | | -- `src/Modules/Calcs/` — the calculation engine. The most important code in the project. |
90 | | -- `src/Modules/ModParser.lua` — converts modifier strings into structured stat objects. |
91 | | -- `src/Data/` — game data: gems, items, passive tree, base item types. |
92 | | -- `spec/System/` — Busted test suite. |
93 | | -- `runtime/lua/` — extra Lua libraries used at runtime. |
94 | | -- `tests/` — Docker-based test harness. |
| 88 | +- `src/Launch.lua` — GUI entry point; loads `src/Modules/Main.lua`, which loads everything else. |
| 89 | +- `src/HeadlessWrapper.lua` — headless bootstrap (what the test suite and any scripted use of PoB boots through). |
| 90 | +- `src/Modules/Calc*.lua` — the calculation engine, flat files (there is **no** `Calcs/` subdirectory): `Calcs.lua` (public entry), `CalcSetup.lua`, `CalcPerform.lua`, `CalcOffence.lua`, `CalcDefence.lua`, `CalcActiveSkill.lua`, `CalcTriggers.lua`, `CalcMirages.lua`, plus `CalcSections.lua`/`CalcBreakdown.lua` for the breakdown UI. |
| 91 | +- `src/Modules/ModParser.lua` — converts modifier text into structured mods; `src/Modules/ModTools.lua` has the `mod()`/`flag()` constructors. |
| 92 | +- `src/Classes/` — UI tab classes and the mod storage classes (`ModDB.lua`, `ModList.lua`, `ModStore.lua`). |
| 93 | +- `src/Data/` — game data: gems, item bases, uniques, crafting mods. Largely **generated** by `src/Export/` from GGPK files — prefer fixing the exporter over hand-editing structure. |
| 94 | +- `src/TreeData/` — passive tree data per game version. |
| 95 | +- `spec/System/` — Busted test suite (`*_spec.lua`). |
| 96 | +- `runtime/lua/` — bundled Lua libraries available at runtime. |
| 97 | +- `docs/` — read `rundown.md`, `calcOffence.md`, `modSyntax.md`, `addingMods.md`, `addingSkills.md` before non-trivial changes. |
| 98 | + |
| 99 | +In `CalcOffence.lua`, attacks run **per-hand passes**: inside a pass the local |
| 100 | +`output` is `actor.output.MainHand`/`OffHand` and `globalOutput` is |
| 101 | +`actor.output`; hand results are merged with `combineStat`. Misattributing |
| 102 | +output to the wrong table is the classic bug — read `docs/calcOffence.md` first. |
95 | 103 |
|
96 | 104 | ## Fork rules |
97 | 105 |
|
98 | | -1. **NEVER open PRs against the upstream repo** (`PathOfBuildingCommunity/PathOfBuilding-PoE2`). All PRs must target the fork (`jay9297/PathOfBuilding-PoE2`). Upstream sync is done by merging `origin/dev` locally, not via PRs. |
| 106 | +1. **NEVER open PRs against the upstream repo** (`PathOfBuildingCommunity/PathOfBuilding-PoE2`). All PRs must target the fork (`jay9297/PathOfBuilding-PoE2`), base branch `dev`. |
99 | 107 |
|
100 | 108 | 2. **Default push target is `fork`.** All `git push` and PR operations default to `fork/dev`. The git config enforces this via `remote.pushDefault=fork` and `branch.dev.remote=fork`. |
101 | 109 |
|
102 | 110 | 3. **Upstream sync is via local merge, not PR.** To sync with upstream: `git fetch origin && git merge origin/dev` on the local `dev` branch, then push to fork. |
| 111 | + |
| 112 | +4. This fork runs automated AI workflows (`ai-fix.yml` triggered by the `ai-fix` issue label, `ai-review.yml`, `auto-merge.yml`). See "AI Workflow Escape Hatches" in `README.md` for how to control them. |
0 commit comments