NOISSUE - Refactor guardrails to sidecar #132
Conversation
smithjilks
force-pushed
the
NOISSUE-guardrails-sidecar
branch
from
e3fac57 to
dcc852e
Compare
smithjilks
changed the title
smithjilks
force-pushed
the
NOISSUE-guardrails-sidecar
branch
2 times, most recently
from
2a61df1 to
7250492
Compare
|
|
||
| cleaned = response.strip() | ||
|
|
||
| cleaned = re.sub(r'^(bot|I)\s+\w+(\s+\w+)*\s*\n', '', cleaned, flags=re.IGNORECASE) |
There was a problem hiding this comment.
removes any first line that begins with “I …”, which will delete normal assistant answers. Constrain this to explicit prefixes
| return cleaned.strip() | ||
|
|
||
|
|
||
| @router.post("/messages", tags=["chat"]) |
There was a problem hiding this comment.
we should not have this endpoint
There was a problem hiding this comment.
if it is purely internal you can leave it
There was a problem hiding this comment.
This is internal. And it's needed. Perhaps renaming, but the functionality remains the same: to receive chat contents in the Guardrails service.
proxy/api/transport.go
Outdated
| if !ok { | ||
| func guardrailsHandler(transport http.RoundTripper, rter *router.Router) http.HandlerFunc { | ||
| return func(w http.ResponseWriter, r *http.Request) { | ||
| if r.Header.Get("X-Guardrails-Request") != "true" { |
There was a problem hiding this comment.
any client can bypass it by simply adding the header. Replace the header check with proper authentication—either apply the same authn.Middleware() used elsewhere or validate a cryptographically signed token.
There was a problem hiding this comment.
Best way to handle is this is building a custom LLM wrapper.
There was a problem hiding this comment.
Best way to handle is this is building a custom LLM wrapper.
| return nil, 0, err | ||
| } | ||
| defer resp.Body.Close() | ||
| req.URL.RawQuery = target.RawQuery |
There was a problem hiding this comment.
Current logic overwrites the incoming query if target.RawQuery is set. Merge them to avoid losing client parameters.
smithjilks
force-pushed
the
NOISSUE-guardrails-sidecar
branch
from
dcd2de0 to
70fbf66
Compare
proxy/middleware/model_auth.go
Outdated
| modelAuth = bearerPrefix + modelAuth | ||
| } | ||
|
|
||
| r.Header.Set(authorizationHeader, modelAuth) |
There was a problem hiding this comment.
remove X -Model-Authorization since it is not used downstream
| ) | ||
|
|
||
| # Extract text from ChatResult | ||
| return result.generations[0].text |
There was a problem hiding this comment.
if generations is nil, will panic
Signed-off-by: Jilks Smith <smithjilks@gmail.com>
Signed-off-by: Jilks Smith <smithjilks@gmail.com>
Signed-off-by: Jilks Smith <smithjilks@gmail.com>
Signed-off-by: Jilks Smith <smithjilks@gmail.com>
Signed-off-by: Jilks Smith <smithjilks@gmail.com>
Signed-off-by: Jilks Smith <smithjilks@gmail.com>
smithjilks
force-pushed
the
NOISSUE-guardrails-sidecar
branch
from
b492ec7 to
85dfc23
Compare
3 participants
Signed-off-by: Jilks Smith smithjilks@gmail.com
What type of PR is this?
This is a bug fix because it routes llm requests from Nemo Guardrails through the proxy.
What does this do?
Routes llm requests from Nemo Guardrails through the proxy.
Which issue(s) does this PR fix/relate to?
Resolves #51
Have you included tests for your changes?
No.
Did you document any new/modified features?
No.
Notes