[jmthomas]

Follow-up to #3951, addressing review findings. Targets feat/stream-safety-trim.

What

• Env var reaches the operator. compose.yaml passes env vars to each service by name, and OPENC3_STREAM_MAX_AGE_SECONDS wasn't listed, so setting it in .env did nothing. It's now in the operator's pass-through list. All writers of these streams (interface and decom microservices) run under the operator. Enterprise/k8s pod specs need the same change.
• A failed log cycle lookup is no longer cached. Before, if TargetModel.get threw on the first packet (e.g. a Redis blip), the 600 s default cycle was cached for the life of the process. A target with TLM_LOG_CYCLE_TIME 3600 would then be capped at 1320 s instead of 7320 s. Now that stream isn't capped until a lookup succeeds. The lookup is retried every LOG_CYCLE_LOOKUP_RETRY_SECONDS (60 s), with one warning per try.
• Minimum max age of 120 s (STREAM_MAX_AGE_MIN_SECONDS, twice TSDB's 60 s retention). Testing against Valkey 9.1.2 showed that a MINID ahead of the stream's ids deletes the whole stream, including the entry being added. For * ids the cutoff comes from the writer's clock, not Redis's, so a tiny value (e.g. 0.5) plus a small clock offset could wipe DECOMCMD__. The docs and code comments now cover the minimum and the need for NTP.
• write_topic raises (Ruby ArgumentError, Python ValueError) if given both maxlen and minid, instead of silently dropping maxlen.
• LogWriter.CLEANUP_DELAY is now defined from Topic.LOG_CLEANUP_DELAY_SECONDS, so there's only one value. Both LogWriters already import Topic.
• Docs: "two log cycles plus one minute" is corrected to "plus two minutes", matching 2 × (cycle + 60).

Testing

• New Ruby and Python tests: the 120 s minimum, a failed lookup that isn't cached and is retried only after the interval and then cached, the lookup raising its error, and write_topic rejecting maxlen + minid.
• bundle exec rspec spec/topics spec/logs: 126 passed. spec/microservices spec/utilities/store* spec/api: 524 passed.
• uv run pytest test/topics test/logs test/microservices: 215 passed. ruff check is clean.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Configuration
  • The operator now accepts the optional OPENC3_STREAM_MAX_AGE_SECONDS setting from the host environment or .env.
  • Positive values below 120 seconds are raised to 120 seconds. Setting the value to zero or a negative number continues to disable the age cap.
  • Telemetry and command streams retain at least two log cycles plus two minutes before trimming. Keep host clocks synchronized for accurate age limits.
• Documentation
  • Updated environment-setting guidance to reflect these limits and clock requirements.

[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: 4789a1d9-96c0-4881-97fc-2e0491894858

📥 Commits

Reviewing files that changed from the base of the PR and between 349f938 and f5c5ded.

📒 Files selected for processing: 10

• compose.yaml
• docs.openc3.com/docs/configuration/environment.md
• openc3/lib/openc3/logs/log_writer.rb
• openc3/lib/openc3/topics/topic.rb
• openc3/lib/openc3/utilities/store_autoload.rb
• openc3/python/openc3/logs/log_writer.py
• openc3/python/openc3/topics/topic.py
• openc3/python/openc3/utilities/store_implementation.py
• openc3/python/test/topics/test_topic.py
• openc3/spec/topics/topic_spec.rb

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

📝 Walkthrough

The operator now receives the optional stream age setting. Ruby and Python Topic implementations enforce a minimum age and retry failed log-cycle lookups. Store implementations reject writes that specify both maxlen and minid.

Changes: Stream retention safeguards

Layer / File(s)	Summary
Configure stream age limits compose.yaml, docs.openc3.com/docs/configuration/environment.md, openc3/lib/openc3/logs/log_writer.rb, openc3/lib/openc3/topics/topic.rb, openc3/python/openc3/logs/log_writer.py, openc3/python/openc3/topics/topic.py, openc3/python/test/topics/test_topic.py, openc3/spec/topics/topic_spec.rb	The operator receives the optional environment variable. Topic implementations clamp positive stream ages below 120 seconds and use the shared cleanup delay in log-retention calculations. Documentation describes the minimum age and synchronized-clock requirement. Tests cover the minimum and warning.
Handle log-cycle lookup failures openc3/lib/openc3/topics/topic.rb, openc3/python/openc3/topics/topic.py, openc3/python/test/topics/test_topic.py, openc3/spec/topics/topic_spec.rb	Failed log-cycle lookups leave streams uncapped and are retried after 60 seconds. Successful results are cached, and cache clearing resets retry timestamps. Tests cover error propagation, retry timing, and caching.
Reject conflicting stream write limits openc3/lib/openc3/utilities/store_autoload.rb, openc3/python/openc3/utilities/store_implementation.py, openc3/python/test/topics/test_topic.py, openc3/spec/topics/topic_spec.rb	Ruby and Python Store implementations raise an error when a write supplies both maxlen and minid. Tests assert the respective exceptions.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to f5c5d

No confirmed issue blocks merging. Confirm the setting is available in any separately maintained Kubernetes or Enterprise deployments before relying on it there.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 29.41% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 17 functions across 8 files. (2 skipped: …	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 accurately summarizes the main changes: stronger stream safety trim configuration and handling of log-cycle lookup failures.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 29.41% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 17 functions across 8 files. (2 skipped: 2 unsupported.)

• Fix all pre-merge checks with AI

✨ 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.}

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.28%. Comparing base (349f938) to head (f5c5ded).

Additional details and impacted files

@@                     Coverage Diff                     @@
##           feat/stream-safety-trim    #4075      +/-   ##
===========================================================
+ Coverage                    80.26%   80.28%   +0.01%     
===========================================================
  Files                          901      901              
  Lines                        68776    68801      +25     
  Branches                      2665     2714      +49     
===========================================================
+ Hits                         55204    55235      +31     
+ Misses                       12906    12894      -12     
- Partials                       666      672       +6     

Flag	Coverage Δ
frontend	66.97% <ø> (-0.06%)	⬇️
python	80.25% <ø> (+0.03%)	⬆️
ruby-api	82.80% <ø> (+0.12%)	⬆️
ruby-backend	85.87% <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