Skip to content

feat(simlab): add deterministic flight recorder#2335

Draft
Mikecranesync wants to merge 10 commits into
mainfrom
codex/flight-recorder-phased
Draft

feat(simlab): add deterministic flight recorder#2335
Mikecranesync wants to merge 10 commits into
mainfrom
codex/flight-recorder-phased

Conversation

@Mikecranesync

Copy link
Copy Markdown
Owner

Summary

  • Add a local deterministic SimLab flight recorder for scenario_loaded, per-tick, and evidence_requested events.
  • Expose recorder read/clear/NDJSON endpoints through SimLab API.
  • Document the Hub/relay boundary: future durable production history should use relay ingest into Hub tag_events, not demo-only live_signal_events.

Capability Boundary

  • Phase 1 is headless/local simulation only.
  • No Hub DB writes, MQTT broker, relay calls, PLC writes, Ignition dependency, UUIDs, or wall-clock timestamps are introduced.
  • Live/external publishers remain opt-in through existing env-gated paths.

Verification

  • python -m pytest tests/simlab/test_flight_recorder.py -q -> 12 passed
  • python -m pytest tests/simlab/test_juice_bottling.py tests/simlab/test_dashboard.py tests/simlab/test_publishers.py -q -> 27 passed
  • python -m pytest tests/simlab -q -> 98 passed, 3 skipped
  • git diff --check origin/main..HEAD -> no output

Review Notes

Implemented via the phased subagent-driven plan in docs/superpowers/plans/2026-06-27-simlab-flight-recorder.md. Task-level reviews and a final whole-branch review completed cleanly.

@github-actions

Copy link
Copy Markdown

🤖 AI Code Review

Review by: groq (llama-3.3-70b-versatile)

Review of Pull Request: feat(simlab): add deterministic flight recorder

🔴 IMPORTANT: Security vulnerabilities

No obvious security vulnerabilities were found in the provided diff. However, it's essential to review the entire codebase to ensure that no hardcoded secrets, SQL injection, path traversal, or command injection vulnerabilities are introduced.

🔴 IMPORTANT: Missing error handling on network/IO operations

Error handling for network/IO operations is not explicitly shown in the provided diff. It's crucial to ensure that all network/IO operations have proper error handling to prevent crashes in production. For example, in simlab/api.py, error handling should be added for API endpoints that interact with the flight recorder.

🟡 WARNING: Logic bugs or incorrect assumptions

The code assumes that the SimEngine will always emit events in a deterministic order. However, if the engine's behavior changes in the future, this assumption might be invalidated. It's essential to consider this possibility and add safeguards to ensure the flight recorder's correctness.

🟡 WARNING: Missing input validation at API boundaries

Input validation is not explicitly shown in the provided diff. It's vital to validate all inputs at API boundaries to prevent potential security vulnerabilities or incorrect data processing. For example, in simlab/api.py, input validation should be added for API endpoints that interact with the flight recorder.

🔵 SUGGESTION: Code quality improvements, naming, maintainability

The code is generally well-structured, and naming conventions are followed. However, some suggestions can be made:

  • In simlab/flight_recorder.py, consider adding more descriptive comments to explain the purpose of each function and class.
  • In simlab/api.py, consider adding more specific error handling for API endpoints that interact with the flight recorder.

✅ GOOD: Noteworthy good practices found

The code uses dataclasses, which is a good practice for defining data structures in Python. Additionally, the use of pytest for testing is a good practice for ensuring code correctness.

Overall, the code looks well-structured, and the pull request seems to be a good addition to the MIRA project. However, it's essential to address the mentioned concerns and suggestions to ensure the code's robustness and maintainability.


Generated by the MIRA automated code review pipeline (Groq → Cerebras → Gemini cascade)
To trigger self-fix: run bash scripts/pr_self_fix.sh 2335 locally, or add the auto-fix label to this PR (or run /autofix-pr from a Claude Code session)

@github-actions

github-actions Bot commented Jun 27, 2026

Copy link
Copy Markdown

MIRA staging gate — ✅ PASS

Engine + NeonDB staging branch + Groq cascade against fixed questions, graded on the 5-dimension rubric in docs/specs/mira-answer-quality-standard.md. Skipped questions (embed sidecar unavailable, etc.) are excluded from pass/fail math; the run fails closed if >50% are skipped.

  • mean of means: 4.95 (pass threshold: 3.5, scored over 15/15)
  • questions passed: 15 / 15
  • skipped (harness): 0
  • below mean 3.0: 0 (max allowed: 2)
  • hard fails: 0
  • full run logs
id category g c a s t mean note
oem-model-fault-powerflex-f004 oem_model_fault 5 5 5 5 5 5.00
oem-only-no-fault-sew oem_only 5 5 5 5 5 5.00
symptom-no-oem-abbrev symptom_only 5 4 5 5 5 4.80
uns-gate-grinding uns_gate 5 5 5 5 5 5.00
safety-arc-flash safety 5 5 5 5 5 5.00
greeting-hygiene greeting 5 5 5 5 5 5.00
session-followup followup 5 5 5 5 5 5.00
photo-less-ocr-claim no_photo 5 5 5 5 5 5.00
off-topic-redirect off_topic 5 5 5 5 5 5.00
cmms-context-followup cmms_context 4 4 5 5 5 4.60
oem-fault-variant-lowercase oem_model_fault 5 5 5 5 5 5.00
cross-oem-confusion oem_model_fault 5 5 5 5 5 5.00
oem-unknown-fault-admit oem_unknown_fault 5 5 5 5 5 5.00
safety-loto-explicit safety 5 5 5 5 5 5.00
uns-gate-no-line uns_gate 5 4 5 5 5 4.80

Rubric: docs/specs/mira-answer-quality-standard.md · Spec: docs/specs/staging-environment-spec.md

@github-actions

Copy link
Copy Markdown

🤖 AI Code Review

Review by: groq (llama-3.3-70b-versatile)

Review of MIRA Project Pull Request

🔴 IMPORTANT: Security Vulnerabilities

No obvious security vulnerabilities, such as hardcoded secrets or SQL injection, were found in the provided diff. However, it's essential to review the entire codebase to ensure no sensitive information is exposed.

🔴 IMPORTANT: Missing Error Handling

No significant issues with missing error handling on network/IO operations were identified in the diff. Nevertheless, it's crucial to verify that all potential error scenarios are handled properly to prevent crashes in production.

🟡 WARNING: Logic Bugs or Incorrect Assumptions

The implementation of the deterministic flight recorder seems reasonable, but it's vital to thoroughly test the feature to ensure it works as expected. One potential issue is the assumption that the SimEngine will always emit events in a deterministic order. If this assumption is incorrect, it could lead to inconsistencies in the recorded events.

🟡 WARNING: Missing Input Validation

Input validation is not explicitly mentioned in the diff, but it's essential to ensure that all inputs to the InMemoryFlightRecorder and API endpoints are properly validated to prevent potential issues.

🔵 SUGGESTION: Code Quality Improvements

The code appears well-structured, and the use of dataclasses and FastAPI is a good choice. However, some suggestions for improvement include:

  • Adding more comprehensive documentation to the code, especially for complex functions or classes.
  • Considering the use of a more robust logging mechanism to facilitate debugging and error tracking.
  • Reviewing the code for potential performance bottlenecks, especially when dealing with large amounts of data.

✅ GOOD: Noteworthy Good Practices

The use of pytest for testing and dataclasses for data representation is a good practice. Additionally, the implementation of the InMemoryFlightRecorder and its integration with the SimEngine and API endpoints seems well-structured and easy to follow.

Overall, the pull request appears to be well-structured, and the implementation of the deterministic flight recorder seems reasonable. However, it's essential to thoroughly review the entire codebase and perform comprehensive testing to ensure the feature works as expected and does not introduce any security vulnerabilities or logic bugs.

Example of how to improve code quality:

# Before
class InMemoryFlightRecorder:
    def __init__(self, run_id):
        self.run_id = run_id
        self.events = []

# After
from dataclasses import dataclass

@dataclass
class FlightRecorderEvent:
    event_type: str
    run_id: str
    seed: int
    line_id: str
    tick: int

class InMemoryFlightRecorder:
    def __init__(self, run_id: str):
        """
        Initializes the InMemoryFlightRecorder with the given run_id.
        
        Args:
        run_id (str): The ID of the run.
        """
        self.run_id = run_id
        self.events: list[FlightRecorderEvent] = []

Generated by the MIRA automated code review pipeline (Groq → Cerebras → Gemini cascade)
To trigger self-fix: run bash scripts/pr_self_fix.sh 2335 locally, or add the auto-fix label to this PR (or run /autofix-pr from a Claude Code session)

@github-actions

Copy link
Copy Markdown

🤖 AI Code Review

Review by: groq (llama-3.3-70b-versatile)

Review of SimLab Flight Recorder Pull Request

🔴 IMPORTANT: Security Vulnerabilities

No hardcoded secrets, SQL injection, path traversal, or command injection vulnerabilities were found in the provided diff. However, it is essential to note that a thorough review of the entire codebase is necessary to ensure that no such vulnerabilities exist.

🔴 IMPORTANT: Missing Error Handling

The diff does not explicitly show error handling for network/IO operations. It is crucial to ensure that the code handles potential errors that may occur during API calls or file operations to prevent crashes in production. Specifically, the curl commands in the README.md file should be reviewed to ensure proper error handling.

🟡 WARNING: Logic Bugs or Incorrect Assumptions

The local flight recorder implementation assumes that the SimEngine will always emit events in a deterministic manner. However, if the SimEngine is not properly synchronized or if there are any concurrency issues, the recorder events may not accurately reflect the simulation state. It is recommended to add tests to verify the deterministic behavior of the SimEngine and the flight recorder.

🟡 WARNING: Missing Input Validation

The API endpoints for the flight recorder do not seem to have input validation. For example, the /simlab/flight-recorder/events endpoint should validate that the run_id parameter is a valid string. Similarly, the /simlab/flight-recorder/export.ndjson endpoint should validate that the run_id parameter matches the expected format. Input validation should be added to prevent potential errors or security vulnerabilities.

🔵 SUGGESTION: Code Quality Improvements

The code could benefit from additional logging to track the execution of the flight recorder and any potential errors that may occur. This would make it easier to debug issues in production.

The InMemoryFlightRecorder class could be refactored to use a more robust data structure, such as a queue or a database, to store the recorder events. This would improve the scalability and reliability of the flight recorder.

✅ GOOD: Noteworthy Good Practices

The use of dataclasses and type hints in the code is a good practice that improves the readability and maintainability of the code. The inclusion of tests, such as test_flight_recorder.py, is also a good practice that ensures the correctness of the implementation. The use of a well-structured README.md file to document the API endpoints and their usage is also noteworthy.

Overall, the pull request seems to be well-structured, and the code is readable. However, some improvements are necessary to ensure the reliability, scalability, and security of the flight recorder implementation.


Generated by the MIRA automated code review pipeline (Groq → Cerebras → Gemini cascade)
To trigger self-fix: run bash scripts/pr_self_fix.sh 2335 locally, or add the auto-fix label to this PR (or run /autofix-pr from a Claude Code session)

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.

1 participant