Skip to content

test(probes): record p7 hero-settings client runs - #140

Merged
e54-bot merged 2 commits into
mainfrom
probe/p7-hero-settings-capture
Nov 3, 2026
Merged

e54-bot merged 2 commits into
mainfrom
probe/p7-hero-settings-capture

Conversation

@e54-bot

@e54-bot e54-bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Refs #11 (Capability 3: room settings and hero state). Partial: this only records the P7 client runs; simulator modeling stays out of scope.

Summary

Records three client runs of probes/p7-hero-settings (client 2.25.0.1 - 154504.1 for the recorded version, locale zh-CN, 2026-11-03). capture.json was produced with athena ingest; client, observed and notes were filled by hand as the convention requires. Result: match, 3/3 runs agree.

Observed vs hypothesis

line expected observed
max_health 375 375 (health% 150 on Ana, pool 250)
health_after_damage 275 275 (scripted Damage 200 landed as 100)
health_after_heal 355 355 (scripted Heal 40 landed as 80)
ability1_cooldown 20 20 (Set Ability Cooldown wrote literally; abilityCooldown% 50 did not scale it)

Caveats

  • Only the version of the latest run was recorded; the versions of runs 1 and 2 were not. The convention does not require a per-run version.
  • The sources names are the original Inspector log names; the three capture*.txt files hold the probe lines saved by ingest.
  • The probe covers script-applied writes, not natural ability cooldowns.

Verification

  • probe_sha256 in capture.json equals shasum -a 256 probes/p7-hero-settings/probe.zh-CN.ow (5aa48b5a...) and equals the value on origin/main.
  • cargo run -p athena-capture -- ingest printed run 1: match, run 2: match, run 3: match, result: match.
  • cargo test: exit 0, all suites passed (includes the probe_sha256 check).
  • cargo clippy --all-targets -- -D warnings: clean.

Three client runs on 2.25.0.1 - 154504.1 (zh-CN) all matched expect.json: Max Health 375, damage 200 landed as 100, heal 40 landed as 80, and Set Ability Cooldown wrote 20 literally.

Refs #11
@e54-bot

e54-bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

Verdict: CHANGES REQUESTED

The capture data is sound. Two same-PR follow-ups are missing.

Verified

  • capture.txt, capture-2.txt, capture-3.txt are byte-identical (cmp) to the owner raw files in the shared checkout. Each holds only five [hh:mm:ss] P7 ... lines (four readings plus P7 done).
  • I re-ran athena ingest on a copy of the probe with runs reset to 0, using the three file names in sources. Result: run 1 match, 2 match, 3 match, final result: match. It saved the same three captures byte-for-byte and wrote the same runs: 3 and sources.
  • Observed values match expect.json line by line (max_health=375, health_after_damage=275, health_after_heal=355, ability1_cooldown=20). observed has one entry for runs [1,2,3] with those values.
  • probe_sha256 5aa48b5a... equals shasum -a 256 probe.zh-CN.ow on the head.
  • Field names and units: ability1_cooldown_s: 20 is right. The probe prints seconds (P7 ability1_cooldown=20; procedure.zh-CN.md:93), and the convention requires milliseconds only for timing values. The _s suffix states the unit; the old _ms: 20000 would have been wrong. notes names the provisional-free state and the unrecorded run 1 and 2 client versions.
  • cargo test passes, cargo clippy --all-targets -- -D warnings is clean, cargo fmt --check is clean. cargo test -p athena-sim --test fixtures -- --ignored passes p7_hero_settings.

Findings

  1. Stale #[ignore]. crates/athena-sim/tests/fixtures.rs:247 says #[ignore = "no captures yet (runs is 0)"]. runs is now 3. With the ignore removed the test passes, so the simulator already agrees with the capture. Remove the attribute in this PR, otherwise the new capture is not asserted in CI. Also fix the doc comment at fixtures.rs:252 ("P7 is inconclusive: it has no captures"). The doc must say that p7_hero_settings asserts the capture and p7_hero_settings_models_the_expected_lines checks the model.
  2. Stale doc claim. docs/architecture.md:69 says whether abilityCooldown% also scales a scripted set "is what probe P7 is for". P7 answered this: the write is literal (20 stays 20). AGENTS.md requires the owning doc to change in the same change. Replace it with the measured fact. The same bullet group opens at docs/architecture.md:67 with "no client capture backs the player state table or the Damage/Heal rules". That is now wrong for health%, damageReceived%, healingReceived% and the literal Set Ability Cooldown. Narrow it to what is still unmeasured. The comment at crates/athena-sim/src/actions.rs:190 ("is what P7 is for") is stale the same way.
  3. Non-blocking. Issue Capability 3: room settings and hero state #11 requires hero values at a named balance version. capture.json records the client version only. The 375 reading depends on Ana health 250. Name the balance version in notes if it is known, or say it is unknown.

Command results (head ea82d51): cargo test ok; clippy ok; fmt ok; ingest 3/3 match, same files and metadata as committed.

Remove the stale ignore on the p7 fixture test. State what P7 measured in architecture.md and actions.rs. Name the balance version limit in capture notes.

Refs #11
@e54-bot

e54-bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

Addressed in db4e62e.

  1. Stale #[ignore] -> removed from p7_hero_settings; doc comment at fixtures.rs now says that test asserts the capture and the other checks the model. Both pass.
  2. Stale docs -> docs/architecture.md (lines 67, 69) and the actions.rs comment now state only what P7 measured: health%, damageReceived%, healingReceived% on scripted Damage/Heal, and Set Ability Cooldown written literally under abilityCooldown% 50. Natural ability cooldowns are stated as not measured. No simulator behavior changed.
  3. Balance version -> capture.json notes: workshop-rs balanceVersion is the client version string (CURRENT_VERSION 2.25.0.0 - 154461.1), not a named patch. This capture's client is 2.25.0.1 and workshop-rs has no file for it, so the balance version of the Ana 250 reading is not determinable. Notes say so.

cargo test, clippy -D warnings, fmt --check all pass. Re-review requested.

@e54-bot

e54-bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

Verdict: LGTM (head db4e62e)

All three round-1 findings are resolved.

Verified

  1. #[ignore] is gone from p7_hero_settings. The doc comment now says that test asserts the capture and the other test checks the model. cargo test -p athena-sim --test fixtures p7 runs both and both pass.
  2. docs/architecture.md:67,69 and actions.rs:189 now state only what P7 measured: health%, damageReceived%, healingReceived% on scripted Damage/Heal, and a literal Set Ability Cooldown under abilityCooldown% 50 (Ana). Natural cooldowns are stated as not measured. grep -i p7 docs crates finds no other doc that says P7 has no captures or is inconclusive. procedure.zh-CN.md records no result status, by design.
  3. capture.json notes say the balance version is not determinable. This matches workshop-rs: CURRENT_VERSION = "2.25.0.0 - 154461.1" (hero_values.rs:14) is a client version string, and the capture client is 2.25.0.1.
  • Diff vs origin/main: 7 files. They are the three capture files, capture.json, one test attribute and doc comment, one doc bullet pair and one code comment. No simulator behavior change.

Non-blocking nit: architecture.md:67 says "client 2.25.0.1", but capture.json notes say the versions of runs 1 and 2 were not recorded. Only run 3 is known to be 2.25.0.1.

Commands: cargo test ok (0 failures), cargo clippy --all-targets -- -D warnings clean, cargo fmt --check clean, git diff --check origin/main...HEAD clean.
CI: Quality pass, Core builds for WebAssembly pass, Detect changes pass, Workflow lint skipped.

@e54-bot
e54-bot merged commit b1275ea into main Nov 3, 2026
4 checks passed
@e54-bot
e54-bot deleted the probe/p7-hero-settings-capture branch November 3, 2026 13:01
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