Add Judgment CLI (JUD-5155)#1
Conversation
JUD-5155 Make Judgment CLI
Background
Technical
|
|
|
||
|
|
||
| def _apply_request_defaults(body: object, schema: dict) -> None: | ||
| """Fill in schema-driven defaults for generated POST bodies.""" |
There was a problem hiding this comment.
why is this needed. the openapi schema defafults are not meant to be followed by consumers
| import click | ||
|
|
||
|
|
||
| def _output(data: object) -> None: |
There was a problem hiding this comment.
can we have a more clean UI file to centralize UI because im sure CLIs will require more stuff specifi here
| _DEFAULT_BASE_URL = "https://api3.judgmentlabs.ai" | ||
|
|
||
|
|
||
| def _config_dir() -> Path: |
There was a problem hiding this comment.
Will this work on windows? also why specifically XDG_CONFIG_HOME?
| or cfg.get("base_url") | ||
| or _DEFAULT_BASE_URL | ||
| ) | ||
| api_key = ( |
There was a problem hiding this comment.
just a nit but do we want to follow similar patterns to judgeval with requireEnvVar and optionalEnvVar instaed of having this kinda thing
| # List traces with custom filters | ||
| judgment traces list <PROJECT_ID> -d '{"pagination": {"limit": 10, "cursorSortValue": null, "cursorItemId": null}}' | ||
|
|
||
| # Get trace details |
There was a problem hiding this comment.
these formats dont seem right for a CLI.
maybe try to follow the strategy something like: https://www.npmjs.com/package/@openapitools/openapi-generator-cli?
I think its a fine V1 but its a bit clunky of a CLI since you need to guess the arg orrder.
Can you also see if click comes with terminal auto completions? Something like this:
https://click.palletsprojects.com/en/stable/shell-completion/
you can look at https://cli.ahh.bet/ as an example CLI installation thats more common practice
| @@ -0,0 +1,66 @@ | |||
| class JudgmentCli < Formula | |||
There was a problem hiding this comment.
If this works its cool. Do we want to support only homebrew or also a:
curl -fsSL https://judgmentlabs.ai/install.sh | bashKinda thing
e318e8b to
00e9e67
Compare
| if qp["required"]: | ||
| type_arg = click_param_args(qp["schema"]) | ||
| lines.append(f'@click.argument("{qp["name"]}"{type_arg})') |
There was a problem hiding this comment.
🟡 Generator emits mismatched Click argument name vs function parameter for camelCase names
At scripts/generate_cli.py:299, required query params generate @click.argument("{qp["name"]}") using the raw API name (e.g. matchCount), but the function signature at line 335 uses py_var_name(q["name"]) (e.g. match_count). Click derives the Python parameter name from the argument declaration by only replacing - with _, so @click.argument("matchCount") maps to parameter matchCount, not match_count. The same mismatch exists for required scalar body props at line 311. When the OpenAPI spec contains a camelCase required parameter, this will produce a TypeError at runtime because Click passes the argument as a keyword argument that doesn't match the function signature.
Affected code paths
Line 299 for required query params:
lines.append(f'@click.argument("{qp["name"]}"...')Line 311 for required scalar body props:
lines.append(f'@click.argument("{prop["name"]}"...')But function signature at line 335 uses py_var_name() which converts camelCase → snake_case.
Was this helpful? React with 👍 or 👎 to provide feedback.
abhishekg999
left a comment
There was a problem hiding this comment.
I like that this is all codegen, I will list some issues with specific commands during testing. Also due to the fact that this is all codegen I think its fair we should have some tests in this repo to test these (non codegen).
| @@ -0,0 +1,141 @@ | |||
| #!/usr/bin/env python3 | |||
| """Render the Homebrew formula for judgment-cli. | |||
| python3 scripts/generate_cli.py $(SPEC_URL) | ||
|
|
||
| # Install the CLI in the current Python environment | ||
| install: |
There was a problem hiding this comment.
wait this will use my global pip by default right
we should be using uv here right
|
|
||
|
|
||
| @cli.command() | ||
| def logout() -> None: |
There was a problem hiding this comment.
One suggestion:
login and logout is fine, I would suggest some judgment configure. right now if i login i dont really have a way to change my organizeation id right other than either manually editing the file, or regoing through the login flow.
I think we should not propagate the error message of requiring organization id like this. For routes that require the organization that should be something we can do a more proper CLI error for. |
❯ JUDGMENT_BASE_URL=http://localhost:10006 judgment docs search hi
Error 401: {
"error": "Authentication failed",
"message": "X-Organization-Id header is required"
}It seeems even docs search required X-Organization-Id? is this intentional. It seems infact everything requires organization id and in that case should we simply make it mandatory. |
|
Also in general we really need a completion script IMO. If we really just want agents to use it then its probably fine but: otherwise this is very inconvenient to work with ids here. I think having tab completion script here wuld be good. if this seems out of scope for this first Pr (which i would say so, lets atleast make a ticket for this) |
dccf0d1 to
753823b
Compare
753823b to
87d20f6
Compare
Uh oh!
There was an error while loading. Please reload this page.