Skip to content

SY-3495: Refactor Arc to Support a Module-Based STL#1954

Open
emilbon99 wants to merge 45 commits intorcfrom
sy-3495-arc-modules
Open

SY-3495: Refactor Arc to Support a Module-Based STL#1954
emilbon99 wants to merge 45 commits intorcfrom
sy-3495-arc-modules

Conversation

@emilbon99
Copy link
Contributor

@emilbon99 emilbon99 commented Feb 7, 2026

Issue Pull Request

Linear Issue

SY-####

Description

Basic Readiness

  • I have performed a self-review of my code.
  • I have added relevant, automated tests to cover the changes.
  • I have updated documentation to reflect the changes.

Greptile Overview

Greptile Summary

This PR refactors ARC (Go + C++ runtimes, compiler, and service integration) to use the new module-based API, updating WASM bindings, runtime state layout, STL module registration, and symbol resolution wiring.

Main correctness issue found is in the framer calculation path: Calculator.Next was updated to use the new state.Channel.Flush, but it no longer clears the transient series/string handle stores each cycle (previously done by state.Flush()). The ARC task runtime path does clear these stores, so the calculator path should match to avoid handle accumulation and leaks when WASM code uses series/strings.

Confidence Score: 4/5

  • Mostly safe to merge once the transient state clearing bug in the calculator path is fixed.
  • Large refactor but the main behavioral change I could verify as incorrect is the missing per-cycle clearing of Series/Strings in Calculator.Next, which can leak/accumulate handles for WASM programs using those facilities. Other reviewed integration points (task runtime, module open, state split) align with the new API patterns.
  • core/pkg/service/framer/calculation/calculator/calculator.go

Important Files Changed

Filename Overview
core/pkg/service/framer/calculation/calculator/calculator.go Refactor to new module-based ARC runtime API; Next now uses state.Channel.Flush but misses clearing state.Series/state.Strings transient stores each cycle (bug).
core/pkg/service/arc/runtime/task.go Updates ARC task runtime to module-based API; ensures transient state is cleared each cycle via state.Series.Clear() and state.Strings.Clear() and flushes authority changes.
core/pkg/service/arc/symbol/resolver.go Adapts service-level symbol resolver to module-based API; mostly plumbing changes for new compiler/runtime interfaces.
core/pkg/service/arc/status/set.go Adjusts ARC status setting integration for new runtime/task structures; no issues spotted in diff review.
arc/go/runtime/wasm/module.go Introduces/updates WASM module opening to accept module registry (STLModules) and state; non-test callers pass state; no definite issues found.
arc/go/runtime/state/state.go Splits runtime state into subcomponents (Channel/Series/Strings/Auth) under module-based API; aligns with per-cycle clearing in task runtime.
arc/cpp/runtime/state/state.cpp Refactors C++ runtime state flushing/clearing to match module-based API; includes clearing of series/string stores during flush.

Sequence Diagram

sequenceDiagram
    participant Calc as Calculator
    participant Ch as state.Channel
    participant Sch as scheduler.Scheduler
    participant Mod as wasm.Module
    participant Ser as state.Series
    participant Str as state.Strings

    Calc->>Ch: Ingest(input)
    loop while output changed
        Calc->>Sch: Next(ctx, since(start), ReasonChannelInput)
        Sch->>Mod: Execute node graph (WASM)
        Mod-->>Ser: Allocate/update series handles
        Mod-->>Str: Allocate/update string handles
        Calc->>Ch: Flush(outFrame)
        Ch-->>Calc: (outFrame, changed?)
    end
    Calc->>Ch: ClearReads()
    Note over Calc,Ser: BUG: Series.Clear() not called
    Note over Calc,Str: BUG: Strings.Clear() not called

    rect rgba(200,255,200,0.25)
      Note over Calc,Ser: Expected each cycle
      Calc->>Ser: Clear()
      Calc->>Str: Clear()
    end
Loading

emilbon99 and others added 27 commits February 5, 2026 14:30
…s-in-arc' of https://github.com/synnaxlabs/synnax into sy-3572-add-mechanisms-for-setting-control-authority-in-arc
Fix issues with the metrics groups appearing twice. This happened because if you moved the metrics channels to another group, and then restarted the Core, the original Metrics -> metric_channel ontology relationship would reappear.
…synnax into sy-3572-add-mechanisms-for-setting-control-authority-in-arc
…2-add-mechanisms-for-setting-control-authority-in-arc
@emilbon99 emilbon99 changed the title [arc] - refactored to use new module based API SY-3495: Refactor Arc to Support a Module-Based STL Feb 7, 2026
@codecov
Copy link

codecov bot commented Feb 8, 2026

Codecov Report

❌ Patch coverage is 25.66964% with 333 lines in your changes missing coverage. Please review.
✅ Project coverage is 53.48%. Comparing base (66ba49b) to head (f39b1d7).

Files with missing lines Patch % Lines
arc/go/compiler/resolve/emit.go 0.00% 172 Missing ⚠️
arc/go/compiler/expression/compiler.go 3.33% 25 Missing and 4 partials ⚠️
arc/go/compiler/resolve/resolver.go 58.20% 28 Missing ⚠️
arc/go/compiler/statement/variable.go 7.14% 18 Missing and 8 partials ⚠️
arc/go/compiler/testutil/testutil.go 0.00% 20 Missing ⚠️
arc/go/compiler/context/context.go 0.00% 16 Missing ⚠️
arc/go/compiler/expression/binary.go 0.00% 4 Missing and 6 partials ⚠️
arc/go/compiler/resolve/derive.go 72.97% 5 Missing and 5 partials ⚠️
arc/go/compiler/compiler.go 74.07% 5 Missing and 2 partials ⚠️
arc/go/compiler/expression/literal.go 0.00% 2 Missing and 3 partials ⚠️
... and 3 more
Additional details and impacted files
@@            Coverage Diff             @@
##               rc    #1954      +/-   ##
==========================================
- Coverage   53.51%   53.48%   -0.04%     
==========================================
  Files        2406     2424      +18     
  Lines      138508   139529    +1021     
  Branches     7106     7111       +5     
==========================================
+ Hits        74121    74624     +503     
- Misses      62582    63059     +477     
- Partials     1805     1846      +41     
Flag Coverage Δ
alamos-go 55.25% <ø> (ø)
arc-go 75.64% <25.66%> (+1.13%) ⬆️
aspen 69.18% <ø> (-0.26%) ⬇️
cesium 83.09% <ø> (ø)
client-py 87.05% <ø> (ø)
core 66.81% <ø> (-0.12%) ⬇️
freighter-go 63.34% <ø> (+0.08%) ⬆️
freighter-integration 1.50% <ø> (ø)
freighter-py 79.47% <ø> (ø)
pluto 48.99% <ø> (ø)
x-go 78.49% <ø> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 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.

Base automatically changed from sy-3572-add-mechanisms-for-setting-control-authority-in-arc to rc February 9, 2026 21:03
Base automatically changed from rc to main February 11, 2026 16:33
@pjdotson pjdotson changed the base branch from main to rc February 11, 2026 16:35
Copy link
Contributor

@pjdotson pjdotson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally good, I like the syntax. Needs to be better tested + some concerns about the exponent function.

@@ -0,0 +1,464 @@
# Arc Module System Specification
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what should and shouldn't get committed for working docs like this in terms of Markdown
files? Don't think we should be committing 4 markdown files for every significant PR but
also support having better documentation at this level (IE the existing RFCs).

@@ -39,7 +38,7 @@ type Context[ASTNode antlr.ParserRuleContext] struct {
func Child[P, ASTNode antlr.ParserRuleContext](ctx Context[P], node ASTNode) Context[ASTNode] {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a test

@@ -63,25 +62,30 @@ func (c Context[AstNode]) WithScope(scope *symbol.Scope) Context[AstNode] {

func (c Context[ASTNode]) WithNewWriter() Context[ASTNode] {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a test

return c
}

func CreateRoot(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a test

// Resolve looks up a symbol by qualified name. If the name has the prefix
// "Name.", it strips the prefix and delegates to Members. Otherwise it returns
// query.ErrNotFound.
func (m *ModuleResolver) Resolve(ctx context.Context, name string) (Symbol, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a test

public:
explicit Module(runtime::errors::Handler handler): handler(std::move(handler)) {}

void bind_to(wasmtime::Linker &linker, wasmtime::Store::Context cx) override {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should have tests

#include "arc/cpp/stl/series/state.h"
#include "arc/cpp/stl/str/state.h"

namespace arc::stl::stateful {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should have tests

#include "arc/cpp/stl/stl.h"
#include "arc/cpp/stl/str/state.h"

namespace arc::stl::stateful {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should have tests

std::unordered_map<uint32_t, x::telem::Series> handles;
uint32_t counter = 1;

public:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should have tests

void Module::bind_to(wasmtime::Linker &linker, wasmtime::Store::Context cx) {
auto ss = this->series_state;

#define BIND_SERIES_OPS(suffix, cpptype, data_type_const) \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should have tests

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

Comments