[jmthomas]

Closes #3932

Problem

disable_limits_group on a large group (70k items in the report) had to be run several times before limits actually turned off.

Root cause: the decom microservice learns about enable/disable changes from SCOPE__openc3_limits_events, which is capped at ~1000 entries (maxlen 1000). _limits_group wrote one LIMITS_ENABLE_STATE event per item, and decom also writes a LIMITS_CHANGE event for every item state change into the same stream. When decom falls behind processing telemetry, events are trimmed before it reads them. current_limits_settings in Redis was always correct, but decom only reads that at startup, so a restart was the only reliable workaround.

Changes (Ruby and Python)

• One event per group. enable_limits_group / disable_limits_group write a single LIMITS_GROUP_ENABLE_STATE event listing the items by target and packet, instead of one event per item. current_limits_settings is updated with one bulk read/write, each packet is read and written once, and items are matched with a hash instead of a nested scan. The call goes from ~45s to ~1.5s for 70k items.
• Separate control stream. LimitsEventTopic.write also sends every event except LIMITS_CHANGE to a new low-volume SCOPE__openc3_limits_control stream. The decom microservice and sync_system_thread_body read only that stream, so a flood of limits state changes can't trim control events (settings, enable state, limits set, state colors) before decom reads them. SCOPE__openc3_limits_events still gets every event, so Limits Monitor and get_limits_events are unchanged.
• Decom skips what it doesn't own. process_event applies the group event only to targets, packets and items in that decom's System.
• Python: _limits_group now uses the result of group_name.upper() (previously discarded, so lowercase group names failed).

Behavior change: the group call now logs one Enabling/Disabling Limits Group: NAME message instead of one log line and one event message per item.

Testing

• New unit tests (Ruby + Python): single event per group call, bulk settings update, decom applying group events (unknown targets ignored), control stream excludes LIMITS_CHANGE, and a flood test (group disable followed by 1500 LIMITS_CHANGE events) that fails when the reader uses the old shared stream.
• spec/api, spec/topics, spec/microservices and the Python test/api, test/topics, test/microservices suites pass. decom_microservice_spec.rb:58 fails when run together with the limits specs, but it fails the same way on main.
• End-to-end in a local COSMOS with a 70-packet × 1000-item target and one 70k-item limits group, writing telemetry at 700 packets/s with values that change every item's limits state:

	Still enabled in decom after one disable
main	68280 / 70000 (only a few hundred more items disabled per extra call)
This PR (3 cycles)	0 / 70000 every time

Each cycle also confirmed enable_limits_group took effect starting from disabled.

Not addressed

A decom microservice that is still starting up can miss control events written before its first read of the stream. That was already true before this change; it should be a separate issue.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Limits groups can be enabled or disabled in a single operation across all member items.
  • Group names are handled case-insensitively.
  • Limits control events use a dedicated stream for more reliable synchronization.

• Bug Fixes

  • Existing item settings are preserved during group state changes.
  • Unknown targets and items are safely ignored.
  • Processing continues when an individual item update fails.

• Tests

  • Expanded coverage for grouped updates, event publishing, synchronization, and high-volume event handling.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 77b8871f-be92-4f31-b49e-ef6d278333d6

📥 Commits

Reviewing files that changed from the base of the PR and between 818b5b3 and fa131ce.

📒 Files selected for processing: 4

• openc3/lib/openc3/topics/limits_event_topic.rb
• openc3/python/openc3/topics/limits_event_topic.py
• openc3/python/test/topics/test_limits_event_topic.py
• openc3/spec/topics/limits_event_topic_spec.rb

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.

📝 Walkthrough

The Ruby and Python limits APIs now batch group updates and emit one aggregate enable-state event. Limits event processing stores grouped settings and uses a dedicated control topic for synchronization.

Changes: Limits group enable state

Layer / File(s)	Summary
Aggregate group updates openc3/lib/openc3/api/limits_api.rb, openc3/python/openc3/api/limits_api.py, openc3/spec/api/limits_api_spec.rb, openc3/python/test/api/test_limits_api.py	Both APIs batch member updates by target and packet and emit one group-level event. Tests cover lowercase group names, persisted member settings, and event contents.
Persist and publish grouped settings openc3/lib/openc3/topics/limits_event_topic.rb, openc3/python/openc3/topics/limits_event_topic.py, openc3/spec/topics/limits_event_topic_spec.rb, openc3/python/test/topics/test_limits_event_topic.py	Both event topics bulk-update stored settings for grouped events. Non-change events also use the control stream, while LIMITS_CHANGE remains on the main stream. Tests cover persistence, routing, and existing-setting preservation.
Process control events openc3/lib/openc3/topics/limits_event_topic.rb, openc3/python/openc3/topics/limits_event_topic.py, openc3/lib/openc3/microservices/decom_microservice.rb, openc3/python/openc3/microservices/decom_microservice.py, openc3/spec/topics/limits_event_topic_spec.rb, openc3/python/test/topics/test_limits_event_topic.py	Event processing applies grouped enable and disable operations across valid items, skips missing objects, and continues after item failures. Decom microservices and synchronization loops consume the control stream. Tests cover synchronization, stream trimming, missing objects, and failure continuation.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant LimitsAPI
  participant LimitsEventTopic
  participant Redis
  participant DecomMicroservice
  LimitsAPI->>LimitsEventTopic: Write aggregate LIMITS_GROUP_ENABLE_STATE event
  LimitsEventTopic->>Redis: Bulk-update current limit settings
  LimitsEventTopic->>Redis: Publish event to limits control stream
  DecomMicroservice->>Redis: Read limits control event
  DecomMicroservice->>LimitsEventTopic: Process grouped enable state

Loading

Suggested reviewers: mcosgriff

Merge Risk: ⚪ Minimal · up to fa131

No actionable issue remains from this review; the change is mergeable after normal checks.

Security architecture risk: 🟡 Moderate · up to fa131

The dedicated control stream reduces interference from telemetry traffic, and existing authorization and scope restrictions are preserved. However, older and newer components cannot reliably exchange limits controls during a mixed-version upgrade or rollback. Without coordinated deployment, limits monitoring can remain inconsistent with saved settings.

Retained concerns

• Medium · reliability · inferred: Mixed-version operation can strand limits controls in either upgrade direction. New consumers ignore controls published only to the old stream; old consumers receive but do not recognize new aggregate group events. Consequently, an enable request can persist successfully while limits checking remains disabled in affected decom processes. Dual publication and startup synchronization do not provide continuous compatibility. Actual exposure depends on upgrade and rollback coordination, which was not established.

Security review details

Security Blast Radius

• inferred — One authorized group operation can affect every configured member of that group, potentially spanning multiple targets and database shards within its scope. Each decom applies only locally owned telemetry members. The inspected changes do not establish cross-scope authority expansion.

Security Findings and Attack Paths

• inferred — The supported concern is monitoring-control drift during incompatible version overlap, not a demonstrated unauthorized attack path. A missed enable operation can leave checking disabled despite updated saved settings. Normal group initiation still requires tlm_set permission.

Trust Boundaries and Controls

• observed — The API derives event membership from an existing scope-specific group rather than accepting arbitrary target/item mappings from the group caller. Publication uses scope-prefixed stream names, and consumption checks local telemetry ownership.

Resilience and Maintainability Implications

• observed — The separate stream prevents LIMITS_CHANGE traffic from trimming control records, although control records still have an approximate 1000-entry retention limit. Startup synchronization reapplies persisted enable settings. Repeating local enable/disable sets the same boolean state, but disable can also invoke a limits-change callback, so replay is not necessarily free of repeated side effects.

Hardening Proposals

• proposed — Define a coordinated, quiesced upgrade and rollback procedure, or introduce explicit version compatibility before enabling the new event contract. Any temporary dual-consumption strategy should account for duplicate callbacks, and the transition should reconcile persisted settings before resuming control operations.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 4.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 25 functions across 10 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: applying limits group enable/disable operations in one pass.
Linked Issues check		Issue #3932 requires one invocation to disable a large limits group. The Ruby and Python APIs now update group items in bulk and emit one LIMITS_GROUP_ENABLE_STATE event. Decom reads a separate contro…
Out of Scope Changes check		The changes stay within issue #3932. Bulk updates reduce the group operation cost. The control stream prevents LIMITS_CHANGE events from hiding group control events. Decom filtering, synchronization c…

Full details: Linked Issues check

Explanation

Issue #3932 requires one invocation to disable a large limits group. The Ruby and Python APIs now update group items in bulk and emit one LIMITS_GROUP_ENABLE_STATE event. Decom reads a separate control stream, and the reported flood test and 70,000-item end-to-end result support one-call disabling. The added tests cover group updates, event delivery, and decom processing.

Full details: Out of Scope Changes check

Explanation

The changes stay within issue #3932. Bulk updates reduce the group operation cost. The control stream prevents LIMITS_CHANGE events from hiding group control events. Decom filtering, synchronization changes, lowercase group-name handling, transactional writes, failure isolation, and related tests support reliable group disable behavior.

✨ Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

🔧 Brakeman (8.0.6)

Brakeman reported 6 analysis error(s); security analysis is incomplete

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.80%. Comparing base (ebd086d) to head (fa131ce).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4112      +/-   ##
==========================================
+ Coverage   79.77%   79.80%   +0.03%     
==========================================
  Files         907      907              
  Lines       69969    70024      +55     
  Branches     2688     2688              
==========================================
+ Hits        55817    55882      +65     
+ Misses      13480    13470      -10     
  Partials      672      672              

Flag	Coverage Δ
frontend	67.27% <ø> (-0.02%)	⬇️
python	78.37% <ø> (+0.02%)	⬆️
ruby-api	83.52% <ø> (+0.24%)	⬆️
ruby-backend	86.13% <100.00%> (+0.01%)	⬆️

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

☔ View full report in Codecov by Harness.
📢 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.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud