Remove crashing extra_params.append in NeMoGuardrailsServer.__init__

[chuenchen309]

What

NeMoGuardrailsServer.__init__ runs:

if self.extra_params and not self.extra_params.get("extra_body"):
    self.extra_params.append("extra_body")

extra_params is a dict (inherited from OpenAICompatible, default {}, consumed via .items()), so .append raises AttributeError: 'dict' object has no attribute 'append'. Any user who sets a non-empty extra_params without an extra_body key crashes generator construction:

# generators.guardrails.NeMoGuardrailsServer.extra_params = {"foo": 1}
NeMoGuardrailsServer(config_root=...)  # AttributeError during __init__

Why remove rather than fix the .append

• The block only ever crashes (condition true) or is skipped (condition false) — it has never completed successfully, so removing it changes no working behaviour.
• Guardrails config is actually delivered via the self.extra_body attribute set just below (and passed to the client by the base class), not via an extra_params["extra_body"] key.
• A naive fix of self.extra_params["extra_body"] = {} would be worse: OpenAICompatible._call_model spreads extra_params into the request after applying self.extra_body, so it would overwrite the real guardrails config with an empty dict.

Test

Added test_nonempty_extra_params_does_not_crash_init: constructing NeMoGuardrailsServer with extra_params={"foo": 1} must not raise and must leave extra_params intact. Fails before the change (AttributeError), passes after. The existing rail-selection tests still pass, confirming the block was vestigial. black --check clean.

---

Disclosure: this contribution was prepared with the assistance of an AI agent (Claude Code). The analysis, fix, and test were reviewed by me before submission.

[jmartin-tech]

This is not the correct way to address this issue, please see detailed explaination.

[jmartin-tech]

Great catch, there is definitely a logic bug here. The approach taken though does not look to align with the intent as the original guard logic was flawed and does not have a clear comment on what it was meant to change after the call to super.__init__() completed.

The fix to this should not be to remove support for an existing extra_body found in the extra_params dict, a fix should properly handle when one does exist. The changes here actually would result in the user provided extra_body overriding what this generator is attempting to support as self.extra_body would shadowed by self.extra_params["extra_body"] in the current OpenAICompatible._call_model().

This suggests the the guard clause is checking the wrong condition and should be improved so the action taken will preform the correct addition. This guard exists augment the existing expectations for extra_params in OpenAICompatible to pass thru any keys in the extra_params provided via config as parameters to the OpenAI client method used for the inference request by merging the guardrails specific extra_body values.

        if self.extra_params and "extra_body" in self.extra_params:
            user_extra_body = self.extra_params.pop("extra_body", None)
            if user_extra_body:
                self.extra_body = user_extra_body

This would align with the rest of the initialization here by setting self.extra_body to the configured value and allowing the guardrails specific injections to be added to a configuration provided extra_body.

Note the suggestion above is still somewhat naive and should probably be extended with type checking to ensure self.extra_params["extra_body"] is a dict to avoid a type mis-match and raise an early exception about invalid configuration values.

Consider exploring adding more unit tests that evaluate how self.extra_body and self.extra_params will behave in the various combinations. Also consider looking at what happens if you were to mock the call to the underlying OpenAI client object in _call_model to see how these values end up mapping into that methods arguments.

[chuenchen309]

Thanks for the detailed review, @jmartin-tech — you're right, removing the guard outright just traded the crash for a silent shadowing bug. Pushed a fix that pops extra_body out of extra_params and merges it into self.extra_body (using your suggested approach, plus the type-check you mentioned), so a user-configured extra_body and the guardrails config now both make it into the request instead of one clobbering the other.

Added a test that verifies this end-to-end through the mocked OpenAI client (checking the actual outgoing request body has both the user's custom key and the guardrails config merged), plus a test for the ValueError on a non-dict extra_body. Both fail against the previous commit and pass now; existing tests still green.