don
/
neon-sprawl
mirror of https://github.com/ViPro-Technologies/neon-sprawl
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity
neon-sprawl/docs/reviews/2026-05-10-NEO-40.md

3.1 KiB
Raw Blame History

Code review — NEO-40 (skill progression telemetry hook sites)

Date: 2026-05-10

Scope: Branch NEO-40-skill-xp-level-up-telemetry-hook-sites vs origin/main (commits through effd234).

Base: origin/main

Verdict

Approve

Summary

The branch adds comment-only telemetry hook markers on the authoritative POST …/skill-progression path after a successful TryApplyXpDelta, plus a second marker on the level-up branch. Behavior and JSON contracts are unchanged; documentation (implementation plan, manual QA, E2.M2 module note, Bruno doc string) matches the comments-only kickoff decision. Server tests pass. One documentation-tracking row should be refreshed when this merges so the implementation alignment table stays consistent with E2_M2_XpAwardAndLevelEngine.md.

Documentation checked

Path Result
docs/plans/NEO-40-implementation-plan.md Matches — hook sites after successful apply + on nextLevel > prevLevel; comments-only; no ILogger; time-to-first-level-up called out; single authoritative layer (HTTP handler).
docs/decomposition/modules/E2_M2_XpAwardAndLevelEngine.md Matches — NEO-40 landed paragraph + backlog narrowed to NEO-41–NEO-43.
docs/decomposition/modules/module_dependency_register.md N/A for diff content (no table edit); E2.M2 row unchanged, still appropriate.
docs/decomposition/modules/documentation_and_implementation_alignment.md Partially matches — E2.M2 narrative still lists “Follow-on: NEO-40 (telemetry hook sites)” while the module doc asks to keep alignment tracking in sync with NEO-40 landed; see Suggestions.
docs/decomposition/modules/contracts.md / client_server_authority.md / data_and_ops_policy.md N/A — no contract or authority change; telemetry ingest remains E9.M1 backlog, consistent with comments.
docs/manual-qa/NEO-40.md Matches — API parity + comment spot-check aligns with plan.

Register / tracking table: After merge, update the E2.M2 cell in documentation_and_implementation_alignment.md to a “NEO-40 landed: …” bullet (and trim the old “Follow-on: NEO-40” line) so it matches E2_M2_XpAwardAndLevelEngine.md.

Blocking issues

(none)

Suggestions

  1. documentation_and_implementation_alignment.md (E2.M2 row): Replace the remaining “Follow-on: NEO-40” wording with a landed summary (hook sites in SkillProgressionSnapshotApi, link to NEO-40 plan + manual QA), mirroring the style used for NEO-37–NEO-39 in the same cell. This satisfies the module doc’s instruction to keep that table in sync.

Nits

  • Nit: The implementation plan’s technical approach for level_up mentions “deltas” in the payload sketch; the in-code comment lists playerId, skillId, previousLevel, newLevel but not XP/total deltas. Optional one-word addition (prevXp/newXp or total XP) if you want comment parity with the plan.

Verification

  • dotnet test NeonSprawl.sln — Passed (133 tests) on review machine.
  • Optional: follow docs/manual-qa/NEO-40.md curl steps against a running server.