Skip to content

[codex] Bound LogSage endpoint outer retries - #367

Open
namitdhameja wants to merge 1 commit into
mainfrom
codex/nvbug6368845-attribution-bounds
Open

[codex] Bound LogSage endpoint outer retries#367
namitdhameja wants to merge 1 commit into
mainfrom
codex/nvbug6368845-attribution-bounds

Conversation

@namitdhameja

@namitdhameja namitdhameja commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR changes the NVRx outer retry behavior for LogSage final attribution endpoint failures.

  • Defaults NVRx endpoint-result outer retries in nvrx_logsage.py to 0 additional retries, with a 60 second backoff if a deployment opts in.
  • Keeps the existing retry budget for real exceptions around the LogSage call.
  • Treats a normal LogSage result containing LLM ENDPOINT FAILED as having already consumed LogSage's internal LLM retry budget.
  • Plumbs optional endpoint retry overrides through attrsvc/controller/orchestration and MCP call arguments instead of reading the new knobs directly inside nvrx_logsage.py.
  • Keeps higher layers at None by default, so unset values are omitted until NVRxLogAnalyzer applies its runtime defaults.
  • Adds service env support for NVRX_ATTRSVC_LOG_ANALYSIS_ENDPOINT_OUTER_RETRIES and NVRX_ATTRSVC_LOG_ANALYSIS_ENDPOINT_OUTER_BACKOFF_SEC; attrsvc also accepts the shorter NVRX_LOG_ANALYSIS_ENDPOINT_OUTER_* aliases.
  • Adds launcher-managed attrsvc CLI knobs:
    • --ft-attribution-log-analysis-endpoint-outer-retries
    • --ft-attribution-log-analysis-endpoint-outer-backoff-sec

Root Cause

For NVBug 6368845, one final attribution attempt was already taking about 15-16 minutes because LogSage and the lower-level LLM client both retried endpoint timeouts. NVRx then retried the same endpoint-failed attribution result three times, producing roughly 48 minutes of wall-clock time before returning LLM FAILURE / UNKNOWN.

Impact

By default, NVRx no longer immediately repeats the full final attribution path after LogSage returns LLM ENDPOINT FAILED. Deployments that want an additional delayed retry can opt in through attrsvc env or launcher CLI config.

Prompt/evidence compaction is intentionally not handled in NVRx; that belongs inside LogSage.

Validation

  • PYTHONPATH=src .venv/bin/python -m pytest tests/attribution/unit/test_nvrx_logsage_retry.py tests/attribution/unit/test_nvrx_logsage_reason_code.py tests/attribution/unit/test_attrsvc_settings.py tests/attribution/unit/test_log_analyzer_observability.py tests/attribution/unit/test_progressive_plumbing.py -q
  • PYTHONPATH=src .venv/bin/python -m py_compile src/nvidia_resiliency_ext/attribution/orchestration/config.py src/nvidia_resiliency_ext/attribution/controller.py src/nvidia_resiliency_ext/services/attrsvc/config.py src/nvidia_resiliency_ext/services/attrsvc/service.py src/nvidia_resiliency_ext/attribution/orchestration/types.py src/nvidia_resiliency_ext/attribution/orchestration/log_analyzer.py src/nvidia_resiliency_ext/attribution/combined_log_fr/combined_log_fr_mcp.py src/nvidia_resiliency_ext/attribution/mcp_integration/module_definitions.py src/nvidia_resiliency_ext/attribution/log_analyzer/nvrx_logsage.py src/nvidia_resiliency_ext/fault_tolerance/attribution_manager.py src/nvidia_resiliency_ext/fault_tolerance/cli_args.py
  • PYTHONPATH=src .venv/bin/python -m black --check .
  • git diff --check

@namitdhameja
namitdhameja force-pushed the codex/nvbug6368845-attribution-bounds branch from 6ce18cf to 7dea388 Compare July 1, 2026 23:36
@namitdhameja namitdhameja self-assigned this Jul 1, 2026
@namitdhameja
namitdhameja force-pushed the codex/nvbug6368845-attribution-bounds branch 4 times, most recently from fc53c92 to 9a8443c Compare July 2, 2026 00:20
@namitdhameja
namitdhameja marked this pull request as ready for review July 2, 2026 14:51
@namitdhameja
namitdhameja requested a review from helisha91 July 2, 2026 14:54
@greptile-apps

greptile-apps Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR bounds the NVRx outer retry behavior for LogSage endpoint failures, fixing a root cause where three full final attribution retries on endpoint-failed results produced ~48 minutes of wall-clock delay. It introduces two new knobs (endpoint_outer_retries, default 0; endpoint_outer_backoff_sec, default 60) and plumbs them consistently from the launcher CLI through attribution_manager → attrsvc env/config/service → orchestration → NVRxLogAnalyzer.

  • The _with_exponential_backoff function is refactored from a for loop to a while loop to allow endpoint failures to be retried with a separate budget without consuming the existing exception-based LLM retry budget.
  • New env-var aliases (NVRX_ATTRSVC_LOG_ANALYSIS_ENDPOINT_OUTER_* / NVRX_LOG_ANALYSIS_ENDPOINT_OUTER_*), launcher CLI flags, and pydantic AliasChoices support are added with corresponding validators at every layer.
  • Tests cover the default no-retry path, opt-in retry path, and verify that exception-based retries remain independent of endpoint failures.

Confidence Score: 5/5

Safe to merge; the default of 0 additional endpoint retries preserves backward-compatible behavior, and the new opt-in path is gated behind explicit config.

The retry separation is logically sound: endpoint failures exit immediately by default and never consume the existing exception-based LLM retry budget. Validation is applied at every layer (pydantic field validators, AttributionConfig.post_init, and _with_exponential_backoff's max() clamp). Plumbing is consistent through all six layers and covered by targeted unit tests. The only structural note (unreachable implicit None return after the while loop) is already captured in a prior review thread and does not affect runtime behavior.

src/nvidia_resiliency_ext/attribution/log_analyzer/nvrx_logsage.py — the refactored while loop is the most complex piece and the site of the previously noted dead-code tail.

Important Files Changed

Filename Overview
src/nvidia_resiliency_ext/attribution/log_analyzer/nvrx_logsage.py Core retry refactor: adds _int_config/_float_config/_endpoint_outer_retry_config helpers, rewrites _with_exponential_backoff from a for-loop to a while-loop that separates endpoint-failure budget from exception-based LLM retries. Function now has an unreachable implicit None return after the while loop (noted in prior review).
src/nvidia_resiliency_ext/attribution/orchestration/config.py Adds endpoint_outer_retries/endpoint_outer_backoff_sec fields to LogSageExecutionConfig and a new endpoint_retry_overrides() helper that clamps negatives and omits None values; clean and consistent with llm_runtime_overrides.
src/nvidia_resiliency_ext/fault_tolerance/attribution_manager.py Adds two new fields to AttributionConfig with non-negative validation in post_init, wires from args via getattr, and exports via NVRX_ATTRSVC_* env vars through _set_if_not_none (which correctly stringifies int/float).
src/nvidia_resiliency_ext/services/attrsvc/config.py Adds two new Settings fields using AliasChoices for both the NVRX_ATTRSVC_ and shorter NVRX_ env-var aliases, with field validators that reject negative values. Pattern matches existing LLM settings.
src/nvidia_resiliency_ext/attribution/controller.py Adds endpoint_outer_retries/endpoint_outer_backoff_sec to AttributionAnalysisConfig, threads them into the Analyzer constructor, and extends the init log message to include both new values.
src/nvidia_resiliency_ext/attribution/orchestration/log_analyzer.py Spreads endpoint_retry_overrides() into the kwargs dict for all three LogSageRunner fetch paths; clean and consistent.
src/nvidia_resiliency_ext/fault_tolerance/cli_args.py Adds two new launcher CLI arguments with both hyphen and underscore forms, None defaults (deferred to attrsvc), and correct dest names matching AttributionConfig.from_args getattr keys.
tests/attribution/unit/test_nvrx_logsage_retry.py Adds three targeted tests: default no-retry on endpoint failure, configured outer retry succeeding on second call, and exception path unaffected by endpoint retry changes.
tests/attribution/unit/test_attrsvc_settings.py Five new tests covering defaults, both env-var aliases, negative-value rejection, and correct plumbing from Settings into controller config.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[launcher CLI
--ft-attribution-log-analysis-endpoint-outer-retries
--ft-attribution-log-analysis-endpoint-outer-backoff-sec] --> B[AttributionConfig.from_args]
    B --> C[AttributionManager._child_env
NVRX_ATTRSVC_LOG_ANALYSIS_ENDPOINT_OUTER_RETRIES
NVRX_ATTRSVC_LOG_ANALYSIS_ENDPOINT_OUTER_BACKOFF_SEC]
    C --> D[attrsvc Settings
AliasChoices: NVRX_ATTRSVC_* / NVRX_*]
    D --> E[_controller_config_from_settings
AttributionAnalysisConfig]
    E --> F[AttributionController
LogAnalyzerConfig / LogSageExecutionConfig]
    F --> G[LogSageRunner.endpoint_retry_overrides
added to run kwargs]
    G --> H{NVRxLogAnalyzer
llm_analyze}
    H --> I[_endpoint_outer_retry_config
reads from effective config]
    I --> J[_with_exponential_backoff
endpoint_outer_retries / endpoint_outer_backoff_sec]
    J --> K{llm_call result}
    K -->|LLM ENDPOINT FAILED
budget exhausted| L[return _endpoint_failed_tuple
ATTR_LLM_FAILURE]
    K -->|LLM ENDPOINT FAILED
budget remaining| M[sleep endpoint_outer_backoff
continue — attempt NOT incremented]
    M --> K
    K -->|valid result| N[return result]
    K -->|exception or empty
attempt == retries| O[return fallback
ATTR_LLM_FAILURE]
    K -->|exception or empty
attempt < retries| P[exponential backoff
attempt += 1]
    P --> K
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[launcher CLI
--ft-attribution-log-analysis-endpoint-outer-retries
--ft-attribution-log-analysis-endpoint-outer-backoff-sec] --> B[AttributionConfig.from_args]
    B --> C[AttributionManager._child_env
NVRX_ATTRSVC_LOG_ANALYSIS_ENDPOINT_OUTER_RETRIES
NVRX_ATTRSVC_LOG_ANALYSIS_ENDPOINT_OUTER_BACKOFF_SEC]
    C --> D[attrsvc Settings
AliasChoices: NVRX_ATTRSVC_* / NVRX_*]
    D --> E[_controller_config_from_settings
AttributionAnalysisConfig]
    E --> F[AttributionController
LogAnalyzerConfig / LogSageExecutionConfig]
    F --> G[LogSageRunner.endpoint_retry_overrides
added to run kwargs]
    G --> H{NVRxLogAnalyzer
llm_analyze}
    H --> I[_endpoint_outer_retry_config
reads from effective config]
    I --> J[_with_exponential_backoff
endpoint_outer_retries / endpoint_outer_backoff_sec]
    J --> K{llm_call result}
    K -->|LLM ENDPOINT FAILED
budget exhausted| L[return _endpoint_failed_tuple
ATTR_LLM_FAILURE]
    K -->|LLM ENDPOINT FAILED
budget remaining| M[sleep endpoint_outer_backoff
continue — attempt NOT incremented]
    M --> K
    K -->|valid result| N[return result]
    K -->|exception or empty
attempt == retries| O[return fallback
ATTR_LLM_FAILURE]
    K -->|exception or empty
attempt < retries| P[exponential backoff
attempt += 1]
    P --> K
Loading

Reviews (2): Last reviewed commit: "Bound LogSage endpoint outer retries" | Re-trigger Greptile

Comment thread src/nvidia_resiliency_ext/attribution/log_analyzer/nvrx_logsage.py Outdated
@namitdhameja
namitdhameja force-pushed the codex/nvbug6368845-attribution-bounds branch from 9a8443c to 0e41fd8 Compare July 2, 2026 15:09
@namitdhameja
namitdhameja force-pushed the codex/nvbug6368845-attribution-bounds branch from 0e41fd8 to 06cfca3 Compare July 2, 2026 15:09
@namitdhameja namitdhameja added the ci-approved Approved to run CI label Jul 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-approved Approved to run CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant