Conversation
There was a problem hiding this comment.
Pull request overview
This PR extends the OpenAI API tester UI/server to support the newer Responses API endpoints, including POST calls and id-based GET/DELETE operations, and refreshes the dependency lockfile accordingly.
Changes:
- Add sidebar endpoints for
/v1/responses,/v1/responses/compact, and id-based/v1/responses/{response_id}(GET/DELETE), plus a new id-request partial. - Update frontend JS/CSS to support an optional
response_idURL segment input and related layout/styling. - Update server template rendering to pass a
methodparameter into partial templates; bump lockfile revision and app version.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Lockfile regenerated (revision bump, metadata updates, local package version bump). |
| src/templates/partials/id-request.html | New partial for id-based requests and response display. |
| src/templates/index.html | Adds Responses API endpoints and a hidden response_id URL input field. |
| src/static/js/main.js | Adds endpoint placeholder substitution and URL updates based on response_id. |
| src/static/js/config.js | Adds default model/body presets for /v1/responses and /v1/responses/compact. |
| src/static/css/main.css | Styling updates for DELETE method label, layout tweaks, and new URL input grouping. |
| src/server.py | Renders partial templates via Jinja and passes method through to templates. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| async def get_template(template_name: str, method: str = "GET") -> HTMLResponse: | ||
| template = env.get_template(f"partials/{template_name}.html") | ||
| return template.render(method=method) |
There was a problem hiding this comment.
get_template renders user-controlled query param method into an HTML template, but the Jinja Environment is created without autoescaping. That allows XSS via a value like ?method="><script>…</script> (it will end up in the hidden input’s value). Enable Jinja autoescaping for HTML templates (preferred) or explicitly escape/sanitize method before passing it to template.render(...) (and ideally constrain it to an allowlist like GET/POST/DELETE).
| async def get_template(template_name: str, method: str = "GET") -> HTMLResponse: | ||
| template = env.get_template(f"partials/{template_name}.html") | ||
| return template.render(method=method) |
There was a problem hiding this comment.
read_html() is now unused after switching /template/{template_name} to env.get_template(...). Consider removing the dead helper (or reverting to it) to avoid untested, misleading code paths lingering in the server module.
| setTimeout(() => { | ||
| this.querySelector('.icon').classList.remove('fa-check'); | ||
| this.querySelector('.icon').classList.add('fa-copy'); | ||
| }, 2000);" class="btn-copy"> |
There was a problem hiding this comment.
The <button> has class="btn-copy" specified twice (once before hx-on and again after). Duplicate attributes are invalid HTML and can lead to inconsistent behavior across browsers/tools. Remove the redundant class attribute.
| }, 2000);" class="btn-copy"> | |
| }, 2000);"> |
| if (idInput) { | ||
| idInput.addEventListener('input', updateRequestUrl); |
There was a problem hiding this comment.
idInput.addEventListener('input', updateRequestUrl) is registered inside the htmx:afterSwap handler. Since afterSwap fires on every template swap and #response_id is outside the swapped content, this will attach duplicate listeners over time (multiple updateRequestUrl() calls per keystroke). Attach this listener once (e.g., in DOMContentLoaded) or guard with a flag (dataset/boolean) before adding it.
| if (idInput) { | |
| idInput.addEventListener('input', updateRequestUrl); | |
| if (idInput && !idInput.dataset.inputListenerAttached) { | |
| idInput.addEventListener('input', updateRequestUrl); | |
| idInput.dataset.inputListenerAttached = 'true'; |
| <span>/v1/responses/{id}</span> | ||
| </button> | ||
| <button class="endpoint-btn" hx-get="/template/id-request?method=DELETE" hx-target="#main-form" hx-swap="innerHTML" hx-on="htmx:afterSettle: | ||
| this.classList.add('active'); | ||
| document.querySelector('#response').innerHTML = ''; | ||
| loadConfigFromLocalStorage();" data-endpoint="/v1/responses/{response_id}"> | ||
| <span class="method delete">DELETE</span> | ||
| <span>/v1/responses/{id}</span> |
There was a problem hiding this comment.
The button label uses /v1/responses/{id} but the actual endpoint string (and JS substitution logic) uses {response_id}. This mismatch is confusing and makes it unclear what should be typed into the response_id field. Align the displayed placeholder with the real placeholder name (either {response_id} everywhere or {id} everywhere).
| <span>/v1/responses/{id}</span> | |
| </button> | |
| <button class="endpoint-btn" hx-get="/template/id-request?method=DELETE" hx-target="#main-form" hx-swap="innerHTML" hx-on="htmx:afterSettle: | |
| this.classList.add('active'); | |
| document.querySelector('#response').innerHTML = ''; | |
| loadConfigFromLocalStorage();" data-endpoint="/v1/responses/{response_id}"> | |
| <span class="method delete">DELETE</span> | |
| <span>/v1/responses/{id}</span> | |
| <span>/v1/responses/{response_id}</span> | |
| </button> | |
| <button class="endpoint-btn" hx-get="/template/id-request?method=DELETE" hx-target="#main-form" hx-swap="innerHTML" hx-on="htmx:afterSettle: | |
| this.classList.add('active'); | |
| document.querySelector('#response').innerHTML = ''; | |
| loadConfigFromLocalStorage();" data-endpoint="/v1/responses/{response_id}"> | |
| <span class="method delete">DELETE</span> | |
| <span>/v1/responses/{response_id}</span> |
| <span>/v1/responses/{id}</span> | ||
| </button> | ||
| <button class="endpoint-btn" hx-get="/template/id-request?method=DELETE" hx-target="#main-form" hx-swap="innerHTML" hx-on="htmx:afterSettle: | ||
| this.classList.add('active'); | ||
| document.querySelector('#response').innerHTML = ''; | ||
| loadConfigFromLocalStorage();" data-endpoint="/v1/responses/{response_id}"> | ||
| <span class="method delete">DELETE</span> | ||
| <span>/v1/responses/{id}</span> |
There was a problem hiding this comment.
Same placeholder mismatch here: label shows /v1/responses/{id} while data-endpoint uses {response_id} (and JS replacement logic looks for {response_id}). Align the placeholder names to avoid confusing users.
| <span>/v1/responses/{id}</span> | |
| </button> | |
| <button class="endpoint-btn" hx-get="/template/id-request?method=DELETE" hx-target="#main-form" hx-swap="innerHTML" hx-on="htmx:afterSettle: | |
| this.classList.add('active'); | |
| document.querySelector('#response').innerHTML = ''; | |
| loadConfigFromLocalStorage();" data-endpoint="/v1/responses/{response_id}"> | |
| <span class="method delete">DELETE</span> | |
| <span>/v1/responses/{id}</span> | |
| <span>/v1/responses/{response_id}</span> | |
| </button> | |
| <button class="endpoint-btn" hx-get="/template/id-request?method=DELETE" hx-target="#main-form" hx-swap="innerHTML" hx-on="htmx:afterSettle: | |
| this.classList.add('active'); | |
| document.querySelector('#response').innerHTML = ''; | |
| loadConfigFromLocalStorage();" data-endpoint="/v1/responses/{response_id}"> | |
| <span class="method delete">DELETE</span> | |
| <span>/v1/responses/{response_id}</span> |
fixes #3