Add X/Y margin to ON relation & access by intent compiler

[xyao-nv]

Summary

Short description of the change (max 50 chars)

Detailed description

• Why: An object placed On a surface could solve to a pose on the rim, causing it topple off the edge. There was no way to keep its footprint a safe distance inside the parent surface.
• What:

1. On has an edge_margin_m parameter (default 0.0) that insets the valid X/Y footprint band by the margin, so the whole footprint stays at least that far from every rim.
2. A margin too large for the parent surface is now rejected in validation
3. IntentCompiler.compile() injects a default edge_margin_m (0.05 m) on every on relation built from an intent spec

• Impact: Agent-generated scenes keep objects clear of surface edges by default.

Results

With agentic env gen pipeline

Before

After

[greptile-apps]

Greptile Summary

This PR adds an edge_margin_m parameter to the On relation that insets the valid X/Y placement band, keeping an object's footprint a safe distance from every rim of the support surface. The intent compiler is simplified in parallel: the old per-call margin injection is removed, INITIAL_STATE_SPEC_ID is extracted to a shared default_params.py, and dict(rel.params) is used to avoid mutating the caller's spec.

• On relation gains edge_margin_m (default DEFAULT_ON_EDGE_MARGIN_M = 0.05 m) with a non-negativity assert; both the loss strategy and the placement validator are updated consistently to enforce the inset band.
• _validate_on_relations adds a two-step check: (1) early rejection with a helpful verbose hint when the margin is too large for the surface; (2) strict XY containment against the inset bounds.
• IntentCompiler no longer special-cases on relations — the 5 cm safety margin is now the On class's own default, so both programmatic and compiler-generated placements behave consistently.

Confidence Score: 5/5

Safe to merge. The margin logic is correct end-to-end across the loss strategy, validator, and relation class, and all previously-reported issues are resolved.

The core placement math is consistent between OnLossStrategy and _validate_on_relations. Every boundary case (inverted band when child >= parent, infeasible margin, exact inset boundary) is covered by new tests with verified numerical assertions. No correctness defects were found; the two flagged items are diagnostic-quality issues in the verbose output path only.

No files require special attention, though the verbose diagnostic path in object_placer.py (lines 558-569) could be improved to print the margin hint even when child >= parent on one axis.

Important Files Changed

Filename	Overview
isaaclab_arena/relations/relations.py	Adds edge_margin_m param (default DEFAULT_ON_EDGE_MARGIN_M = 0.05) to On.init; includes a non-negativity assert and docstring update. Clean change.
isaaclab_arena/relations/relation_loss_strategies.py	Insets valid X/Y band by relation.edge_margin_m in OnLossStrategy.compute_loss; correctly documents the inverted-band constant-loss behaviour when margin or child is too large.
isaaclab_arena/relations/object_placer.py	Adds two-step margin validation in _validate_on_relations. Diagnostic hint is silently suppressed when child >= parent on either axis (max_feasible_margin clamps to 0.0); correctness is preserved but verbose output is less informative in mixed-freespace cases.
isaaclab_arena/agentic_environment_generation/intent_compiler.py	Extracts _INITIAL_STATE_SPEC_ID to default_params.py, uses dict(rel.params) copy to avoid caller mutation, and removes earlier margin-injection logic. All previously-reported issues resolved.
isaaclab_arena/agentic_environment_generation/default_params.py	New file extracting INITIAL_STATE_SPEC_ID constant from the compiler. Straightforward refactor.
isaaclab_arena/tests/test_validate_placement.py	Adds three focused edge-margin tests (within band, in margin gap, too large); updates existing rotated-bbox test to explicit edge_margin_m=0.0. Tests cover boundary conditions correctly.
isaaclab_arena/tests/test_relation_loss_strategies.py	Adds three new tests for the margin-inset loss: exact-inset boundary, oversized-child constant plateau, and inverted-band high-loss. Numerical assertions are consistent with the loss formula.
isaaclab_arena/tests/test_intent_compiler.py	Adds assertion that constraint.params == {} for an on relation; correctly documents that the compiler no longer injects edge margin.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[_validate_on_relations] --> B{edge_margin_m > 0?}
    B -- No --> E
    B -- Yes --> C{freespace lt 2xm on any XY axis?}
    C -- No --> E[Step 2: XY inset check]
    C -- Yes --> D{max_feasible_margin > 0?}
    D -- Yes --> F[verbose hint + return False]
    D -- No, child ge parent --> E
    E -- Fails --> H[verbose: XY outside parent + return False]
    E -- Passes --> G[Step 3: Z band check]
    G -- Fails --> J[verbose: Z outside band + return False]
    G -- Passes --> I[return True]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[_validate_on_relations] --> B{edge_margin_m > 0?}
    B -- No --> E
    B -- Yes --> C{freespace lt 2xm on any XY axis?}
    C -- No --> E[Step 2: XY inset check]
    C -- Yes --> D{max_feasible_margin > 0?}
    D -- Yes --> F[verbose hint + return False]
    D -- No, child ge parent --> E
    E -- Fails --> H[verbose: XY outside parent + return False]
    E -- Passes --> G[Step 3: Z band check]
    G -- Fails --> J[verbose: Z outside band + return False]
    G -- Passes --> I[return True]

Loading

_{Reviews (5): Last reviewed commit: "address comments" | Re-trigger Greptile}

[zhx06]

LGTM

[alexmillane]

LGTM

A few nits but my main suggestion is to just make this margin a default then we can get rid of the complexity resulting from needing to thread the parameter around.