AGENTS.md: proposed units policy

[veprbl]

Briefly, what does this PR introduce? Please link to any relevant presentations or discussions.

What is the urgency of this PR?

• High (please describe reason below)
• Medium
• Low

What kind of change does this PR introduce?

• Bug fix (issue #__)
• New feature (issue #__)
• Optimization (issue #__)
• Updated parameters, constants (issue #__)
• Updated documentation
• other: __

Please check if any of the following apply

• This PR requires changes to geometry (epic PR: __)
• This PR requires changes to EDM4eic (EDM PR: __)
• This PR introduces breaking changes. Please describe changes users need to make below.
• This PR changes default behavior. Please describe changes below.
• AI was used in preparing this PR. Please describe usage below.

[Copilot]

🟡 Changes recommended

The new policy text includes a specific code reference (PrimaryVertices.cc snippet) that does not match the actual code, which could mislead readers trying to verify the guidance.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds a proposed “Unit System Policy” section to AGENTS.md to document how contributors should handle conversions between edm4eic::unit, dd4hep, and Acts::UnitConstants and what anti-patterns to flag in review.

Changes:

• Documented a three-unit-system policy and explicit conversion rules for EDM fields, geometry-derived values, and Acts interfaces.
• Added a short list of anti-pattern examples to help reviewers detect incorrect unit usage.

File summaries

File	Description
AGENTS.md	Adds unit-system rules and conversion guidance intended to prevent subtle unit-mismatch bugs.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[github-actions]

Capybara summary for PR 2939

• rec_dis_10x100_minQ2=0_craterlake_10x100_MT (Δ: 398)
• rec_dis_10x100_minQ2=100_craterlake_tracking_only (Δ: 0)
• rec_dis_10x130_minQ2=1_craterlake_10x130 (Δ: 0)
• rec_dis_10x250_minQ2=1_craterlake_10x250 (Δ: 0)
• rec_dis_18x275_minQ2=0_craterlake_18x275 (Δ: 4517)
• rec_dis_18x275_minQ2=0_craterlake_18x275_MT (Δ: 374)
• rec_dis_18x275_minQ2=0_craterlake_18x275_ST_MT (Δ: 369)
• rec_dis_18x275_minQ2=0_craterlake_18x275_Seeding_Seeding2 (Δ: 959)
• rec_dis_18x275_minQ2=100_craterlake_18x275 (Δ: 0)
• rec_dis_18x275_minQ2=100_craterlake_18x275_MT (Δ: 822)
• rec_dis_18x275_minQ2=100_craterlake_18x275_ST_MT (Δ: 602)
• rec_dis_18x275_minQ2=100_craterlake_18x275_Seeding_Seeding2 (Δ: 1705)
• rec_dis_5x41_minQ2=0_craterlake_5x41 (Δ: 0)
• rec_e_1GeV_20GeV_craterlake (Δ: 0)
• rec_pi_1GeV_20GeV_craterlake (Δ: 0)
• two_stage_raw_e_1GeV_20GeV_craterlake (Δ: 0)
• two_stage_rec_e_1GeV_20GeV_craterlake (Δ: 0)

Last updated 2026-09-03T16:04-04:00 ce6c8a3

[Copilot]

🟢 Approval recommended

The change is documentation-only and the added policy aligns with existing in-repo usage patterns, with only a minor clarification suggested.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

The new documentation includes an inaccurate concrete reference and one ambiguous phrase that should be corrected to avoid misleading future reviewers/contributors.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

AGENTS.md:195

• The phrase "Acts EDM" is unclear here (Acts isn't the project's event data model); this reads like a specific concept rather than "values passed to/from Acts". Clarifying the wording would make the policy easier to follow.

AGENTS.md:206

• The reference to PrimaryVertices.cc is inaccurate: the code uses ... / edm4eic::unit::mm on transverse radius and z (not v.x / edm4eic::unit::mm). This may mislead reviewers trying to find a concrete example.

These should use `edm4eic::unit::mm` / `edm4eic::unit::GeV` / `edm4eic::unit::MeV` instead. Correct reference: `PrimaryVertices.cc` (`v.x / edm4eic::unit::mm`) and `SimCalorimeterHitProcessor.cc` (`constant<double>(...) * edm4eic::unit::mm / dd4hep::mm`).

• Files reviewed: 1/1 changed files
• Comments generated: 0 new
• Review effort level: Lite

[DraTeots]

Tested on an agent. Works.