Improve macro compile-time diagnostics and key resolution validation

[adiman9]

Summary

• tighten the hyperstack-macros validation pipeline so attribute parsing preserves source spans, validates arguments eagerly, and reports actionable compile-time errors instead of opaque expansion failures
• add IDL-aware validation and diagnostic collection for malformed #[map], #[resolve], #[aggregate], #[event], #[view], and top-level macro arguments, plus resolver and computed-field validation paths
• enforce handler key-resolution rules during macro expansion for account sources, instruction handlers, event handlers, and #[derive_from] hooks so entities must prove a path back to their primary key
• update the interpreter compiler to treat lookup indexes ending in _address as aliases for the underlying lookup field when resolving handlers
• expand regression coverage with dynamic compile-failure tests and UI fixtures for the new diagnostics, including dedicated coverage for key-resolution validation failures

What Changed

• introduced shared diagnostic plumbing in hyperstack-macros to accumulate and surface multiple validation errors with source-aware spans
• refactored attribute parsing to validate strategy values, conditions, field paths, resolver conditions, URL templates, and resolver hook declarations as part of macro expansion
• added validation passes for IDL references, computed cycles, invalid view sort fields, malformed resolver configuration, missing proto inputs, and unsupported top-level arguments
• added a dedicated key-resolution validation pass that checks whether each handler can resolve the entity primary key via explicit primary-key mappings, lookup indexes, joins, lookup_by, or resolver hooks
• refreshed the generated Ore SDK artifacts and kept the follow-up runtime formatting changes that came from touching related code paths during this work

Commit Breakdown

• fix: surface hyperstack macro validation failures during expansion
• chore: refresh ore generated stack artifacts
• chore: format touched runtime crates
• fix: validate handler key resolution paths during macro expansion

Reviewer Notes

• the substantive review is in bb6d515 and 8f23f4c; the other two commits are generated artifact refreshes and formatting follow-up
• this branch was rewritten to remove accidentally committed planning docs and IDL fixtures, so the PR now reflects only the compiler and runtime changes still intended for merge
• reviewing commit-by-commit is likely the easiest path because the validation work touches parser, entity processing, codegen-adjacent plumbing, and test fixtures at once

Testing

• added new dynamic and UI regression coverage in hyperstack-macros/tests/

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
hyperstack-docs	Ready	Preview, Comment	Mar 25, 2026 1:17am

[greptile-apps]

adiman9 has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.

[greptile-apps]

Greptile Summary

This PR introduces a substantial compile-time validation pipeline for hyperstack-macros, adding span-preserving attribute parsing, an ErrorCollector-based multi-error accumulation system, IDL-aware field-reference validation, and a dedicated key-resolution validation pass. It also adds parse_resolver_condition_expression with proper operator parsing, fixes parse_url_template to return syn::Result, and updates the interpreter compiler with a one-directional _address alias for lookup index resolution.

Key observations:

• Accidentally included planning document — docs/plans/entropy-capture-implementation.md (1,252 lines, Status: Planning) is added despite the PR description stating that planning docs were removed in the rewrite. This file should be dropped before merging.
• Contradictory is_event_source comments — entity.rs now correctly documents is_event_source: false as intentional, but validation/mod.rs (line 159) simultaneously claims event-derived MapAttributes "have is_event_source=true". The dead guard if mappings.iter().all(|mapping| mapping.is_event_source) { continue; } (line ~545) has never been reachable and the two new comments directly conflict with each other.
• Many issues raised in prior review rounds have been addressed: idls.first() fallback removed, Box::leak eliminated, syn::Ident::new panic risks guarded by try_field_spec_from_leaf, null literal normalised in both condition parsers, single-quoted string stripping added to resolver conditions, find_top_level_operator used for logical-operator detection, aggregate-condition key mismatch fixed via split_once('.'), and validate_views now handles duplicate IDs with a continue to prevent cascading noise.
• Remaining open threads from prior rounds (nondeterministic error ordering in some HashMap-backed loops, __account_address bypass gap in event handler keys, capture_fields not scanned for key resolution in event handlers) are partially addressed but not fully closed.

Confidence Score: 3/5

• Not safe to merge as-is: an unintended planning document is present, and contradictory is_event_source comments leave the codebase in a misleading state; the substantive validation logic is largely sound but carries forward several open issues from prior rounds.
• The validation pipeline itself is a meaningful improvement and most of the critical prior-round issues have been resolved. However, the accidental inclusion of a planning document and the newly introduced contradictory is_event_source comments are concrete problems that should be fixed before merge. The residual nondeterminism and dead-guard issues from prior rounds lower confidence further, though they are unlikely to cause runtime failures.
• docs/plans/entropy-capture-implementation.md (should be removed), hyperstack-macros/src/validation/mod.rs (contradictory comment at line 158–162 and dead is_event_source guard at line ~545), hyperstack-macros/tests/key_resolution_dynamic.rs and sibling dynamic test harnesses (timestamp collision + leaked temp directories from prior rounds).

Important Files Changed

Filename	Overview
hyperstack-macros/src/validation/mod.rs	New 1,772-line validation module implementing the core semantic validation pipeline; contains many issues previously flagged in prior review rounds (nondeterministic HashMap iteration, dead is_event_source guard, aggregate-condition fallback gaps) — several are addressed but contradictory comments and dead guards remain.
hyperstack-macros/src/parse/conditions.rs	Refactored condition parsing: parse_condition_expression renamed to parse_condition_expression_strict returning Result, new parse_resolver_condition_expression with proper find_top_level_operator-based logical rejection, backslash escape fix, and improved parse_value error handling — most prior-round issues addressed.
hyperstack-macros/src/validation/idl_refs.rs	New IDL-aware validation helpers; resolve_instruction_lookup_from_string now returns InvalidPath when the program name is unrecognised instead of falling back to idls.first(), fixing a previously flagged false-positive issue.
hyperstack-macros/src/parse/attributes.rs	Adds span-preserving MapAttribute/EventAttribute fields, classify_source_type_path, parse_join_on_literal using syn::parse_str (panic-safe), eager strategy and condition validation; is_event_source is still always set to false and the new comment in entity.rs confirms this is intentional.
hyperstack-macros/src/stream_spec/entity.rs	Wires validate_semantics into entity processing; parse_url_template now returns syn::Result and validates empty field refs; aggregate_conditions upgraded from HashMap<String,String> to typed ConditionExpr; key format uses entity.field prefix that validate_aggregate_conditions correctly strips via split_once('.').
interpreter/src/compiler.rs	Adds intentional one-directional _address alias resolution (foo_address index resolves a handler keyed on foo) with clear inline comment linking to the compile-time mirror in lookup_index_leafs.
hyperstack-macros/src/diagnostic.rs	New shared diagnostic helpers (ErrorCollector, invalid_choice_message, suggestion_or_available_suffix); the test invalid_choice_message_does_not_repeat_available_values confirms the previously flagged duplicate-suffix issue is resolved.
docs/plans/entropy-capture-implementation.md	1,252-line internal planning document (Status: Planning) that the PR description states was removed in the rewrite — accidentally included again and should be dropped before merge.
hyperstack-macros/tests/key_resolution_dynamic.rs	New dynamic compile-failure test harness for key-resolution validation; previously flagged issues (nanosecond-timestamp collision, leaked temp directories) are present but were already covered in prior review rounds.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[#hyperstack macro invoked] --> B{ItemMod or ItemStruct?}
    B -->|ItemMod| C[process_module]
    B -->|ItemStruct| D[process_entity_struct_with_idl]

    D --> E[Parse field attributes\n#map #event #aggregate #resolve #view]
    E --> F{parse_recognized_field_attribute}
    F -->|Map/FromInstruction| G[parse_map_attribute\nvalidate strategy / condition / join_on]
    F -->|Event| H[parse_event_attribute\nvalidate from / lookup_by]
    F -->|Resolve| I[parse_resolve_attribute\nvalidate URL template / resolver type]

    G & H & I --> J[Accumulate:\nsources_by_type\nevents_by_instruction\nderive_from_mappings\naggregate_conditions]

    J --> K[validate_semantics]

    K --> L[validate_key_resolution_paths]
    L --> L1[validate_source_handler_keys\ncheck primary_key / lookup_by / join_on / resolver_hook]
    L --> L2[validate_event_handler_keys\ncheck lookup_by / join_on / capture_fields]
    L --> L3[validate_instruction_hook_keys\ncheck derive_from field or lookup_by]

    K --> M[validate_mapping_references\nIDL field validation per source type]
    K --> N[validate_event_references\nIDL capture_field validation per instruction]
    K --> O[validate_derive_from_references\nIDL field lookup per instruction hook]
    K --> P[validate_aggregate_conditions\ncondition leaf vs IDL instruction/account fields]
    K --> Q[validate_resolve_specs\nfrom / schedule_at / condition vs entity fields]
    K --> R[validate_views\nSort/MaxBy/MinBy/Filter field vs entity fields\nduplicate ID check]
    K --> S[validate_computed_fields\nfield refs vs entity fields\ncycle detection]

    K -->|All errors collected| T{ErrorCollector.finish}
    T -->|errors empty| U[Proceed to codegen]
    T -->|errors present| V[combine_errors → compile error with spans]

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: docs/plans/entropy-capture-implementation.md
Line: 1-5

Comment:
**Planning document accidentally included in the PR**

The PR description explicitly states:

> "this branch was rewritten to remove accidentally committed planning docs and IDL fixtures, so the PR now reflects only the compiler and runtime changes still intended for merge"

Despite this, `docs/plans/entropy-capture-implementation.md` (1,252 lines) has been added to the diff. Its status header reads `Status: Planning`, confirming this is an internal design document that was not intended for this PR.

This should be removed from the branch before merging.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: hyperstack-macros/src/validation/mod.rs
Line: 158-162

Comment:
**Contradictory `is_event_source` comments introduced in the same PR**

Two new comments added in this PR directly contradict each other.

`validation/mod.rs` (here, lines 159–160) says:
```rust
// Event-derived MapAttributes are validated here first (before being merged into
// sources_by_type for codegen). They have is_event_source=true and are created
// in handlers.rs, while other mappings in entity.rs have is_event_source=false.
```

`stream_spec/entity.rs` (lines 101–105, also new in this PR) says:
```rust
// NOTE: is_event_source=false is correct here.
// ...
// is_event_source=true and validated separately via validate_event_handler_keys
is_event_source: false,
```

One comment says event-derived `MapAttribute` values have `is_event_source=true`; the other says `false` is correct. Both are new in this PR. Neither is accurate: every construction site sets `is_event_source: false`, and no code path ever sets it to `true`.

The comment here should be updated to match `entity.rs`, for example:

```rust
// Event-derived MapAttributes live in `events_by_instruction` and are validated
// via `validate_event_handler_keys`. Sources in `sources_by_type` always have
// `is_event_source=false`; the guard below (`all(|m| m.is_event_source)`) is
// intentionally dead scaffolding for a future migration path.
```

Or, if the dead guard is not intended for future use, remove it along with the misleading comment.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (54): Last reviewed commit: "chore: Update stacks" | Re-trigger Greptile}

[greptile-apps]

Greptile encountered an error while reviewing this PR. Please reach out to support@greptile.com for assistance.