fix(terminus-2): preserve terminal response ID

[ananthsub]

Summary

• Preserve the final model server response ID inside the Terminus 2 LLM adapter.
• Reuse that ID on the outer agent response so terminal attribution can join the verified response to the captured model call.
• Keep the existing generated response ID as a fallback when no model response ID is available.

This is needed when Terminus 2 uses out-of-band token capture. Responses that already carry native token IDs continue to use the existing short-circuit and do not perform terminal attribution.

Test plan

• Run the Terminus 2 agent tests (41 passed).
• Run Ruff checks and formatting validation on all changed files.

[ananthsub]

/ok to test ae77831

[ananthsub]

/claude review

[claude]

SHIP — no reliability concerns.

Verified the correctness-critical invariants in finalize_rollout_token_capture:

• Native token ids are never overwritten — the carries_token_ids branch returns rebuilt_response: None, so response.output is left as the policy sampled it. The new behavior only adds terminal-attribution metrics; it does not touch training tokens.
• Native rollouts are never masked — MASK_SAMPLE_KEY: False is forced even if the reconstruction build itself masked (e.g. incomplete capture). Correct: native ids are authoritative ground truth, and this matches prior behavior (no masking regression that could silently drop good samples).
• Freeze-failure fallback preserved — if source.freeze fails inside trajectories_from_source, the returned build lacks _capture_snapshot; the guard at delivery.py:172-175 warns and returns None, leaving native ids intact. Equivalent to the removed except → return None path.
• Retirement still works — _redundant_capture=True + mask=False + a present _capture_snapshot keeps capture_build_can_retire True, and the snapshot dict guard ensures retire_rollout_token_capture has a valid id/version.
• No stale consumers — nothing outside delivery.py relied on native rollouts lacking _ng_token_capture; rollout_collection.py only reads mask/metrics, and native builds are unmasked.

Net effect is improved observability (native rollouts now carry terminal-attribution metrics) with the 'existing token ids take precedence' invariant intact. Tests assert real behavior and are not vacuous. Both PR revision checks passed.

[ananthsub]

/claude review

[claude]

SHIP — LGTM, no reliability concerns.

Reviewed the token-capture correctness path closely (nemo_gym/token_id_capture/delivery.py) plus the Terminus-2 response-id plumbing. This is a clean refactor that preserves the invariant that matters most for training data integrity, and adds real terminal attribution for native-token rollouts.

What I verified:

• Native token ids are never overwritten or masked on failure. In the base, a rollout carrying generation_token_ids short-circuited before the build. Now it flows through trajectories_from_source first, but trajectories_from_source catches its own freeze/build failures internally and returns a _failed_build dict (never raises, never None) — so the new carries_token_ids branch at delivery.py:169 sees no _capture_snapshot, warns, and returns None, leaving result["response"]["output"] untouched. The policy's sampled ids survive every failure path. The outer except at delivery.py:149 only fires on an unexpected raise that _assemble is designed to prevent.
• Retirement is now more correct than base. The returned build spreads the real _capture_snapshot from the actual assemble (**built), rather than base's separately-fabricated source.freeze call, so capture_build_can_retire/retire_rollout_token_capture operate on the snapshot that was actually read.
• TOKEN_CAPTURE_KEY now present on native rollouts (was absent). No downstream consumer treats its presence as an error signal — the only readers are the delivery module and its tests.
• Response-id coherence (app.py:466, llm.py:138-140): propagating the terminal chatcmpl id into response.id is what makes the response_id witness in resolve_terminal actually match a captured entry; the old random resp_{uuid4} guaranteed response_id_no_match. On context-length-exceeded, the raise precedes the last_response_id assignment, so it correctly reflects the last successful call. getattr(agent, "_llm", None) + isinstance guard degrades gracefully to a UUID.
• Test coverage is solid: new test_finalize_attributes_without_overwriting_native_token_ids asserts attribution runs while native output is preserved, and the native_tokens_preserved flag is checked in both suites.

No async/httpx/ray issues, no public-API breaks, no config-default violations.

[ananthsub]

/claude review

[claude]

SHIP — LGTM, no reliability concerns.

Propagates the terminal chat-completion id from NemoGymLLM into NeMoGymResponse.id, falling back to resp_{uuid4} when absent. Defensive throughout: last_response_id is only assigned on a truthy str (llm.py:139), _run_terminus guards with isinstance(llm, NemoGymLLM) (app.py:413), and responses() uses terminal_response_id or f"resp_{uuid4().hex}" (app.py:466). No impact on scoring/verification, no new async paths, and both the id-present and None branches are covered by the updated tests.

[cmunley1]

can we get the terminal response id from _handle_llm_interaction, since terminus 2 has summarization enabled by default which goes through the same _llm https://github.com/harbor-framework/harbor/blob/9dddd797b57ab8a0f9d6352a20fce73abbb29573/src/harbor/agents/terminus_2/terminus_2.py#L531

instead maybe

async def _handle_llm_interaction(self, *args, **kwargs) -> tuple[Any, ...]:
    result = await super()._handle_llm_interaction(*args, **kwargs)
    llm = getattr(self, "_llm", None)
    if isinstance(llm, NemoGymLLM):
        self.terminal_response_id = llm.last_response_id   
    ...

Then app.py:412-413 reads getattr(agent, "terminal_response_id", None).

[cmunley1]

actually not sure, since subagents and compaction

[cmunley1]

terminus-2 is the first of 13 agents to stop minting resp_{uuid4().hex}. Nothing in-repo asserts a resp_ prefix for it, so no contract break, but if the pattern is right the others want it too and it might belong in a shared helper.