fix(cost): charge NAV_BUILD past the nav-cache short-circuit

[wezell]

Proposed Changes

Follow-up to #36978, addressing a review finding that landed after merge: @RequestCost(Price.NAV_BUILD) on NavTool.getNav fired on method entry, so a live-mode navigation served entirely from NavToolCache still paid the full NAV_BUILD(10) — the common case on live traffic.

This moves the charge to a direct RequestCostHandler.incrementCost call on the cache-miss/build path, matching the cache-aware placement deliberately used in DotDirective.render and VelocityLiveMode.writePage:

• Warm nav (live-mode cache hit): costs 0 — caching visibly pays off, consistent with cached containers/pages.
• Cache miss / edit / preview: flat NAV_BUILD(10) — covers the folder/menu-item walk (findMenuItems, findSubFolders, IdentifierAPI.find), which is not metered anywhere else.
• Content-backed menu items still stack their own CONTENT_FROM_CACHE/CONTENT_FROM_DB charges through the content factory — that stacking is intended and documented in the code comment.

Related to #36977.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VTt9ymEwfmHV9DSKe1vPuJ

This PR fixes: #36977

This PR fixes: #36977

[claude]

Claude finished @sfreudenthaler's task in 1m 18s —— View job

---

PR Review: charge NAV_BUILD past the nav-cache short-circuit

• Read the diff and both changed files
• Verify charge placement, control flow, and import cleanup
• Post review

I reviewed RequestCostHandler.java and NavTool.java against the base of #36978.

New Issues

No issues found.

The change is correct and consistent with the stated intent:

• Charge placement (NavTool.java:127) sits after the live-mode cache short-circuit (NavTool.java:114-119), so a warm live-mode nav returns at line 117 and pays 0 — matching the DotDirective.render / VelocityLiveMode.writePage idiom now documented in RequestCostHandler's Javadoc.
• Failure isolation is preserved: the direct call uses the incrementCost(Price, Class, String, Object[]) overload, which wraps the API call in try/catch (Throwable) and only logs — metering cannot take down nav rendering. Good choice over an inline APILocator.getRequestCostAPI() call.
• Import cleanup is complete: @RequestCost had no other use in NavTool (only occurrence was the removed annotation), so swapping the import to RequestCostHandler leaves no dangling reference.
• No new double-charge: the annotation previously fired once per getNav(host, path, languageId, user) entry; the direct call also fires once per build-path execution. The only behavioral deltas are the two intended ones — cache hits and the null-folder early return (NavTool.java:103-105) no longer charge. Not charging when no build work happens is the correct outcome.
• Content-item stacking (CONTENT_FROM_CACHE/CONTENT_FROM_DB via the content factory) is unaffected and, as the comment notes, intentional.

Minor, non-blocking observations (not defects):

• The method name is hardcoded as "getNav" in the args to incrementCost — this matches the convention at the sibling direct-charge sites and is only used for the HTML accounting report, so it's fine.
• The Javadoc's "if a fourth site shows up, promote this to a real helper" note is a good guardrail against the copy-paste comment proliferating.

· issue-36977-navtool-cache-pricing

[sfreudenthaler]

Added a pointer for the cache-short-circuit charge idiom on RequestCostHandler's class javadoc — it's the one file all three direct-charge sites (NavTool.getNav, DotDirective.render, VelocityLiveMode.writePage) already call through, so it's where both a human and a future Claude session land before adding a fourth. 9e9a35a

[sfreudenthaler]

good catch on review. Added a note as well so hopefully humans and bots find it before a reviewer needs to point it out in the future