Skip to content

feat(tinydocs-bus): SlideSpec::new constructor over resolved SlideImage values - #41

Merged
senamakel merged 1 commit into
mainfrom
tiny-dep-tinydocs
Oct 12, 2026
Merged

senamakel merged 1 commit into
mainfrom
tiny-dep-tinydocs

Conversation

@senamakel

Copy link
Copy Markdown
Member

Adds SlideSpec::new(title, body, bullets, speaker_notes, images) to tinydocs-bus so hosts can build slides without the struct literal. Takes already-resolved bus SlideImage values. Test slide_spec_new_matches_literal_and_pins_wire_shape pins the wire shape. CONTRACT_VERSION stays 4.

Closes #40
Part of tinyhumansai/openhuman#7578
Part of tinyhumansai/openhuman#7527

Tests: cargo test -p tinydocs-bus (50 passed); clippy 1.98 clean.

…ge values

Hosts resolve image sources under their own policy and build slides from
text fields plus bus SlideImage bytes. Pins the wire shape with a serde test.
CONTRACT_VERSION stays 4.

Co-authored-by: Medulla <medulla@tinyhumans.ai>
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@coderabbitai

coderabbitai Bot commented Oct 12, 2026

Copy link
Copy Markdown

Review in Change Stack →

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: a8fb5f1e-76ba-4fd8-91d9-242b1bb8a143
📥 Commits

Reviewing files that changed from the base of the PR and between dcac0f8 and f2fe9e5.

📒 Files selected for processing (2)
  • crates/tinydocs-bus/src/spec/presentation/mod.rs
  • crates/tinydocs-bus/src/spec/presentation/mod_tests.rs
 _________________________________________________________________________
< Congratulations, you implemented a distributed single point of failure. >
 -------------------------------------------------------------------------
  \
   \   \
        \ /\
        ( )
      .( o ).
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@tinysweeper

tinysweeper Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Tiny Sweeper review

Tiny Sweeper completed its review; deterministic results follow.

State: Ready for maintainer review
Priority: none
Reviewed head: f2fe9e5ec516
Updated: 2026-10-13T11:14:17Z

Review snapshot

Change surface Files Review signal Count
Production 0 Active findings 0
Tests 2 Noted findings 0
Documentation 0 Resolved findings 0
Configuration 0 Pending checks/questions 0

Completeness: Complete
Test assessment: No supported feature-to-test mapping was available; this does not mean tests are absent or passed.

What changed

Adds a public, `#[must_use]`, documented `SlideSpec::new` constructor that takes a `title: impl Into<String>`, optional `body`, `bullets`, optional `speaker_notes`, and `images: Vec<SlideImage>`, and assigns each directly to the corresponding struct field with no branching, transformation, or validation. Because every field is copied one-to-one, the serialized `SlideSpec` and `SlideImage` key sets are unchanged and `CONTRACT_VERSION` is untouched. A crate-local unit test is added that builds a slide two ways (constructor and struct literal), asserts they are equal, pins the sorted JSON key sets for the slide and its image, and round-trips through `serde_json`. No production call sites are modified in this diff.

Features

None identified with supported citations.

Tests

No supported feature-to-test mapping was produced. Test execution is not inferred.

Findings

No active actionable findings.

Before merge

None.

How this fits together

flowchart LR
  n0["SlideSpec<br/>changed"]:::changed
  n1["spec_defaults_optional_fields<br/>changed"]:::changed
  n2["spec"]:::impacted
  n3["assert_rejects"]:::impacted
  n4["...cts_over_long_slide_text_naming_its_index"]:::impacted
  n5["PresentationSpec"]:::impacted
  n1 -->|uses| n5
  n2 -->|uses| n5
  n3 -->|uses| n5
  n4 -->|uses| n0
  n4 -->|calls| n2
  n4 -->|tests| n2
  n4 -->|calls| n3
  n4 -->|tests| n3
  n5 -->|uses| n0
  classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
  classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
  classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
  classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Loading
Agent review details

critique

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The constructor is a one-to-one field mapping, so the test assertions on field equality, serialized key sets, and round-trip are a direct and sufficient check of the only behaviour that could regress.
  • Lane summary: The change adds a `SlideSpec::new` constructor and a test asserting it matches the struct literal and pins the JSON key set; the constructor maps each field one-to-one with no transformation, and the test's expected key lists match the `SlideSpec` and `SlideImage` field sets shown in the repository. Nothing in the diff is buggy. _The code index is behind this pull request (indexed at `f1692787ebd4`), so retrieved context may be out of date._

security

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: No attacker-controlled path is introduced: the constructor is a pure struct literal with no validation, I/O, subprocess, or untrusted-input sink, and the existing validation path is not weakened.
  • Lane summary: This change adds a `SlideSpec::new` constructor and a unit test that pins the serialized `SlideSpec` wire shape. The constructor is a pure struct literal with no validation, no I/O, no subprocess, and no untrusted-input sink; validation remains in `PresentationSpec::validate`, which callers were already required to invoke before synthesis. Nothing in the diff introduces an attacker-controlled path or weakens an existing check, and the test is deterministic and does not rely on network, clock, or ordering. No defects found. _The code index is behind this pull request (indexed at `f1692787ebd4`), so retrieved context may be out of date._

tests

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: The change adds a `SlideSpec::new` constructor plus a test that pins its field mapping and the serialized wire shape, and the diff is otherwise the existing code. The constructor is a straight field-by-field copy with no branching, so the only behaviour that could regress is the field order/wire keys and the equality with a struct literal, both of which the new test asserts directly. No defect found. _The code index is behind this pull request (indexed at `f1692787ebd4`), so retrieved context may be out of date._

commits

  • Conclusion: Neutral
  • Scope reviewed: all assigned evidence
  • Lane summary: Nothing sensitive found in what this pull request commits.

description

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The documentation states the intended contract boundary — hosts resolve their own image sources and hand the contract only the bytes — and the code follows the repository's documentation and module-test conventions.
  • Lane summary: The change adds a documented `SlideSpec::new` constructor that mirrors the struct literal, plus a unit test in the crate's `mod_tests.rs` that pins the serialized key set and round-trips. The description matches the diff exactly — constructor over already-resolved `SlideImage` values, no wire-contract change, `CONTRACT_VERSION` untouched — and the new code follows the repo's documentation and module-test rules. Nothing here looks unsafe to merge. _The code index is behind this pull request (indexed at `f1692787ebd4`), so retrieved context may be out of date._
Evidence and run details
  • Models: ladder/vectors-oai3, deepseek-v4.1-flash
  • Spend: $0.000566
  • Tokens: 116070 input · 4378 output · 13824 cached · 141 embedding
Head State Pass summary
f2fe9e5ec516 ready for maintainer review 0 active finding(s), 0 resolved finding(s) (at 2026-10-13T11:12:57Z)
f2fe9e5ec516 ready for maintainer review 0 active finding(s), 0 resolved finding(s) (at 2026-10-13T11:14:17Z)

tinysweeper 0.1.2

@tinysweeper tinysweeper 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.

tinysweeper found nothing blocking. Approving.

             $0.0022 · 203,065 in / 18,124 out · 49,536 cached (24%) · ladder/vectors-oai3, deepseek-v4.1-flash · 141 embedded
critique:    $0.0007 · 81,127 in  / 10,145 out · 39,936 cached (49%) · deepseek-v4.1-flash
security:    $0.0006 · 59,507 in  / 1,419 out  · 3,584 cached (6%)   · deepseek-v4.1-flash
tests:       $0.0007 · 46,685 in  / 4,181 out  · 1,920 cached (4%)   · deepseek-v4.1-flash
description: $0.0000 · 13,075 in  / 327 out    · 4,096 cached (31%)  · deepseek-v4.1-flash

@tinysweeper tinysweeper Bot added the priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect. label Oct 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[openhuman#7523] tinydocs: bus helper to unify presentation spec types

1 participant