WiP: .^create from belongs-to relationship by FCO · Pull Request #523 · FCO/Red

FCO · 2021-11-06T16:24:03Z

This is not working when using strings as referencing's model (but works if use the actual type)

…CO/Red into create-to-one

…/Red into create-to-one

…tributes

Copilot

Pull request overview

This PR attempts to make .^create work when invoked from a belongs-to (single) relationship where the referenced model is specified by string (e.g. :model<Bli>), by ensuring the relationship context (alias/join info + parent) is preserved so the FK can be propagated back to the owning row.

Changes:

Adds new test coverage for creating records via single/belongs-to relationships (including type-based relationship declarations).
Adjusts relationship/alias machinery so relationship selectors can carry a parent reference and accept an explicit alias in join conditions.
Updates relationship AST generation and reference resolution to better handle aliasing.

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
`t/35-create.t`	Adds new subtests for create-from-relationship scenarios; reorders an existing array-create subtest.
`lib/Red/PrepareCode.pm6`	Minor formatting/line-number-only change.
`lib/Red/Column.pm6`	Tweaks `ReferencesProxy.CALL-ME` to treat definite aliases differently when resolving references.
`lib/Red/Attr/Relationship.pm6`	Adjusts relationship AST generation to wrap RHS with `ast-value`.
`lib/MetamodelX/Red/Relationship.pm6`	Changes scalar relationship accessor behavior to try to return a join alias with `parent` set.
`lib/MetamodelX/Red/Model.pm6`	Modifies alias role to add `parent` and parameterize `join-on` with an alias argument.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

 schema(Bla, Ble).drop.create;

+# TODO: Figure out why this test can't be the last one
+subtest "Create on has-one", {


+    is $bla.bles.head.gist, $ble.gist;
+    is $ble.bla.gist, $bla.gist;


+    is $ble.bla.gist, $bla.gist;
+}
+
+subtest "belogs-to using types", {


+    is $bli.blos.head.gist, $blo.gist;
+    is $blo.bli.gist, $bli.gist;


    my role RAlias[Red::Model:U \rtype, Str $rname, \alias, \rel, \base, \join-type, @cols] {
-        method columns(|)     { @cols }
-        method table(|)       { rtype.^table }
-        method as(|)          { self.table-formatter: $rname }
-        method orig(|)        { rtype }
-        method join-type(|)   { join-type }
-        method tables(|)      { [ |base.^tables, alias ] }
-        method join-on(|)     {
+        method parent(|) is rw { $ }
+        method columns(|)      { @cols }


+                my \value = nqp::getattr(self, self.WHAT, $attr.name);
+                my $n = $self.WHAT."$name"();
+                $n.^parent = self if $n.HOW.^can: "parent";
+                return $n unless value;
+                value


hermes-fco · 2026-06-22T08:47:31Z

Code Review — PR #523 (WiP: .^create from belongs-to relationship)

⚠️ This PR has been superseded. PR #596 ("feat: .^create on belongs-to relationship with auto FK") explicitly states "Port and fix of PR #523 (from 2021)" and was already reviewed by Hermes Agent on 2026-06-22.

🔍 Minor observations

lib/MetamodelX/Red/Describable.pm6 — Typo fix column-formater → column-formatter. This is cosmetic but should be applied everywhere consistently (also in Model.pm6, Red.pm6).
lib/MetamodelX/Red/Dirtable.pm6:set-helper-attrs — Refactors %attr is Set to .map(-> $a { $a.name => $a }).Hash to detect already-composed attributes. This pattern (check if attr exists before adding) is generally correct and prevents double-registration, but many of these changes have landed in main through subsequent PRs.
lib/MetamodelX/Red/Id.pm6 — New id-hash method. Useful utility for FK management during create operations (used by feat: .^create on belongs-to relationship with auto FK #596).
DirtableWrapped role — Guards against double-wrapping TWEAK. Good defensive pattern.
clean-up-columns method — New method to decrement dirty counters. Not used anywhere visible in this PR; likely intended for the belongs-to create flow.

💡 Suggestion

Close this PR in favor of #596. If there are infrastructure changes here that weren't ported (the DirtableWrapped guard, clean-up-columns), consider opening a focused follow-up.

Reviewed by Hermes Agent

hermes-fco · 2026-06-22T11:00:04Z

Code Review — `WiP: .^create from belongs-to relationship`

Verdict: Comment (0 critical, 2 warnings, 3 suggestions)

PR from FCO (2021-11) adding has-one relationship support in .^create, type-checking on attribute STORE, model-type-based referencing, attribute reuse fixes, operator documentation, and Specialisable role. 237 files, +19,883 / −1,240 — but most of the diff is test/CI files added from main merges over 4 years. Core lib changes are focused: ~10 files with ~200 meaningful lines.

✅ Looks Good

Dirtable.rakumod — attribute reuse guard: Switching from my %attr is Set = type.^attributes>>.name to type.^attributes.map( -> $a { $a.name => $a }).Hash and checking for existing attrs before creating new ones prevents double-adding %!___COLUMNS_DATA___ etc. when compose is called multiple times. This is a real bug fix.
Dirtable.rakumod — type checking on STORE: Adding die X::TypeCheck::Assignment.new(...) unless value ~~ col.type in the Proxy STORE block catches type mismatches early. Good defensive improvement.
Dirtable.rakumod — TWEAK wrap guard: The DirtableWrapped role check prevents double-wrapping when compose runs multiple times. This avoids infinite recursion or stacked wrappers.
Red::Model.rakumod — alias caching: %!alias-cache with return %!alias-cache{$name} if %!alias-cache{$name}:exists avoids recreating alias types on repeated calls. Good performance optimization.
Red::Traits.rakumod — model-type referencing: Adding :model-type variants of referencing and relationship traits allows passing type objects (Mu:U $model) instead of string names. This enables compile-time model resolution.
Red::Operators.rakumod — documentation sweep: Adding #| POD comments to every operator multi candidate is great for tooling and discoverability.

⚠️ Warnings

Red::Model.rakumod — create method has grown 80+ lines with no tests for new paths: The new has-one relationship handling, join-model parent creation, and %!alias-cache path are substantial additions. The test files added (t/57-nm-relationship.t, t/58-tomgraceys-schema.t, etc.) test other features. Verify that has-one .^create is covered.
Dirtable.rakumod:94 — is hidden-from-backtrace on STORE: The is hidden-from-backtrace trait on the Proxy's STORE block hides the Proxy from stack traces, making type-check errors harder to debug when they occur. Consider using it only in production builds or adding a RED_DEBUG env toggle.

💡 Suggestions

Red::Schema.rakumod — schema() proto: The proto sub schema(|) + multi sub schema(*%models) addition for named-model schemas is nice, but the FALLBACK + model split could be simplified — FALLBACK just calls model(), so the indirection adds little value.
Red::ResultSeq.rakumod:73 — initialize $.chain with :filter(.^id-filter): Moving chain initialization to the attribute declaration (.= new: |(:filter(.^id-filter) with $!obj)) is clean but changes the initialization order subtly — has $!chain handles ... is now the third attribute instead of first. Verify that no code depends on chain being available before obj is set.
Red::Operators.rakumod — :bind-right / :bind-left: The new :bind-right and :bind-left variants for every operator are useful but add significant surface area (each operator goes from 1–2 candidates to 3–5). Consider generating these via a role or macro to reduce code duplication — the current approach is 400+ lines of near-identical candidates.
Documentation gap: The Red::Type role (4 lines: is-red-type, red-type-accepts) is added but never explained. What does red-type-accepts(Any:U --> Bool) gate? Add POD or a comment.

📊 Stats

Metric	Value
Files changed	237 (most from main merges)
Core lib files changed	~10
New lib features	has-one .^create, model-type referencing, attribute reuse fix, alias cache, Specialisable
New tests	12 test files
Security scan	Clean

Reviewed by Hermes Agent

hermes-fco · 2026-06-22T14:25:37Z

Code Review — WiP: .^create from belongs-to relationship

Verdict: Comment (0 critical, 0 warnings, 4 suggestions)

Early WiP implementing .^create on belongs-to relationships. This PR (#523, Nov 2021) was superseded by PR #596 (feat: .^create on belongs-to relationship with auto FK) which implements the same feature more completely and has already been reviewed. 6 files, +118/−28.

✅ Looks Good

ReferencesProxy.CALL-ME stringy-model fix (Column.pm6:142) — When $alias.DEFINITE, passes it directly to &!references instead of looking up the model first. This is the core fix for "not working with stringy model" mentioned in the PR body. Clean and correct.
join-on explicit alias parameter (Model.pm6) — Adding \a = alias to join-on and threading it through what-does-it-do fixes a closure capture issue where alias in nested scopes could refer to the wrong binding.
ast-value wrapper (Attr/Relationship.pm6:140) — Wrapping .ref: $type in ast-value() for relationship-ast. This sub exists in the .rakumod equivalents and is the correct translation layer.

💡 Suggestions

Superseded by PR feat: .^create on belongs-to relationship with auto FK #596 — This PR is an early iteration of the feature fully implemented in PR feat: .^create on belongs-to relationship with auto FK #596. Consider closing this PR in favor of feat: .^create on belongs-to relationship with auto FK #596 which has more test coverage, handles edge cases, and targets the current .rakumod file structure. If there are unique fixes here (like the ReferencesProxy.CALL-ME stringy-model path), they should be cherry-picked into feat: .^create on belongs-to relationship with auto FK #596.

Redundant die guard (Model.pm6:476) — die "Not possible call .^create on a defined model" if mo.defined is unreachable in the where *.DEFINITE candidate. This candidate is only selected when mo IS DEFINITE. Either the guard is dead code or the where clause should be relaxed.

# Current (contradictory):
multi method create(\mo where *.DEFINITE, ...) {
    die "Not possible call .^create on a defined model" if mo.defined;
    # ...
}
# Suggested — remove either the 'where' or the 'die':
multi method create(\mo, ...) {  # no 'where' — let the die handle dispatch
    die "..." if mo.defined;
    # ...
}

Test ordering dependency (t/35-create.t) — The TODO comment "Figure out why this test can't be the last one" indicates a test ordering issue. Combined with a subtest being moved from the middle to the end ("Simple create and creating by array"), this suggests state leakage between tests. Consider isolating each subtest with explicit schema drop/create.
.pm6 format obsolete — This PR targets .pm6 files that no longer exist on master. The repo migrated to .rakumod. Any revival of this feature should target the current file structure.

🔍 Minor observations

$!type attribute silently removed (Attr/Relationship.pm6:18) — #has Mu:U $!type; commented out without explanation. If unused, remove the line entirely instead of commenting out. If it had a purpose, document why it was removed.
3 TODO comments in production code (Model.pm6:475, test file ×2) — the "make it before creating transaction" hint is worth addressing.
%!alias-cache{$name} := alias; uses binding (:=) while the rest of the codebase uses assignment (=). Intentional (ensures identity) but worth a comment explaining why binding is needed here.

📊 Stats

Metric	Value
Files changed	6
Lines added	+118
Lines removed	−28
New tests	1 new subtest + 2 pending TODOs
Security scan	Clean
TODOs found	3

Reviewed by Hermes Agent

hermes-fco · 2026-06-23T02:39:25Z

Code Review — WiP: .^create from belongs-to relationship

Verdict: Comment (0 critical, 1 warning, 3 suggestions)

This PR adds .^create support for belongs-to relationships, the :has-one relationship trait, :model-type for passing types directly to relationships, transaction wrapping in create, and several quality-of-life improvements (typo fixes, inflator/deflator refactoring, alias caching). The code is well-structured and uses idiomatic Raku patterns throughout.

✅ Looks Good

lib/MetamodelX/Red/Model.pm6:444-465 — Transaction wrapping in .^create: proper KEEP/UNDO pattern ensures atomicity. Clean implementation.
lib/Red/Attr/Relationship.pm6:2-3 — :model-type addition solves the core problem described in the PR body (string vs type resolution for relationship models). Good design choice.
lib/Red/Column.pm6:6-31 — inflator/deflator refactoring: cleaner, handles Enumeration types explicitly, reduces error-prone logic.
lib/MetamodelX/Red/Relationship.pm6:160-168 — Relationship accessor enhancement: sets $n.^parent = self for the chaining pattern ($blo.bli.^create(...)). Elegant.
Red::Formater → Red::Formatter typo fix throughout — good catch.
%GLOBAL::RED-DEFULT-DRIVERS → %GLOBAL::RED-DEFAULT-DRIVERS — fixes another typo.

⚠️ Warnings

lib/MetamodelX/Red/Model.pm6:527 — Variable $no is cryptic. Based on context (.^find($filter)), this is the matched parent record. Rename to $existing-record or $found for clarity.
lib/MetamodelX/Red/Model.pm6:458 — Variable name typo: $*RED-TRANSCTION-RUNNING (missing "A"). Should be $*RED-TRANSACTION-RUNNING. If this variable is used elsewhere, this will create two separate variables.
PR is WiP with known limitations — The PR body notes it doesn't fully work with string-based model references, and the commented-out test at t/35-create.t:80-100 shows a known unsupported case. Some TODO comments remain in the codebase.

💡 Suggestions

lib/Red/Column.pm6:161 — The ref method now defaults to $!model-type when not Red::Model. This is a behavioral change — the old code returned Nil when no model was passed. Verify this doesn't break callers that check for Nil return from ref.
lib/MetamodelX/Red/Model.pm6:123-130 — The new method's experimental role mixing is creative but may have unexpected side effects. Consider documenting the intent and ensuring all experimental roles are opt-in.
Superseded by PR feat: .^create on belongs-to relationship with auto FK #596 — This PR (2021) has been superseded by PR feat: .^create on belongs-to relationship with auto FK #596 (2026) which implements .^create on belongs-to relationship with auto FK more comprehensively. If PR feat: .^create on belongs-to relationship with auto FK #596 covers all the use cases here, this PR can be closed in favor of it.

🔍 Minor observations (non-blocking)

lib/Red/PrepareCode.pm6 diff is just a newline-at-EOF fix — noise in the diff.
The what-does-it-do function name is pre-existing and not part of this PR, but worth noting as a candidate for renaming in a future cleanup.
Some left-internally #TODO comments reference known issues (multi-column FK handling) that PR feat: .^create on belongs-to relationship with auto FK #596 may address.

📊 Stats

Metric	Value
Files changed	6
Lines added	+118
Lines removed	−28
New tests	3 subtests in t/35-create.t
Security scan	Clean

Reviewed by Hermes Agent

hermes-fco · 2026-06-23T04:31:26Z

🤖 Hermes Code Review — PR #523

Summary: FCO's WiP: .^create from belongs-to relationships. 6 files, +118/−28 lines. Core feature: when calling .^create on a belongs-to accessor, automatically sets up the FK relationship on the parent.

✅ Looks Good

Architecture — solid metaclass approach: Using mo.^orig to resolve the original model from the alias, then setting up parent attributes via the HOW's join-on + parent chain. Clean use of Red's existing alias and relationship infrastructure.
Relationship.pm6 — accessor enhancement: The $n.^parent = self assignment in the add-relationship generated accessor is the key wiring that makes the believes-to .^create flow possible — the relationship descriptor carries the parent reference, which the create method uses to set FK attributes.
Column.pm6 — ReferencesProxy.CALL-ME fix: The if $alias.DEFINITE guard correctly handles the case where the referencing model is passed as a type object vs a string. Fixes the "not working with stringy model" issue mentioned in the PR body.
Attr/Relationship.pm6 — ast-value wrapping: The .ref: $type → ast-value .ref: $type change is correct — ensures the column reference is wrapped in the appropriate AST value node for expression building.
Test coverage: 2 new subtests cover the happy path for both has-one and believes-to .^create. Tests use schema(Bli, Blo).create explicitly (correct ordering for FK dependencies).

⚠️ Warnings

die guard timing (line 476): die "Not possible call .^create on a defined model" if mo.defined is annotated # TODO: make it before creating transaction — the check currently happens inside the transaction block, meaning the transaction is already started when the error is thrown. Move the guard before $RED-DB .= begin (line 463) to avoid an unnecessary rollback.
Test ordering fragility (t/35-create.t): The "Simple create and creating by array" subtest was moved to end of file, with a TODO comment: "Figure out why this test can't be the last one." This implies test isolation issues — state leakage between subtests (likely $*RED-DB or schema state). Worth investigating before merging.

💡 Suggestions

Commented test (belogs-to using types not using it on attrs): The commented-out subtest (line ~56) tests .^create when relationship attributes are untyped (has $.blb, not has Blb $.blb). If this is a known limitation, add a skip-based subtest with todo instead of commenting it out — makes the limitation visible in test reports.
Relationship.pm6 — $n.HOW.^can: "parent": The .^can check is defensive, but parent is set on the alias by the alias method in Model.pm6. If the alias always has parent when used in a believes-to context, the check could be simplified (or made a compile-time assertion). Minor — conservative is fine for WiP.
#has Mu:U $!type; (Attr/Relationship.pm6:18): Commented-out attribute with a # prefix. If this attribute is genuinely dead code, remove it rather than commenting. If it's temporarily disabled for experimentation, consider adding a brief comment explaining why.

🔍 Minor Observations

PrepareCode.pm6: Trailing newline addition — good hygiene.
Model.pm6 alias role changes: The join-on parameter change from alias to $a (default alias) is a refactor that makes the join-on logic work with the new parent passing — $a is either the alias (default) or the explicit parent passed through the relationship chain.
PR body says "not working when using strings as referencing's model (but works if use the actual type)" — the Column.pm6 fix addresses this limitation.
PR is from 2021 — the subsequent PR feat: .^create on belongs-to relationship with auto FK #596 (2026) appears to build on this work. No stale issues detected (FCO maintainer PR, never expires).

Reviewed by Hermes Agent — 2026-06-23

FCO added 2 commits November 6, 2021 16:11

belongs-to .^create (not working with stringy model for referencing)

3ed6576

Merge branch 'master' of github.com:FCO/Red into create-to-one

832643a

FCO mentioned this pull request Nov 6, 2021

Create a way for has-one relationship use .create (created after #472) #475

Open

FCO added 2 commits November 7, 2021 01:07

index on create-to-one: 832643a Merge branch 'master' of github.com:F…

97d118d

…CO/Red into create-to-one

WIP on create-to-one: 832643a Merge branch 'master' of github.com:FCO…

8d2e3fd

…/Red into create-to-one

FCO force-pushed the create-to-one branch from 832643a to 37adcd6 Compare November 7, 2021 01:19

Fix string model but something is requiring test order and to type at…

2a4e086

…tributes

FCO force-pushed the create-to-one branch from 37adcd6 to 2a4e086 Compare November 7, 2021 02:40

FCO requested a review from Copilot June 11, 2026 14:29

Copilot started reviewing on behalf of FCO June 11, 2026 14:29 View session

Copilot AI reviewed Jun 11, 2026

View reviewed changes

hermes-fco mentioned this pull request Jun 11, 2026

feat: .^create on belongs-to relationship with auto FK #596

Open

hermes-fco mentioned this pull request Jun 22, 2026

WiP: first test to try to resolve #507 #508

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

WiP: .^create from belongs-to relationship#523

WiP: .^create from belongs-to relationship#523
FCO wants to merge 5 commits into
masterfrom
create-to-one

FCO commented Nov 6, 2021

Uh oh!

Copilot AI left a comment

Uh oh!

hermes-fco commented Jun 22, 2026

Uh oh!

hermes-fco commented Jun 22, 2026

Uh oh!

hermes-fco commented Jun 22, 2026

Uh oh!

hermes-fco commented Jun 23, 2026

Uh oh!

hermes-fco commented Jun 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		is $bla.bles.head.gist, $ble.gist;
		is $ble.bla.gist, $bla.gist;

		is $bli.blos.head.gist, $blo.gist;
		is $blo.bli.gist, $bli.gist;

Uh oh!

Conversation

FCO commented Nov 6, 2021

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

hermes-fco commented Jun 22, 2026

Code Review — PR #523 (WiP: .^create from belongs-to relationship)

🔍 Minor observations

💡 Suggestion

Uh oh!

hermes-fco commented Jun 22, 2026

Code Review — WiP: .^create from belongs-to relationship

✅ Looks Good

⚠️ Warnings

💡 Suggestions

📊 Stats

Uh oh!

hermes-fco commented Jun 22, 2026

Code Review — WiP: .^create from belongs-to relationship

✅ Looks Good

💡 Suggestions

🔍 Minor observations

📊 Stats

Uh oh!

hermes-fco commented Jun 23, 2026

Code Review — WiP: .^create from belongs-to relationship

✅ Looks Good

⚠️ Warnings

💡 Suggestions

🔍 Minor observations (non-blocking)

📊 Stats

Uh oh!

hermes-fco commented Jun 23, 2026

🤖 Hermes Code Review — PR #523

✅ Looks Good

⚠️ Warnings

💡 Suggestions

🔍 Minor Observations

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Code Review — `WiP: .^create from belongs-to relationship`