FANNG1 commented on PR #12424:
URL: https://github.com/apache/gravitino/pull/12424#issuecomment-5322435798
Read this along one flow the document doesn't currently describe: an agent
drafts a semantic model from a set of tables, a human reviews and corrects it,
and other agents then consume it. The design covers governed CRUD on a
finished, correct model well — identity, versioning, storage, authorization,
events, parent lifecycle are all thought through. What's missing is that the
write path assumes a single writer who already knows the right answer, while
this flow has multiple writers, a trust boundary between them, and an iteration
loop. I'd suggest adding a short section that walks it. Two items below seem
cheap enough to commit to now.
### 1. Expose version history — the data is already stored
`semantic_model_version_info` keeps a full snapshot per version with
`current_version` / `last_version` bookkeeping, but the design states the API
exposes no version identifiers, historical loads, or rollback. Review means
diff: "what did the last write change?" Without a historical load a reviewer
has nothing to compare against, and those snapshots stay purely internal
bookkeeping. Read-only `GET .../semantic-models/{name}/versions` plus
`load?version=N` would be enough; rollback can stay out of scope, since
`replaceDefinition` with an older snapshot covers it. One caveat worth stating
either way: agent-driven iteration produces versions quickly and retention is
EntityStore-configured, so the version a reviewer wants may already have been
collected.
### 2. Validate without persisting, and return structured violations
Create and alter are strict-schema, `additionalProperties: false`,
all-or-nothing, `400`. An agent generating a model has no way to check a
candidate without writing it, and no way to learn more than one problem per
attempt if validation is fail-fast. `POST .../semantic-models:validate` (or
`?validateOnly=true`), returning every violation at once with a JSON pointer
path and a code rather than one concatenated message, is what makes an agent's
repair loop converge in a bounded number of rounds. It is also the natural home
for the three validation layers the design already separates — contract,
model-local, and catalog.
### Consumption
- **Discovery costs O(catalogs × schemas) round trips.**
`listSemanticModels` returns identifiers only, per schema, so answering "where
is revenue defined" means listing everywhere and loading everything to filter
client-side. `listFunctionInfos` + `?details=true`
(`api/src/main/java/org/apache/gravitino/function/FunctionCatalog.java:48`,
`server/src/main/java/org/apache/gravitino/server/web/rest/FunctionOperations.java:93`)
is the existing precedent for summaries; a metalake-level lookup returning
fully-qualified `catalog.schema.model.metric` identity matters more. Ranking
candidates is the agent's job rather than Gravitino's, but the inputs should be
reachable in one call instead of one request per object against
`/metalakes/{metalake}/objects/{type}/{fullName}/tags`
(`server/src/main/java/org/apache/gravitino/server/web/rest/MetadataObjectTagOperations.java:71`)
and its owner and policy equivalents.
- **`ai_context.synonyms` should be part of the discovery surface, not just
stored payload.** It is the field that resolves a user's word — "revenue" — to
`total_revenue`, and in an agent setting that resolution *is* the operation,
whereas in a BI tool a human simply clicks the right entry. The design only
promises it is retained losslessly, so today an agent must pull every model
down and build its own synonym index client-side. That means each tool builds a
different index, they drift, and the same question resolves to different
metrics across tools — which undercuts the single-source-of-truth claim at
exactly the step that matters most. Returning `synonyms` in the list summary
would already be enough to build that index in one call; whether server-side
lookup is in scope is worth stating either way.
- **No canonical Ossie output**, although the server already builds exactly
that projection in memory to validate against the pinned schema. A read-only
export (`?format=ossie`, or `Accept: application/yaml`) would let any
Ossie-compatible consumer skip reimplementing the entity→Ossie mapping,
including the three-part `source` string handling raised above.
- **Dialect resolution is unspecified** — exact match, `ANSI_SQL` fallback,
then error? Without a stated rule the same metric yields different results per
engine, which compounds the closed-enum concern raised above.
- **A stored definition may reference a dropped source and loads still
return it**, so an agent cannot separate "my SQL is wrong" from "the metadata
is stale," and will retry the same failure. A `lastValidatedAt` field, or an
opt-in `load?validate=true`, would be enough.
Finally, the development plan covers Java client, Python client and UI, but
has no `mcp-server` item, even though the repo already ships a full tool set
under `mcp-server/mcp_server/tools/`. If AI agents are a target consumer, that
is the task that makes the path real.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
