Skip to content

feat: number every record with a meta sequenceId - #33

Merged
DavidCozens merged 1 commit into
mainfrom
stage/sequence-id
Jul 29, 2026
Merged

feat: number every record with a meta sequenceId#33
DavidCozens merged 1 commit into
mainfrom
stage/sequence-id

Conversation

@DavidCozens

@DavidCozens DavidCozens commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Pull request

What this stage adds

<134>1 ... solid-syslog-example - BOOT [meta sequenceId="1"] device started

Flash +6,032 B (+932 on the previous stage)
RAM +436 B (+64)

The counter increments once per record formatted, not per record delivered. A
record that never reaches the collector therefore leaves a gap in the sequence
rather than no trace at all — which is the whole value of the field, and the
reason it lands before the circular buffer rather than after it. Instrument
first, then introduce the failure mode; the other order means asserting a drop
you already caused.

The first SD-ELEMENT also introduces the rule that governs the rest: unlike a
header field, an SD PARAM has no NILVALUE, so an unset one is omitted entirely
rather than written as "-".

Atomics joins the platform list. It is named but not linked — a platform over a
stable system API is already inside libSolidSyslog.a, and adding a
SolidSyslog::Atomics link target fails the configure with "target was not found".
Only the header-configured packs have one.

Files

 CMakeLists.txt               |  4 +++-
 README.md                    | 12 ++++++------
 app/syslog/Syslog.c          | 17 ++++++++++++++++-
 measurements/sequence-id.csv | 13 +++++++++++++
 measurements/stages.tsv      |  1 +
 run-report.md                | 32 ++++++++++++++++----------------
 6 files changed, 55 insertions(+), 24 deletions(-)

Checklist

  • The diff touches build infra (1 file(s)) — see Files above; deliberate for this stage.
  • measurements/sequence-id.csv committed.
  • Row added to measurements/stages.tsv.
  • README regenerated: python3 scripts/gen-cost-table.py.
  • ./run.sh green (build + QEMU + baseline self-check) — run-report.md committed.

Context for review: 6 of 21 sequential single-commit PRs replaying the integration on top of the Baseline root. Each lands green and reviewed before the next is built on it, because a change to an early commit would force every commit above it to be re-run and re-measured.

Summary by CodeRabbit

  • New Features

    • Added support for Atomics platform selection.
    • Log records now include incrementing sequence IDs, making gaps from undelivered records visible.
    • Structured metadata now omits unset parameters instead of displaying “-”.
  • Documentation

    • Updated usage, resource-cost, run-report, and validation documentation for sequence IDs and revised measurements.

  <134>1 ... solid-syslog-example - BOOT [meta sequenceId="1"] device started

  Flash  +6,032 B  (+932 on the previous stage)
  RAM      +436 B    (+64)

The counter increments once per record *formatted*, not per record delivered. A
record that never reaches the collector therefore leaves a gap in the sequence
rather than no trace at all — which is the whole value of the field, and the
reason it lands before the circular buffer rather than after it. Instrument
first, then introduce the failure mode; the other order means asserting a drop
you already caused.

The first SD-ELEMENT also introduces the rule that governs the rest: unlike a
header field, an SD PARAM has no NILVALUE, so an unset one is omitted entirely
rather than written as "-".

Atomics joins the platform list. It is named but not linked — a platform over a
stable system API is already inside libSolidSyslog.a, and adding a
SolidSyslog::Atomics link target fails the configure with "target was not found".
Only the header-configured packs have one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The example adds Atomics platform support, wires persistent meta sequenceId structured data into Syslog_Start, and updates README and run-report records for sequence-number behavior and measurements.

Changes

Sequence ID integration

Layer / File(s) Summary
Atomics platform configuration
CMakeLists.txt
Adds Atomics to SOLIDSYSLOG_PLATFORMS.
Persistent sequence metadata wiring
app/syslog/Syslog.c
Creates structured data backed by an atomic counter and connects it through SolidSyslogConfig.
Sequence ID documentation and measurements
README.md, run-report.md
Documents meta sequenceId behavior and updates recorded costs, output, ELF sizes, and self-check results.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and matches the main change: adding a sequenceId to every syslog record.
Description check ✅ Passed The description follows the template and includes the stage summary, file list, and checklist with the required details.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch stage/sequence-id

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
run-report.md (1)

50-64: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Exercise sequence progression in the self-check.

The report only proves that the first formatted record contains sequenceId="1"; it does not verify that a subsequent formatted record receives the next ID. Add a two-record check (or a deterministic test) that parses and compares both sequence IDs.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@run-report.md` around lines 50 - 64, The self-check currently validates only
the first formatted record’s sequence ID. Extend the report validation around
the formatted-record parsing to generate or parse two consecutive records,
extract both sequence IDs, and assert they progress from 1 to 2 (or compare
against the expected deterministic sequence), while preserving the existing
first-record and measurement checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@run-report.md`:
- Around line 50-64: The self-check currently validates only the first formatted
record’s sequence ID. Extend the report validation around the formatted-record
parsing to generate or parse two consecutive records, extract both sequence IDs,
and assert they progress from 1 to 2 (or compare against the expected
deterministic sequence), while preserving the existing first-record and
measurement checks.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: db8363f3-3fc4-41da-ba1f-4f004951277e

📥 Commits

Reviewing files that changed from the base of the PR and between 7762593 and f51f8dc.

⛔ Files ignored due to path filters (2)
  • measurements/sequence-id.csv is excluded by !**/*.csv
  • measurements/stages.tsv is excluded by !**/*.tsv
📒 Files selected for processing (4)
  • CMakeLists.txt
  • README.md
  • app/syslog/Syslog.c
  • run-report.md

@DavidCozens
DavidCozens merged commit 47835a5 into main Jul 29, 2026
2 checks passed
@DavidCozens
DavidCozens deleted the stage/sequence-id branch July 29, 2026 15:43
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