Skip to content

feat(node): add context-level default namespace inheritance for nodes#138

Merged
YuanYuYuan merged 7 commits intoZettaScaleLabs:mainfrom
schmidma:context-namespace
May 8, 2026
Merged

feat(node): add context-level default namespace inheritance for nodes#138
YuanYuYuan merged 7 commits intoZettaScaleLabs:mainfrom
schmidma:context-namespace

Conversation

@schmidma
Copy link
Copy Markdown
Contributor

@schmidma schmidma commented Mar 14, 2026

Description

This adds namespace inheritance at the context level so a ZContextBuilder can define a default namespace that is forwarded to nodes created from that context.

Nodes still keep their existing with_namespace(...) override behavior, so explicit node namespaces continue to take precedence.

Review Notes

This PR is intended to be reviewed independently against main.
The local integration branch next carries the combined stack used for downstream preview work.

Checklist

  • Ran ./scripts/check-local.sh successfully
  • Added or updated tests and documentation when applicable

@schmidma schmidma changed the title feat(node): Add context-level default namespace inheritance for nodes feat(node): add context-level default namespace inheritance for nodes Mar 14, 2026
@schmidma schmidma force-pushed the context-namespace branch from 70e05ef to 0ddedae Compare April 2, 2026 13:23
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 2, 2026

Codecov Report

❌ Patch coverage is 50.00000% with 11 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
crates/ros-z/src/context.rs 20.00% 8 Missing ⚠️
crates/ros-z/src/lifecycle/node.rs 0.00% 2 Missing ⚠️
crates/ros-z/src/node.rs 0.00% 1 Missing ⚠️
Files with missing lines Coverage Δ
crates/ros-z/src/entity.rs 63.46% <100.00%> (+3.88%) ⬆️
crates/ros-z/src/node.rs 73.68% <0.00%> (+0.54%) ⬆️
crates/ros-z/src/lifecycle/node.rs 0.00% <0.00%> (ø)
crates/ros-z/src/context.rs 50.39% <20.00%> (-0.61%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@schmidma schmidma force-pushed the context-namespace branch 2 times, most recently from a6ee505 to 0d2b3e2 Compare April 2, 2026 21:33
schmidma and others added 5 commits April 13, 2026 14:12
create_lifecycle_node was always setting namespace: None, silently
ignoring any context-level default set via ZContextBuilder::with_namespace.
Mirrors the same propagation logic used by create_node.
…mespace

Used Into<String> without normalization, unlike ZNodeBuilder and
ZContextBuilder which both use AsRef<str> + normalize_node_namespace.
Ensures consistent namespace formatting across all builder types.
Inserting a field in the middle of a #[repr(C)] struct shifts the memory
layout of all subsequent fields, breaking C/Go callers that zero-initialize
and assign by field name. New fields must be appended at the end.
Verify that lifecycle nodes inherit the context namespace by default,
that a node-level override takes precedence, and that no namespace
defaults to the root.
@YuanYuYuan
Copy link
Copy Markdown
Collaborator

YuanYuYuan commented Apr 13, 2026

Review fixes

The following issues were addressed and pushed to this branch:

Bug fix (High)

  • create_lifecycle_node now propagates the context-level namespace to lifecycle nodes, matching the behaviour of create_node. Previously namespace: None was always passed, silently ignoring any ZContextBuilder::with_namespace() setting.

ABI fix (Medium)

  • Moved the namespace field to the end of CContextConfig (#[repr(C)]). Inserting it in the middle shifted the layout of all subsequent fields, breaking C/Go callers that zero-initialize and assign by field name.

Consistency fix (Low)

  • ZLifecycleNodeBuilder::with_namespace changed from Into<String> (no normalization) to AsRef<str> + normalize_node_namespace, consistent with ZNodeBuilder and ZContextBuilder.

Tests (Medium)

  • Added 3 tests in ros-z-tests/tests/lifecycle.rs covering context namespace inheritance, node-level override precedence, and no-namespace defaulting to root.

Revised. Thanks @schmidma!

@YuanYuYuan YuanYuYuan marked this pull request as ready for review April 13, 2026 06:50
@YuanYuYuan YuanYuYuan self-requested a review April 13, 2026 06:51
YuanYuYuan added 2 commits May 8, 2026 15:03
- Replace is_empty() || == "/" dual check with assert_eq!("") since
  normalize_node_namespace always converts "/" to "" and context
  default is already empty
- Add three parallel tests for create_node context namespace inheritance
  (inherit, override precedence, no-namespace default) matching the
  existing lifecycle node coverage
@YuanYuYuan YuanYuYuan merged commit ad73cb3 into ZettaScaleLabs:main May 8, 2026
23 of 25 checks passed
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.

2 participants