terrymanu commented on PR #38797:
URL: https://github.com/apache/shardingsphere/pull/38797#issuecomment-4619479013
### Decision
- **Merge Verdict: Mergeable**
- **Reviewed Scope:** PR #38797 at latest head SHA
`0235206b2e4501fb1df3193d1afda0bea3fcd563`, with local merge-base
`731ef1e2e223df7146071b46dcdb8df38b412fe9`. The local changed-file list matches
GitHub `/pulls/38797/files`.
- **Reviewed files:** `test/e2e/mcp` LLM E2E configuration, artifact writer,
runtime support, related tests, and `env/e2e-env.properties`.
- **Not Reviewed Scope:** GitHub CI/check-runs, full repository behavior
outside MCP LLM E2E, and unrelated SQL parser/dialect/runtime paths.
- **Need Expert Review:** No.
### Basis
The change removes redundant `model-size-bytes` metadata while keeping the
two fields that actually preserve model identity and content verification:
`model-revision` and `model-sha256`.
The Docker/runtime path still resolves the model by repository, revision,
and file name, and still verifies the downloaded artifact with `MODEL_SHA256`.
Removing the size value does not weaken the existing checksum-based integrity
check.
Runtime reuse remains safe because `LLME2EConfiguration.ModelMetadata` still
compares repository, file name, quantization, revision, and SHA-256. The
removed size field was only carried through configuration, score evidence, and
assertions.
The score artifact still includes enough model evidence for reproducibility
and debugging, including model reference, revision, file name, SHA-256, runtime
image ID, and base image digest.
No unrelated production path, SQL dialect behavior, or shared high-frequency
runtime behavior is changed.
### Verification
- Fetched PR head and base locally; confirmed local file list matches GitHub
PR files.
- Confirmed no remaining MCP/E2E references to removed model size metadata:
- `rg
"model-size-bytes|modelSizeBytes|getSizeBytes|sizeBytes|1282439264|image-size-bytes|imageSizeBytes"
...`
- Exit code `1`, expected because no matches remain.
- Verified LLM runtime dry-run still resolves required model/image metadata
without size:
- `sh test/e2e/mcp/src/test/resources/docker/llm-runtime/build-local.sh
--dry-run`
- Exit code `0`.
- Ran targeted MCP E2E unit tests:
- `./mvnw -pl test/e2e/mcp -am -DskipITs -Dspotless.skip=true
-Dcheckstyle.skip=true
-Dtest=LLME2EConfigurationTest,LLME2EArtifactWriterTest,LLMChatModelClientTest,LLMRuntimeSupportTest
-Dsurefire.failIfNoSpecifiedTests=false test`
- Exit code `0`.
--
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]
