fix: gto --plain doesnt truncate wide versions#490
fix: gto --plain doesnt truncate wide versions#490ZivotJeKrasny wants to merge 1 commit intoiterative:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes issue #489 where gto show --plain (global registry view) could truncate “wide” SemVer strings (e.g. v0.100.0 → v0.100.), breaking machine parsing of CLI output.
Changes:
- Update
_show_registry()to truncate only real 40-char hex SHAs (viais_hexsha()), not arbitrary strings. - Add regression tests covering wide SemVer in global
--plainoutput and_show_registry()table output. - Add an additional test intended to ensure hexsha truncation still occurs (currently incomplete; see comments).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
gto/api.py |
Fixes truncation logic by guarding truncation with is_hexsha() in _show_registry(). |
tests/test_cli.py |
Adds CLI regression test asserting global gto show --plain output preserves wide SemVer. |
tests/test_api.py |
Adds API-level regression test for _show_registry() wide SemVer; adds a hexsha truncation “sanity” test that currently doesn’t assert truncation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| from gto.cli import app | ||
| from gto.exceptions import GTOException | ||
| import gto as gto_lib |
| """Sanity check: actual hex SHAs in stage columns are still truncated.""" | ||
| version = "v0.0.1" | ||
| gto.api.register(tmp_dir, "model", "HEAD", version) | ||
| # assign using raw commit hexsha as version (non-semver) via API internals | ||
| # — we verify via the stage dict that real hexshas are truncated while | ||
| # semver strings are left intact. | ||
| rows, _ = gto.api._show_registry(tmp_dir, table=True, truncate_hexsha=True) | ||
| model_row = next(r for r in rows if "model" in r["name"]) | ||
| # The semver-registered version must never be truncated | ||
| assert model_row["latest"] == version | ||
|
|
||
|
|
| @@ -319,7 +319,7 @@ def _show_registry( | |||
| """Show current registry state""" | |||
|
|
|||
| def format_hexsha(hexsha): | |||
There was a problem hiding this comment.
how / where is called outside?
It seems the problem here is that we pass something that is not hash into a function that is named format_hexsha . T
This guard should be outside of this function.
shcheklein
left a comment
shcheklein
left a comment
There was a problem hiding this comment.
Thanks for your PR. Please review comment's address each meaningfully. Probably a bit broader refactoring is needed here.
Fixes #489