Skip to content

feat(pipeline): Ensemble production topology routing pass for ObjectScript/IRIS - #1063

Open
isc-tdyar wants to merge 1 commit into
DeusData:mainfrom
isc-tdyar:feat/ensemble-routing
Open

feat(pipeline): Ensemble production topology routing pass for ObjectScript/IRIS#1063
isc-tdyar wants to merge 1 commit into
DeusData:mainfrom
isc-tdyar:feat/ensemble-routing

Conversation

@isc-tdyar

Copy link
Copy Markdown
Contributor

Adds pass_ensemble_routing — a pipeline pass that statically extracts InterSystems Ensemble/IRIS Interoperability production topology from ObjectScript .cls files at index time. No live IRIS instance required.

Closes #1061. Follows up on #467 (ObjectScript language support) — split out at reviewer request.

What it does

Pass A — EnsembleItem nodes: reads each EnsembleProduction class's ProductionDefinition XData block (XML), parses <Item Name="..." ClassName="..."> entries, and creates EnsembleItem nodes for each production component (business services, processes, operations).

Pass B — ROUTES_TO edges: for each EnsembleItem, reads the source .cls file and extracts routing rules referencing other components by class name, emitting ROUTES_TO edges. Uses segment-anchored matching (qn_ends_with_segment) to avoid false positives from partial class name matches.

WorkMgr dispatch: detects .Queue("##class(X).method", ...) calls and emits CALLS edges to the target method — the same static dispatch pattern as task queues in Celery/Airflow.

Changes from the version reviewed in #467

Per review feedback:

  • Language gate: early-exits immediately if the project contains no ObjectScript files — zero overhead for non-IRIS projects
  • Truncation logging: cbm_log_warn fires when MAX_ITEMS or MAX_SETTINGS caps are hit so silent truncation is visible in logs

Files changed

  • src/pipeline/pass_ensemble_routing.{c,h} — new pass
  • src/pipeline/pipeline.c — wired into run_predump_passes
  • Makefile.cbm — added to SRCS

@isc-tdyar
isc-tdyar requested a review from DeusData as a code owner July 12, 2026 19:58
@DeusData DeusData added enhancement New feature or request parsing/quality Graph extraction bugs, false positives, missing edges language-request Request for new language support priority/backlog Valuable contribution, lower scheduling urgency; review when maintainer capacity opens. labels Jul 14, 2026
@DeusData DeusData added this to the 0.9.2-rc milestone Jul 14, 2026
@DeusData

Copy link
Copy Markdown
Owner

Thanks for splitting the Ensemble topology pass from the base ObjectScript support. This is a 0.9.2-rc language enhancement. Review will focus on bounded parsing of embedded production XML, deterministic EnsembleItem identity, conservative ROUTES_TO matching, malformed-input handling, and representative IRIS fixtures without requiring a live server.

@DeusData

Copy link
Copy Markdown
Owner

Reviewed in full. This is a marked step up from #1064, and I want to say that first because the contrast is the most useful thing I can tell you.

It demonstrably works end to end. Your test runs the full pipeline on two real .cls fixtures and asserts at least two EnsembleItem nodes and at least one ROUTES_TO edge in the dumped store — return codes checked, post-dump, not at the extraction boundary. Every CI leg is green. That is exactly what #1064 was missing: there, the queries could not parse and the return codes were dropped, so it always returned empty while looking healthy. Here I could confirm the inputs genuinely exist (XData nodes are emitted at extract_defs.c:5798, parent_class at pass_definitions.c:288) and the outputs land in the store.

The review feedback from #467 was clearly absorbed too: the language gate, truncation warnings, the segment-anchored class_name_matches so Ens cannot match Ens.BusinessService, and cbm_fopen for Windows paths. The confidence scoring (0.95 literal, 0.85 property-resolved, −0.10 on the entry-point fallback) fits our existing cross-service linking model well.

The direction question is the maintainer's, and it is about schema rather than quality. EnsembleItem is a new node label and ROUTES_TO a new edge type — permanent core-schema surface, for one vendor's language. Whether vendor-specific labels belong in the core schema is his call, not mine, and I have put it to him.

Worth knowing while that is decided: EnsembleItem is excluded by all four hardcoded label IN ('Function','Method','Class') lists in store.c (arch boundaries, clustering, hotspots, vector search). For topology nodes that is arguably right — but it means get_architecture would show none of a production's topology, which cuts against the feature's stated purpose. Three separate contributors have now hit that same allowlist trap in this review round, so it is a systemic thing rather than a criticism of your PR.

One defect I would fix before merge, because it is a graph-accuracy problem.

scan_source_for_send_targets takes a method_name parameter and then discards it ((void)method_name, line 327). It scans the whole file and returns only the first SendRequestSync target. resolve_method_routes then attributes that single target to every production-member method in the file. So a class with N methods and M distinct targets produces N identical edges to the first target — wrong-method attribution and undercounting at the same time. Your single-method fixture cannot catch it; a two-method, two-target fixture would.

And one thing to correct in the description: the third bullet advertises WorkMgr dispatch — detecting .Queue("##class(X).method") and emitting CALLS edges. That code is not in the diff; there is no Queue or ##class handling anywhere. Either it belongs to a different branch or the body is ahead of the code. Worth fixing so the next reviewer is not looking for it.

Three smaller notes:

  • The "zero overhead for non-IRIS projects" claim is not quite right. has_objectscript_nodes walks every node in the graph buffer with a strlen and up to three suffix comparisons, and the visitor API has no early break — so it is O(N) per index, in both FAST and FULL modes. On an 8.5M-node graph that is tens of milliseconds; small, but not zero, and the discover layer already knows per-language file counts if you want a cheaper gate. Also .cls is shared with Apex, so Salesforce repos pass the extension gate too.
  • extract_xml_attr searches unbounded from the tag offset, so an Item or Setting missing an attribute silently inherits it from a later tag — a wrong Enabled or a mispaired ClassName. Bounding the search to the tag's closing > fixes it.
  • properties_json is assembled by snprintf without JSON-escaping the XML-derived strings, so a quote in an item or class name produces malformed JSON in the graph.

One genuine question rather than a defect: you match only TargetConfigName (singular), but real Ensemble routers commonly use TargetConfigNames (plural, comma-separated). Is the singular-only set deliberate?

Static extraction of production topology without a live IRIS instance is a real capability for IRIS shops. The open question on our side is the schema surface, not the work.

@DeusData

Copy link
Copy Markdown
Owner

Maintainer's answer on the schema question, and it's a yes to the capability with a change to how it's expressed: please model the topology in the schema vocabulary we already have, rather than adding EnsembleItem and ROUTES_TO.

The reasoning is the one you'd expect — node labels and edge types are permanent, agent-visible surface, and a vendor-specific pair sets a precedent we'd owe every other platform. But that's not actually the strongest argument here. The strongest argument is that the model you need already exists, and adopting it fixes the problem flagged in my last review instead of leaving it open.

The existing vocabulary

pass_route_nodes.c already has a general channel-topology model that isn't HTTP-specific:

  • Route — a named channel, deduplicated by a synthetic qualified name of the form __route__<kind>__<name>, with the transport recorded in properties ({"broker":"..."}). It's already used for pubsub, cloud tasks and generic broker channels, not just URLs.
  • HANDLES — code → channel: "this method services this channel."
  • ASYNC_CALLS — code → channel for non-HTTP transports. pass_calls.c picks this over HTTP_CALLS precisely when the target isn't an HTTP service.

An Ensemble production maps onto that almost exactly:

yours existing
EnsembleItem node Route, qn __route__ensemble__<Production>.<Item>, props {"broker":"ensemble","class":"...","enabled":...}
ROUTES_TO edge ASYNC_CALLS from the sending method to the target item's Route
(implicit today) HANDLES from the item's implementing class/method to its own Route

Note the second row also fixes the defect below for free: ASYNC_CALLS originates at the sending method, so per-method attribution becomes the natural shape rather than something you have to be careful about.

What you get immediately by doing it this way

This is the part I'd have led with if I'd seen it earlier. Last time I told you EnsembleItem is excluded by the four hardcoded label lists in store.c, so get_architecture would show none of a production's topology — cutting against the feature's whole purpose. Route doesn't have that problem:

  • get_architecture already renders Route nodes in their own section (arch_routes() in store.c). Your topology shows up in the architecture view with no allowlist edits at all.
  • search_graph ranks Route above everything else — the label boost for Route is higher than the one for Function/Method (compute_search_score() in mcp.c). Production items would surface at the top of results instead of being unfindable.
  • Package/grouping queries already collect by Route.
  • The cross-service matching phase in pass_route_nodes.c already bridges producer channels to handler channels by name — the same problem your pass solves, so there may be machinery to reuse rather than rewrite.

So this isn't "rename your labels to please the schema police." Going through Route is the difference between a topology the tools can see and one they can't.

If the mapping turns out to lose something real — a property with nowhere to live, or a relationship ASYNC_CALLS genuinely can't express — say so concretely and we'll look at it again. A gap that forces new vocabulary is a legitimate answer; I just want to be sure we've established there is one before minting labels.

Still needs fixing regardless of the above

The multi-target defect from my last review is a graph-accuracy bug and blocks merge either way: scan_source_for_send_targets discards its method_name parameter, scans the whole file, and returns only the first target — so every production-member method in a class gets an identical edge to that first target. Wrong-method attribution and undercounting in one. A two-method, two-target fixture catches it; your single-method fixture can't.

And the PR description still advertises WorkMgr dispatch (.Queue("##class(X).method") → CALLS edges) that isn't in the diff.

Your TargetConfigNames question from last time is still open and still yours to answer — I'd expect the plural form to matter in real routers.

Thanks for your patience on the direction call. The static-extraction-without-a-live-instance capability is genuinely worth having; I'd just like it speaking the vocabulary the rest of the graph already speaks.

@isc-tdyar
isc-tdyar force-pushed the feat/ensemble-routing branch from db30e5d to 5321936 Compare August 24, 2026 19:58
@isc-tdyar

Copy link
Copy Markdown
Contributor Author

Rebased and reworked per your review:

  • EnsembleItem nodes replaced with Route nodes (__route__ensemble__<prod>.<item>) so Ensemble topology is visible to search_graph, trace_path, and get_architecture without special-casing
  • ROUTES_TO edges replaced with ASYNC_CALLS (method → Route) and HANDLES (entry-point → Route) using the existing vocabulary
  • scan_source_for_send_targets now scopes its scan to the calling method's brace-delimited body — fixes the cross-method contamination where the first SendRequestSync in any method would win regardless of which method was being resolved
  • TargetConfigNames (plural) added to TOPOLOGY_SETTINGS and split on comma so multi-target routers emit a separate edge per target
  • Tests updated: pipeline_ensemble_routing_edges checks Route/ASYNC_CALLS/HANDLES; new pipeline_ensemble_routing_method_scoping test verifies no cross-contamination between two methods routing to different operations

@isc-tdyar
isc-tdyar force-pushed the feat/ensemble-routing branch from 5321936 to 9a1b386 Compare August 24, 2026 20:52
Parses XData ProductionDefinition blocks to extract production topology
and emits Route/ASYNC_CALLS/HANDLES graph edges for Ensemble
interoperability routing.

Key design points:
- Emit Route nodes (qn: __route__ensemble__<prod>.<item>) with props
  {broker, class, enabled, production} — visible in get_architecture
  and search_graph at the same priority as HTTP routes
- Emit ASYNC_CALLS edges from source method/item to target Route node
  via SendRequestSync literal scan and TargetConfigName(s) settings
- Emit HANDLES edges from each item's entry-point method to its Route
  node, completing the code-to-topology link
- Fix scan_source_for_send_targets: scope search to calling method body
  using brace-depth tracking to eliminate cross-method contamination
- Handle TargetConfigNames (plural, comma-separated) by emitting one
  ASYNC_CALLS edge per token
- Segment-anchored class_name_matches() prevents prefix collisions
  ("Ens" not matching "Ens.BusinessService")
- Language-gate: skip entirely when no ObjectScript nodes present

Tests: pipeline_ensemble_routing_edges (Route nodes + ASYNC_CALLS +
HANDLES); pipeline_ensemble_routing_method_scoping (two methods, two
targets, no cross-contamination).

Fixes DeusData#467

Signed-off-by: Thomas Dyar <tdyar@intersystems.com>
@isc-tdyar
isc-tdyar force-pushed the feat/ensemble-routing branch from 9a1b386 to ee12615 Compare August 24, 2026 23:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request language-request Request for new language support parsing/quality Graph extraction bugs, false positives, missing edges priority/backlog Valuable contribution, lower scheduling urgency; review when maintainer capacity opens.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ensemble production topology routing pass for ObjectScript/IRIS

2 participants