Skip to content

feat: support configurable GTS ID prefixes#106

Open
aviator5 wants to merge 2 commits into
GlobalTypeSystem:mainfrom
aviator5:custom-gts-id-prefix
Open

feat: support configurable GTS ID prefixes#106
aviator5 wants to merge 2 commits into
GlobalTypeSystem:mainfrom
aviator5:custom-gts-id-prefix

Conversation

@aviator5

@aviator5 aviator5 commented Jun 25, 2026

Copy link
Copy Markdown
Contributor
  • Add compile-time GTS_ID_PREFIX configuration with validation and Cargo rebuild tracking.
  • Thread the configured prefix through ID parsing, schema generation, validation, and schema reference handling.
  • Add macro argument support and tests for prefix-aware ID construction.

Summary by CodeRabbit

  • New Features
    • Added compile-time configurable GTS ID prefix (GTS_ID_PREFIX) and updated identifier generation/validation to honor it.
    • Introduced gts_id! macro with a prefix-aware gts_id!("...") marker form.
    • Added gts-dylint lint to flag hard-coded GTS ID prefixes, configurable via GTS_DYLINT_PREFIXES.
  • Bug Fixes
    • Updated JSON schema, $id/$ref, normalization, Markdown discovery, and x-gts-ref handling to use the configured prefix consistently.
  • Documentation
    • Expanded README with prefix rules, make dylint, and prefix-aware test instructions.
  • Tests
    • Refreshed unit/compile-fail/golden fixtures and added prefix-specific CI checks (test-gts-id-prefix) and dylint runs.

@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR makes GTS identifier prefixes configurable at compile time, updates parsing, macro, validator, and schema-generation code to use the new prefix constants, adds a gts_id! suffix macro, and refreshes related docs and tests.

Changes

Configurable prefix and identifier macro

Layer / File(s) Summary
Prefix constants and tooling
README.md, gts-id/README.md, gts-id/build.rs, gts-id/src/prefix.rs, gts-id/src/lib.rs, gts/src/lib.rs, gts/src/gts.rs, Cargo.toml, Makefile, .github/workflows/ci.yml, .github/workflows/release.yml, CLAUDE.md, gts-dylint/*, gts-macros-cli/src/main.rs, gts/src/ops.rs
Compile-time prefix constants, build-script rebuild hints, public re-exports, workspace/tooling config, and prefix documentation now describe the configurable GTS identifier prefix.
GTS ID parsing
gts-id/src/parse.rs, gts-id/src/gts_id.rs, gts-id/src/gts_id_pattern.rs, gts-id/src/gts_id_segment.rs
GTS ID parsing, formatting, and prefix-length checks now use GTS_ID_PREFIX and GTS_ID_MAX_LENGTH, with matching test updates for the new prefix text.
Macro argument parsing
gts-macros/src/id_arg.rs, gts-macros/src/lib.rs, gts-macros/src/instance.rs
Shared id-argument helpers and the gts_id! proc macro expand suffix markers, and struct_to_gts_schema, gts_instance!, and gts_instance_raw! use them for typed and raw IDs.
Macro docs and tests
gts-macros/README.md, gts-macros/tests/compile_fail/*, gts-macros/tests/golden/*, gts-macros/tests/*_tests.rs
Macro documentation, compile-fail output, golden cases, and instance/integration tests now use gts_id! and the revised id-format diagnostics.
Schema identifier outputs
gts-cli/src/gen_schemas.rs, gts/src/gts.rs, gts/src/schema.rs, gts/src/schema_modifiers.rs, gts/src/schema_traits.rs
Public schema $id and x-gts-ref values, plus generated schema references, now use the configurable prefix constants instead of hardcoded gts. and gts:// strings.
Schema ref normalization
gts/src/entities.rs, gts/src/schema_refs.rs, gts/src/schema_resolver.rs, gts/src/store.rs, gts/src/testing.rs, gts/src/x_gts_ref.rs
Entity extraction, $ref canonicalization, chain traversal, and x-gts-ref validation now strip or match against the configured URI and ID prefixes.
Validator prefix matching
gts-validator/src/format/*, gts-validator/src/normalize.rs, gts-validator/src/output.rs
JSON and Markdown discovery, candidate normalization, and human-readable validation text now compare against GTS_ID_PREFIX and GTS_ID_URI_PREFIX.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • GlobalTypeSystem/gts-rust#51: Shares the same gts-macros struct_to_gts_schema validation path that this PR extends with gts_id! marker support.
  • GlobalTypeSystem/gts-rust#81: Touches the gts_instance! and gts_instance_raw! implementation path that this PR updates for expression-based id parsing.
  • GlobalTypeSystem/gts-rust#103: Modifies the schema $ref prefix-stripping path that this PR also updates to use GTS_ID_URI_PREFIX.

Suggested reviewers

  • Artifizer
  • MikeFalcon77

Poem

A rabbit hops through prefix fields,
gts_id! seeds what suffix yields.
The schema moon now twinkles bright,
With configurable dots of light. 🐇

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: configurable GTS ID prefixes.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov-commenter

codecov-commenter commented Jun 25, 2026

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 97.56098% with 12 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
gts-macros/src/id_arg.rs 86.66% 4 Missing ⚠️
gts-macros-cli/src/main.rs 0.00% 3 Missing ⚠️
gts-macros/src/lib.rs 92.30% 2 Missing ⚠️
gts-id/src/parse.rs 98.14% 1 Missing ⚠️
gts-id/src/prefix.rs 97.91% 1 Missing ⚠️
gts-macros/src/instance.rs 94.11% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@aviator5 aviator5 force-pushed the custom-gts-id-prefix branch 3 times, most recently from d21a1ee to 1229d16 Compare June 26, 2026 12:23
@aviator5 aviator5 requested a review from Artifizer June 26, 2026 12:23
@aviator5 aviator5 marked this pull request as ready for review June 26, 2026 12:27

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
gts-validator/src/format/json.rs (1)

80-98: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Validate every non-pointer x-gts-ref value.

Line 98 still uses the generic candidate pre-filter for x-gts-ref. Because x-gts-ref only permits GTS IDs/patterns, /..., or *, a stale prefix like gts.x... under a custom GTS_ID_PREFIX is now silently skipped instead of reported.

Proposed fix
-            if looks_like_filename {
+            if looks_like_filename && !is_xgts_ref {
                 return;
             }
 
-            if looks_like_gts_candidate(candidate_str) {
+            if is_xgts_ref || looks_like_gts_candidate(candidate_str) {
                 match normalize_candidate(candidate_str) {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@gts-validator/src/format/json.rs` around lines 80 - 98, The x-gts-ref
validation path in json formatting is still relying on the generic
looks_like_gts_candidate pre-filter, which can skip invalid non-pointer values
instead of reporting them. Update the validation logic around is_xgts_ref,
looks_like_filename, and looks_like_gts_candidate so that x-gts-ref accepts only
"/", "*", or valid GTS IDs/patterns and does not suppress stale prefixed values
like gts.x... under a custom prefix; ensure non-pointer x-gts-ref strings are
always checked and rejected when they are not valid refs.
🧹 Nitpick comments (2)
gts-validator/src/normalize.rs (1)

46-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Cover the non-default prefix path in this module's tests.

normalize_candidate is now driven by GTS_ID_PREFIX/GTS_ID_URI_PREFIX, but the tests below still hardcode gts. literals. That leaves the newly supported custom-prefix build unexercised here.

🧪 Example adjustment
 #[test]
 fn test_normalize_gts_uri() {
-    let result = normalize_candidate("gts://gts.x.core.type.v1~").unwrap();
-    assert_eq!(result.gts_id, "gts.x.core.type.v1~");
-    assert_eq!(result.original, "gts://gts.x.core.type.v1~");
+    let id = format!("{GTS_ID_PREFIX}x.core.type.v1~");
+    let uri = format!("{GTS_ID_URI_PREFIX}{id}");
+    let result = normalize_candidate(&uri).unwrap();
+    assert_eq!(result.gts_id, id);
+    assert_eq!(result.original, uri);
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@gts-validator/src/normalize.rs` around lines 46 - 60, The tests in this
module still hardcode the default gts. prefix, so the custom-prefix path in
normalize_candidate is not being exercised. Update the tests around
normalize_candidate to reference GTS_ID_PREFIX and GTS_ID_URI_PREFIX instead of
literal gts. strings, and add coverage that verifies normalization and rejection
behavior when the build is configured with a non-default prefix. Keep the
assertions aligned with the same helper names so the prefix-driven logic in
normalize_candidate is covered in both plain and URI forms.
gts-macros/src/lib.rs (1)

2302-2306: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add direct negative coverage for gts_id!.

The new proc-macro path is still missing coverage according to the PR notes, and the uncovered branches here are the ones most likely to regress silently: malformed standalone input in gts-macros/src/lib.rs and bad-body handling in gts-macros/src/id_arg.rs. A small compile-fail case for gts_id!(123) plus one nested type_id = gts_id!(123) case would lock down both diagnostics cheaply.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@gts-macros/src/lib.rs` around lines 2302 - 2306, Add direct negative test
coverage for gts_id! by exercising the proc-macro entry point in gts_id and the
parsing path in id_arg::build_prefixed_lit. Add a compile-fail case for
malformed standalone input like gts_id!(123), and a second case for invalid
nested usage like type_id = gts_id!(123), so both the top-level macro input
handling and the bad-body diagnostics stay covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@gts-id/src/parse.rs`:
- Around line 319-323: The parser test module is only partially prefix-agnostic
because some success-path fixtures still use hardcoded gts IDs while the error
assertions already reference GTS_ID_PREFIX. Update the affected tests in
parse.rs to build expected and input IDs from the configurable prefix instead of
literal "gts...." strings, using the existing parser test helpers and any
prefix-aware constants or constructors in the module. Make sure the cases around
the checked assertions and related success-path tests all derive from the same
prefix source so the module passes under non-default compile-time prefixes.

In `@gts-id/src/prefix.rs`:
- Around line 86-90: The test test_default_prefix_is_in_effect is hardcoding the
default prefix, so it fails when GTS_ID_PREFIX is overridden at compile time.
Update the assertion logic to reference GTS_ID_PREFIX and DEFAULT_GTS_ID_PREFIX
in a way that accepts the active configured value, rather than requiring "gts."
specifically. Keep the test aligned with the prefix constants in prefix.rs so it
validates the compile-time override behavior instead of rejecting it.

In `@gts-macros/README.md`:
- Around line 797-799: The README text for the gts_id macro is hardcoding the
backward-compatible full literal as gts...., which is misleading when
GTS_ID_PREFIX is overridden. Update the documentation around gts_id! and the
full-literal form to state that the accepted literal uses the configured prefix
value, while keeping the note that qualified paths like
gts_macros::gts_id!("...") are supported.

---

Outside diff comments:
In `@gts-validator/src/format/json.rs`:
- Around line 80-98: The x-gts-ref validation path in json formatting is still
relying on the generic looks_like_gts_candidate pre-filter, which can skip
invalid non-pointer values instead of reporting them. Update the validation
logic around is_xgts_ref, looks_like_filename, and looks_like_gts_candidate so
that x-gts-ref accepts only "/", "*", or valid GTS IDs/patterns and does not
suppress stale prefixed values like gts.x... under a custom prefix; ensure
non-pointer x-gts-ref strings are always checked and rejected when they are not
valid refs.

---

Nitpick comments:
In `@gts-macros/src/lib.rs`:
- Around line 2302-2306: Add direct negative test coverage for gts_id! by
exercising the proc-macro entry point in gts_id and the parsing path in
id_arg::build_prefixed_lit. Add a compile-fail case for malformed standalone
input like gts_id!(123), and a second case for invalid nested usage like type_id
= gts_id!(123), so both the top-level macro input handling and the bad-body
diagnostics stay covered.

In `@gts-validator/src/normalize.rs`:
- Around line 46-60: The tests in this module still hardcode the default gts.
prefix, so the custom-prefix path in normalize_candidate is not being exercised.
Update the tests around normalize_candidate to reference GTS_ID_PREFIX and
GTS_ID_URI_PREFIX instead of literal gts. strings, and add coverage that
verifies normalization and rejection behavior when the build is configured with
a non-default prefix. Keep the assertions aligned with the same helper names so
the prefix-driven logic in normalize_candidate is covered in both plain and URI
forms.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e9139331-41c1-4b40-bbff-e2da11b562ba

📥 Commits

Reviewing files that changed from the base of the PR and between 54b4091 and 1229d16.

📒 Files selected for processing (34)
  • README.md
  • gts-cli/src/gen_schemas.rs
  • gts-id/README.md
  • gts-id/build.rs
  • gts-id/src/gts_id.rs
  • gts-id/src/gts_id_segment.rs
  • gts-id/src/lib.rs
  • gts-id/src/parse.rs
  • gts-id/src/prefix.rs
  • gts-macros/README.md
  • gts-macros/src/id_arg.rs
  • gts-macros/src/instance.rs
  • gts-macros/src/lib.rs
  • gts-macros/tests/compile_fail/instance_id_not_literal.stderr
  • gts-macros/tests/compile_fail/instance_raw_id_not_literal.stderr
  • gts-macros/tests/golden/traits_bool_false.rs
  • gts-macros/tests/golden/traits_bool_true.rs
  • gts-macros/tests/golden/traits_generic_child.rs
  • gts-macros/tests/golden/traits_inline_chain.rs
  • gts-macros/tests/golden/traits_referenced_chain.rs
  • gts-macros/tests/golden/traits_schema_narrowing.rs
  • gts-macros/tests/golden/traits_struct_literal.rs
  • gts-macros/tests/instance_macro_tests.rs
  • gts-validator/src/format/json.rs
  • gts-validator/src/format/markdown.rs
  • gts-validator/src/normalize.rs
  • gts-validator/src/output.rs
  • gts/src/entities.rs
  • gts/src/gts.rs
  • gts/src/lib.rs
  • gts/src/schema_refs.rs
  • gts/src/schema_resolver.rs
  • gts/src/schema_traits.rs
  • gts/src/x_gts_ref.rs

Comment thread gts-id/src/parse.rs
Comment thread gts-id/src/prefix.rs
Comment thread gts-macros/README.md
@aviator5 aviator5 force-pushed the custom-gts-id-prefix branch from 1229d16 to e722627 Compare June 26, 2026 13:52

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
gts-id/src/gts_id.rs (1)

550-555: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Make the UUID snapshot conditional on the default prefix.

Line 550 now feeds to_uuid() a prefix-configured ID, but Line 555 still asserts the UUID for the default gts. input. Any non-default GTS_ID_PREFIX will fail this test even if to_uuid() is correct.

Proposed fix
     let uuid1 = id.to_uuid();
     let uuid2 = id.to_uuid();
     // UUIDs should be deterministic
     assert_eq!(uuid1, uuid2);
-    assert_eq!(uuid1.to_string(), "154302ad-df5c-56e6-97d4-f87c5faca44b");
+    if crate::GTS_ID_PREFIX == crate::DEFAULT_GTS_ID_PREFIX {
+        assert_eq!(uuid1.to_string(), "154302ad-df5c-56e6-97d4-f87c5faca44b");
+    }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@gts-id/src/gts_id.rs` around lines 550 - 555, The UUID snapshot test in
GtsId::to_uuid is still hardcoded to the default gts. value even though the test
now builds the ID with gts_id("x.core.events.event.v1~"), so it will fail under
non-default GTS_ID_PREFIX settings. Update the test to derive the expected UUID
from the actual configured prefix or only assert the fixed snapshot when the
default prefix is in use, keeping the determinism check for uuid1 and uuid2
intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@gts-id/src/gts_id.rs`:
- Around line 550-555: The UUID snapshot test in GtsId::to_uuid is still
hardcoded to the default gts. value even though the test now builds the ID with
gts_id("x.core.events.event.v1~"), so it will fail under non-default
GTS_ID_PREFIX settings. Update the test to derive the expected UUID from the
actual configured prefix or only assert the fixed snapshot when the default
prefix is in use, keeping the determinism check for uuid1 and uuid2 intact.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4c93ba0c-3158-4b0b-9319-3bb0ce69a52e

📥 Commits

Reviewing files that changed from the base of the PR and between 1229d16 and e722627.

📒 Files selected for processing (35)
  • README.md
  • gts-cli/src/gen_schemas.rs
  • gts-id/README.md
  • gts-id/build.rs
  • gts-id/src/gts_id.rs
  • gts-id/src/gts_id_pattern.rs
  • gts-id/src/gts_id_segment.rs
  • gts-id/src/lib.rs
  • gts-id/src/parse.rs
  • gts-id/src/prefix.rs
  • gts-macros/README.md
  • gts-macros/src/id_arg.rs
  • gts-macros/src/instance.rs
  • gts-macros/src/lib.rs
  • gts-macros/tests/compile_fail/instance_id_not_literal.stderr
  • gts-macros/tests/compile_fail/instance_raw_id_not_literal.stderr
  • gts-macros/tests/golden/traits_bool_false.rs
  • gts-macros/tests/golden/traits_bool_true.rs
  • gts-macros/tests/golden/traits_generic_child.rs
  • gts-macros/tests/golden/traits_inline_chain.rs
  • gts-macros/tests/golden/traits_referenced_chain.rs
  • gts-macros/tests/golden/traits_schema_narrowing.rs
  • gts-macros/tests/golden/traits_struct_literal.rs
  • gts-macros/tests/instance_macro_tests.rs
  • gts-validator/src/format/json.rs
  • gts-validator/src/format/markdown.rs
  • gts-validator/src/normalize.rs
  • gts-validator/src/output.rs
  • gts/src/entities.rs
  • gts/src/gts.rs
  • gts/src/lib.rs
  • gts/src/schema_refs.rs
  • gts/src/schema_resolver.rs
  • gts/src/schema_traits.rs
  • gts/src/x_gts_ref.rs
✅ Files skipped from review due to trivial changes (11)
  • gts-macros/tests/compile_fail/instance_raw_id_not_literal.stderr
  • gts/src/schema_traits.rs
  • README.md
  • gts-macros/tests/golden/traits_referenced_chain.rs
  • gts-macros/tests/golden/traits_inline_chain.rs
  • gts-macros/tests/golden/traits_schema_narrowing.rs
  • gts/src/schema_refs.rs
  • gts-id/README.md
  • gts-macros/tests/golden/traits_bool_true.rs
  • gts-macros/tests/compile_fail/instance_id_not_literal.stderr
  • gts-macros/README.md
🚧 Files skipped from review as they are similar to previous changes (21)
  • gts-macros/tests/golden/traits_struct_literal.rs
  • gts-id/build.rs
  • gts/src/schema_resolver.rs
  • gts/src/lib.rs
  • gts-macros/tests/golden/traits_generic_child.rs
  • gts-macros/src/id_arg.rs
  • gts-validator/src/output.rs
  • gts-id/src/prefix.rs
  • gts-validator/src/format/markdown.rs
  • gts-validator/src/normalize.rs
  • gts-id/src/gts_id_segment.rs
  • gts/src/entities.rs
  • gts/src/x_gts_ref.rs
  • gts-macros/tests/golden/traits_bool_false.rs
  • gts-id/src/lib.rs
  • gts-id/src/parse.rs
  • gts-macros/src/instance.rs
  • gts-validator/src/format/json.rs
  • gts-macros/src/lib.rs
  • gts-cli/src/gen_schemas.rs
  • gts-macros/tests/instance_macro_tests.rs

@aviator5 aviator5 force-pushed the custom-gts-id-prefix branch 2 times, most recently from 8b76709 to e722627 Compare June 29, 2026 16:15
@Artifizer

Copy link
Copy Markdown
Contributor

[high] Configurable-prefix support is still incomplete for chain-derived IDs.

The PR updates parsing/validation to honor GTS_ID_PREFIX, but some production paths still reconstruct IDs with a hard-coded "gts." prefix. That means custom-prefix builds can parse IDs successfully and still fail later during parent/type resolution or chained schema lookup.

Examples I found:

  • gts/src/entities.rs: let parent_id = format!("gts.{}", parent_segments.join("~"));
  • gts/src/store.rs: chain-derived base_id / derived_id / schema_id are still built with format!("gts.{}" ...)

Why this matters:
With GTS_ID_PREFIX=acme., these paths still synthesize gts.... IDs, which can cause failed lookups, incorrect parent/type resolution, and broken chain validation.

Suggested fix:
Replace these reconstructed "gts." literals with GTS_ID_PREFIX everywhere chain-derived IDs are built.

- Add compile-time GTS_ID_PREFIX configuration with validation and Cargo rebuild tracking.
- Thread the configured prefix through ID parsing, schema generation, validation, and schema reference handling.
- Add macro argument support and tests for prefix-aware ID construction.

Signed-off-by: Aviator 5 <ai.agent.tor@gmail.com>
@aviator5 aviator5 force-pushed the custom-gts-id-prefix branch from e722627 to 522a8ee Compare June 29, 2026 17:55

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
gts-macros/tests/inheritance_tests.rs (1)

400-419: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Finish the schema-URI migration in this suite.

A lot of the raw ID expectations were updated to gts_id!(...), but this file still has multiple $id/$ref assertions hard-coded to "gts://gts...." (for example Line 664, Line 707, Line 718, Line 1570, Line 1778, and Line 2580). That keeps the inheritance tests tied to the default prefix even when the configured-prefix run is enabled.

🔧 Suggested fix
-        assert_eq!(topic_schema["$id"], "gts://gts.x.core.events.topic.v1~");
+        assert_eq!(
+            topic_schema["$id"],
+            format!("gts://{}", gts_id!("x.core.events.topic.v1~"))
+        );

-        assert_eq!(first["$ref"], "gts://gts.x.core.events.topic.v1~");
+        assert_eq!(
+            first["$ref"],
+            format!("gts://{}", gts_id!("x.core.events.topic.v1~"))
+        );

Based on learnings, make check includes test-gts-id-prefix; the PR objective says schema generation and schema reference handling should honor the configured prefix.

Also applies to: 2548-2551, 2697-2701, 2715-2717

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@gts-macros/tests/inheritance_tests.rs` around lines 400 - 419, Finish the
schema-URI migration in the inheritance tests by replacing the remaining
hard-coded "gts://gts...." $id/$ref expectations with prefix-aware assertions
using gts_id!(...) or equivalent configured-prefix values. Update the affected
checks in the test suite around test_schema_inheritance and the other referenced
assertions so schema generation and reference handling validate against the
active prefix instead of the default URI.

Source: Learnings

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/ci.yml:
- Around line 241-242: The Checkout step currently leaves the default token
persisted in git config, which broadens exposure after the repository code is
checked out. Update the actions/checkout usage in the Checkout job step to
explicitly set persist-credentials to false so the token is not retained for
subsequent build scripts or proc macros.

In `@gts-dylint/src/lib.rs`:
- Around line 42-48: The default prefix list in configured_prefixes() only
includes gts., so gts:// literals are not caught when GTS_DYLINT_PREFIXES is
unset. Update configured_prefixes() to include gts:// in the default Vec, or
ensure the entrypoints that invoke cargo dylint --all explicitly export both
prefixes. Use the existing configured_prefixes() symbol as the main place to
make the default behavior cover both schema URI forms.
- Line 47: The default branch in the prefix list builder currently hard-codes
the forbidden "gts." literal; keep the behavior but make the exception explicit
by adding the documented narrow suppression on the relevant item in
gts-dylint/src/lib.rs. Locate the match arm in the prefix-building logic and
apply #[allow(unknown_lints, gts_id_hardcoded_prefix)] only where this
intentional fallback is needed, so the hard-coded prefix is clearly exempted
rather than silently reintroduced.

In `@gts-macros/tests/integration_tests.rs`:
- Line 995: The $id assertions in the integration tests are still hardcoded to
the default gts:// prefix even though the schemas are registered with
gts_id!(...), so update the affected assertions to derive the expected $id using
the configured prefix instead of comparing against literal gts://gts... values.
Use the same prefix-aware helper or construction already used around
register_schema and the runtime-resolution checks in
gts-macros/tests/integration_tests.rs so the test coverage validates
configurable-prefix behavior consistently.

In `@Makefile`:
- Around line 44-50: The alternate-prefix coverage in test-gts-id-prefix is too
narrow because it only rebuilds and tests gts-id, so prefix-aware changes in
other touched crates can still slip through. Update the Makefile target to run
the relevant prefix-consuming crates from this PR under GTS_ID_PREFIX=acme., or
widen it to a workspace subset that exercises all affected code paths, so make
check actually validates the changed prefix behavior beyond gts-id.

---

Outside diff comments:
In `@gts-macros/tests/inheritance_tests.rs`:
- Around line 400-419: Finish the schema-URI migration in the inheritance tests
by replacing the remaining hard-coded "gts://gts...." $id/$ref expectations with
prefix-aware assertions using gts_id!(...) or equivalent configured-prefix
values. Update the affected checks in the test suite around
test_schema_inheritance and the other referenced assertions so schema generation
and reference handling validate against the active prefix instead of the default
URI.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 61530301-149a-4386-ae40-d0b5ec636cb9

📥 Commits

Reviewing files that changed from the base of the PR and between 2aadb05 and 5a33081.

⛔ Files ignored due to path filters (1)
  • gts-dylint/Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (66)
  • .github/workflows/ci.yml
  • .github/workflows/release.yml
  • CLAUDE.md
  • Cargo.toml
  • Makefile
  • README.md
  • gts-cli/src/gen_schemas.rs
  • gts-dylint/.cargo/config.toml
  • gts-dylint/Cargo.toml
  • gts-dylint/README.md
  • gts-dylint/rust-toolchain.toml
  • gts-dylint/src/lib.rs
  • gts-dylint/ui/no_trigger_non_gts.rs
  • gts-dylint/ui/no_trigger_with_allow.rs
  • gts-dylint/ui/trigger_gts_prefix.rs
  • gts-dylint/ui/trigger_gts_prefix.stderr
  • gts-id/README.md
  • gts-id/build.rs
  • gts-id/src/gts_id.rs
  • gts-id/src/gts_id_pattern.rs
  • gts-id/src/gts_id_segment.rs
  • gts-id/src/lib.rs
  • gts-id/src/parse.rs
  • gts-id/src/prefix.rs
  • gts-macros-cli/src/main.rs
  • gts-macros/README.md
  • gts-macros/src/id_arg.rs
  • gts-macros/src/instance.rs
  • gts-macros/src/lib.rs
  • gts-macros/tests/compile_fail/gts_id_invalid_pattern.rs
  • gts-macros/tests/compile_fail/gts_id_invalid_pattern.stderr
  • gts-macros/tests/compile_fail/gts_id_missing_prefix.rs
  • gts-macros/tests/compile_fail/gts_id_missing_prefix.stderr
  • gts-macros/tests/compile_fail/instance_id_not_literal.stderr
  • gts-macros/tests/compile_fail/instance_raw_id_not_literal.stderr
  • gts-macros/tests/deprecated_schema_id_alias.rs
  • gts-macros/tests/golden/traits_bool_false.rs
  • gts-macros/tests/golden/traits_bool_true.rs
  • gts-macros/tests/golden/traits_generic_child.rs
  • gts-macros/tests/golden/traits_inline_chain.rs
  • gts-macros/tests/golden/traits_referenced_chain.rs
  • gts-macros/tests/golden/traits_schema_narrowing.rs
  • gts-macros/tests/golden/traits_struct_literal.rs
  • gts-macros/tests/inheritance_tests.rs
  • gts-macros/tests/inheritance_tests_mixed.rs
  • gts-macros/tests/instance_macro_tests.rs
  • gts-macros/tests/integration_tests.rs
  • gts-macros/tests/serde_rename_tests.rs
  • gts-macros/tests/traits_tests.rs
  • gts-macros/tests/value_dispatch_tests.rs
  • gts-validator/src/format/json.rs
  • gts-validator/src/format/markdown.rs
  • gts-validator/src/normalize.rs
  • gts-validator/src/output.rs
  • gts/src/entities.rs
  • gts/src/gts.rs
  • gts/src/lib.rs
  • gts/src/ops.rs
  • gts/src/schema.rs
  • gts/src/schema_modifiers.rs
  • gts/src/schema_refs.rs
  • gts/src/schema_resolver.rs
  • gts/src/schema_traits.rs
  • gts/src/store.rs
  • gts/src/testing.rs
  • gts/src/x_gts_ref.rs
✅ Files skipped from review due to trivial changes (26)
  • gts-macros/tests/compile_fail/gts_id_missing_prefix.rs
  • gts-dylint/.cargo/config.toml
  • gts-dylint/rust-toolchain.toml
  • gts-macros/tests/compile_fail/gts_id_invalid_pattern.stderr
  • gts-macros/tests/compile_fail/gts_id_missing_prefix.stderr
  • gts-macros/tests/compile_fail/gts_id_invalid_pattern.rs
  • gts-dylint/ui/trigger_gts_prefix.stderr
  • gts-dylint/ui/no_trigger_non_gts.rs
  • gts-dylint/ui/trigger_gts_prefix.rs
  • gts/src/ops.rs
  • gts-dylint/Cargo.toml
  • gts-dylint/ui/no_trigger_with_allow.rs
  • gts-macros/tests/golden/traits_bool_false.rs
  • gts-macros/README.md
  • gts-validator/src/output.rs
  • gts-macros/tests/compile_fail/instance_raw_id_not_literal.stderr
  • gts/src/testing.rs
  • gts-id/README.md
  • gts-macros/tests/compile_fail/instance_id_not_literal.stderr
  • gts/src/schema_traits.rs
  • gts-macros/tests/golden/traits_bool_true.rs
  • gts/src/schema_modifiers.rs
  • gts-macros/tests/golden/traits_referenced_chain.rs
  • gts-macros-cli/src/main.rs
  • README.md
  • CLAUDE.md
🚧 Files skipped from review as they are similar to previous changes (25)
  • gts-macros/tests/golden/traits_generic_child.rs
  • gts/src/lib.rs
  • gts-id/build.rs
  • gts-id/src/lib.rs
  • gts-validator/src/normalize.rs
  • gts-macros/tests/golden/traits_schema_narrowing.rs
  • gts-validator/src/format/markdown.rs
  • gts-id/src/prefix.rs
  • gts-macros/tests/golden/traits_inline_chain.rs
  • gts-macros/src/id_arg.rs
  • gts-validator/src/format/json.rs
  • gts/src/schema_resolver.rs
  • gts-id/src/gts_id_segment.rs
  • gts/src/entities.rs
  • gts-macros/tests/golden/traits_struct_literal.rs
  • gts/src/schema_refs.rs
  • gts-macros/src/instance.rs
  • gts/src/gts.rs
  • gts/src/x_gts_ref.rs
  • gts-macros/src/lib.rs
  • gts-id/src/gts_id.rs
  • gts-id/src/parse.rs
  • gts-cli/src/gen_schemas.rs
  • gts-macros/tests/instance_macro_tests.rs
  • gts-id/src/gts_id_pattern.rs

Comment thread .github/workflows/ci.yml
Comment thread gts-dylint/src/lib.rs
Comment thread gts-dylint/src/lib.rs Outdated
Comment thread gts-macros/tests/integration_tests.rs
Comment thread Makefile
@aviator5 aviator5 force-pushed the custom-gts-id-prefix branch 5 times, most recently from cbf68c2 to 64bf86d Compare June 30, 2026 12:05
- Add a gts-dylint crate that warns on hard-coded "gts." string literals.
- Wire the lint into workspace metadata, CI, release checks, and make check.
- Document lint usage and add targeted suppressions for intentional prefix constants.

Signed-off-by: Aviator 5 <ai.agent.tor@gmail.com>
@aviator5 aviator5 force-pushed the custom-gts-id-prefix branch from 64bf86d to 02a6752 Compare June 30, 2026 12:16
@aviator5

aviator5 commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

[high] Configurable-prefix support is still incomplete for chain-derived IDs.

The PR updates parsing/validation to honor GTS_ID_PREFIX, but some production paths still reconstruct IDs with a hard-coded "gts." prefix. That means custom-prefix builds can parse IDs successfully and still fail later during parent/type resolution or chained schema lookup.

Examples I found:

  • gts/src/entities.rs: let parent_id = format!("gts.{}", parent_segments.join("~"));
  • gts/src/store.rs: chain-derived base_id / derived_id / schema_id are still built with format!("gts.{}" ...)

Why this matters: With GTS_ID_PREFIX=acme., these paths still synthesize gts.... IDs, which can cause failed lookups, incorrect parent/type resolution, and broken chain validation.

Suggested fix: Replace these reconstructed "gts." literals with GTS_ID_PREFIX everywhere chain-derived IDs are built.

thanks, fixed. I implemented gts-dylint crate for checking hard-coded "gts." (configurable via env var) prefix that could be used in external projects as well. It has its own tests. Now dylint is used in CI. Additionally, I added test-gts-id-prefix Makefile target for running unit tests for gts-id crate with custom GTS_ID_PREFIX.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants