API: generate /api/state documentation from UI types - #32431
Merged
Conversation
Contributor
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The Vehicle.name field has been made optional but many UI usages still assume a non-empty identifier (e.g. Vehicles/Title.vue), so consider either keeping name required in the schema or explicitly handling missing names in all consumers.
- The IGNORE set in scripts/state-schema/validate.ts hard-codes undocumented paths (e.g. "$.evopt"), which can easily drift from the actual payload; consider deriving this from schema metadata or a shared config instead of string literals.
- scripts/state-schema/schemas.ts relies on hard-coded file paths and type names (ROOT, OPENAPI_PATH, STATE_PATH), so it may be worth adding small assertions or configuration hooks to fail fast if these change to avoid silent mis-generation of openapi.state.yaml.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The Vehicle.name field has been made optional but many UI usages still assume a non-empty identifier (e.g. Vehicles/Title.vue), so consider either keeping name required in the schema or explicitly handling missing names in all consumers.
- The IGNORE set in scripts/state-schema/validate.ts hard-codes undocumented paths (e.g. "$.evopt"), which can easily drift from the actual payload; consider deriving this from schema metadata or a shared config instead of string literals.
- scripts/state-schema/schemas.ts relies on hard-coded file paths and type names (ROOT, OPENAPI_PATH, STATE_PATH), so it may be worth adding small assertions or configuration hooks to fail fast if these change to avoid silent mis-generation of openapi.state.yaml.
## Individual Comments
### Comment 1
<location path="tests/state-api.spec.ts" line_range="16-25" />
<code_context>
+ await stop();
+});
+
+test("/api/state matches the openapi State schema", async ({ request }) => {
+ // loadpoint values appear over the first update cycles, poll until stable
+ await expect
+ .poll(
+ async () => {
+ const res = await request.get("/api/state");
+ const { errors, undocumented } = validateState(await res.json());
+ return [...errors, ...undocumented];
+ },
+ { timeout: 30000 }
+ )
+ .toEqual([]);
+});
</code_context>
<issue_to_address>
**suggestion (testing):** Assert basic HTTP response properties in addition to schema compliance
Right now the test assumes the request succeeds and only checks schema compliance. Please also assert basic HTTP properties (e.g. `expect(res.ok()).toBeTruthy()` or `expect(res.status()).toBe(200)`, and optionally the `application/json` content type) before passing the body to `validateState`, so failures in the endpoint are distinguished from malformed payloads.
Suggested implementation:
```typescript
test("/api/state matches the openapi State schema", async ({ request }) => {
// loadpoint values appear over the first update cycles, poll until stable
await expect
.poll(
async () => {
const res = await request.get("/api/state");
// Assert basic HTTP response properties before validating the payload
expect(res.ok()).toBeTruthy();
expect(res.status()).toBe(200);
const contentType = res.headers()["content-type"] ?? "";
expect(contentType).toContain("application/json");
const { errors, undocumented } = validateState(await res.json());
return [...errors, ...undocumented];
},
{ timeout: 30000 }
)
.toEqual([]);
});
```
You may want to remove the duplicated `test.afterAll(async () => { await stop(); });` earlier in the file if it is not intentional, to avoid running teardown twice.
</issue_to_address>
### Comment 2
<location path="server/mcp/openapi.md" line_range="1151" />
<code_context>
+## getState
+
+Returns the complete state of the system. This structure is used by the UI and also published via websocket and MQTT. It can be filtered by JQ to only return a subset of the data. Note: the response mirrors the internal UI state and carries no compatibility promise. Fields may change or disappear between releases.
+
+**Tags:** state
</code_context>
<issue_to_address>
**suggestion (typo):** Use the standard capitalization for the WebSocket protocol name.
Change "websocket" to "WebSocket" to match the standard protocol name.
```suggestion
Returns the complete state of the system. This structure is used by the UI and also published via WebSocket and MQTT. It can be filtered by JQ to only return a subset of the data. Note: the response mirrors the internal UI state and carries no compatibility promise. Fields may change or disappear between releases.
```
</issue_to_address>
### Comment 3
<location path="scripts/state-schema/schemas.ts" line_range="70" />
<code_context>
+ });
+}
+
+function collectRefs(schema: AnySchema): Set<string> {
+ const refs = new Set<string>();
+ walk(schema, (node) => {
</code_context>
<issue_to_address>
**issue (complexity):** Consider inlining closely-related helpers, splitting dense transformations into smaller functions, and centralizing shared `$ref` logic to make the schema-processing flow easier to follow and maintain.
The main complexity comes from the combination of a generic `walk` plus multiple global passes. You can keep all functionality but make the flow easier to follow by:
---
### 1. Inline `collectRefs` into `reachableDefs`
You only ever use `collectRefs` from `reachableDefs`, and both rely on the same `walk` pattern. Inlining the logic removes one helper and one conceptual “jump” when reading the code:
```ts
function reachableDefs(
root: AnySchema,
defs: Record<string, AnySchema>
): Record<string, AnySchema> {
const keep: Record<string, AnySchema> = {};
const queue: string[] = [];
const collectRefs = (schema: AnySchema) => {
walk(schema, (node) => {
if (typeof node.$ref === "string") {
queue.push(decodeURIComponent(node.$ref.replace("#/definitions/", "")));
}
});
};
collectRefs(root);
while (queue.length > 0) {
const name = queue.shift()!;
if (keep[name] || !defs[name]) continue;
keep[name] = defs[name];
collectRefs(defs[name]);
}
return keep;
}
```
This keeps the reachability semantics intact but makes the analysis self-contained.
---
### 2. Split `normalizeNullables` into explicit helpers
Handling both `type: ['...', 'null']` and `anyOf`-based unions in one dense visitor is hard to parse. Small, named helpers make the intent clearer:
```ts
function normalizeTypeUnionNullable(node: AnySchema): void {
if (!Array.isArray(node.type) || !node.type.includes("null")) return;
const rest = node.type.filter((t: string) => t !== "null");
if (rest.length !== 1) {
throw new Error(`unsupported type union ${node.type}`);
}
node.type = rest[0];
node.nullable = true;
}
function normalizeAnyOfNullable(node: AnySchema): void {
if (!Array.isArray(node.anyOf)) return;
if (!node.anyOf.some((b: AnySchema) => b.type === "null")) return;
const rest = node.anyOf.filter((b: AnySchema) => b.type !== "null");
delete node.anyOf;
node.nullable = true;
if (rest.length === 1 && !rest[0].$ref) {
Object.assign(node, rest[0]);
} else {
node.allOf = rest;
}
}
function normalizeNullables(schema: AnySchema): void {
walk(schema, (node) => {
normalizeTypeUnionNullable(node);
normalizeAnyOfNullable(node);
if ("const" in node) {
node.enum = [node.const];
delete node.const;
}
if (Array.isArray(node.examples)) {
node.example = node.examples[0];
delete node.examples;
}
});
}
```
Same behavior, but the nullable transforms are now readable and testable independently.
---
### 3. Centralize `$ref` parsing/formatting
Right now you decode and rewrite `#/definitions/...` in multiple places (`collectRefs`, `rewriteRefs`). A small utility keeps that logic in one place and reduces duplication:
```ts
const DEFINITIONS_PREFIX = "#/definitions/";
const COMPONENTS_PREFIX = "#/components/schemas/";
function parseDefinitionRef(ref: string): string | null {
if (!ref.startsWith(DEFINITIONS_PREFIX)) return null;
return decodeURIComponent(ref.slice(DEFINITIONS_PREFIX.length));
}
function formatComponentRef(name: string): string {
return `${COMPONENTS_PREFIX}${name}`;
}
function rewriteRefs(schema: AnySchema, finalNames: Map<string, string>): void {
walk(schema, (node) => {
const rawRef = node.$ref;
if (typeof rawRef !== "string" || rawRef.startsWith(COMPONENTS_PREFIX)) return;
const name = parseDefinitionRef(rawRef);
if (!name) return; // or throw if every non-components ref must be a definition
const renamed = finalNames.get(name);
if (!renamed) throw new Error(`unresolved $ref "${rawRef}"`);
node.$ref = formatComponentRef(renamed);
});
}
```
Then `reachableDefs` can reuse `parseDefinitionRef` instead of repeating the `replace`/`decodeURIComponent` logic.
---
These changes keep your feature intact but reduce incidental complexity by:
- Collapsing closely-coupled helpers (`collectRefs` + `reachableDefs`).
- Making nullable transformations explicit and independently understandable.
- Centralizing `$ref` handling so future changes only touch one utility.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
naltatis
enabled auto-merge (squash)
August 3, 2026 11:05
andig
added a commit
that referenced
this pull request
Aug 3, 2026
The /state operation moved from the general to the state tag in #32431, but the tag was not added to the MCP tag filter, so getState stopped being registered as a tool.
This was referenced Aug 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
replaces #31530, which was reverted in #32109
Re-lands the
/api/stateschema generation. The revert was needed because docker builds broke in two places:Dockerfilestill copied the removedcmd/openapi/, andnpm run buildhad grown an openapi generation step the node stage cannot run, since it needstsconfig.json,scripts/and the specs underserver/.Rather than pushing all of that into the node build context, the generation now stays out of the UI build entirely. Its outputs are committed artifacts consumed by the Go build, never by vite, so there is no reason for docker to regenerate them:
make openapitarget (vp run openapi), run as its own step in the UI CI job and covered by the existing porcelain check. Same shape asmake assetsforgo generateDockerfiledrops the staleCOPY cmd/openapi/, the node stage is untouchedyamloutput is already oxfmt clean after the vite+ migrationTwo changes from the original PR are not re-applied because master has since settled on them: response codes stay quoted (#31984) and the state endpoint keeps its
getStateoperation id, so the MCP tool name does not change.Original work by @naltatis.
🤖 Generated with Claude Code