Post-v0.5.0 cruft sweep - #3
Merged
Merged
Conversation
Eight stale-doc and minor-dedup items found in a second-pass review post-v0.5.0. Zero behavior change, zero schema change, 399 tests still green, ruff clean. .env.example (item 1): - TOPICFORGE_DDS_BACKEND backend list and `auto` chain were stuck at v0.3.0 (Fast > Cyclone > Mock). Refreshed to the v0.4.0 Phase 1.5 8-vendor list and the rti > opensplice > coredx > intercom (Pro) > opendds > fast > cyclone > dust > mock auto chain. MAX_SAMPLE_COUNT consolidation (items 2-3): - Two adapters had silently re-declared `_MAX_SAMPLE_COUNT = 50` locally (`ros2_mock/adapter.py`, `bag_service.py`) — exactly the smell audit item A4 set out to prevent. - A4's home `services/constants.py` was layer-misplaced — importing it from `adapters/` triggered a circular load through `services/__init__.py`. Moved the canonical home up to `topicforge/constants.py` (root-level, layer-neutral). - Dropped the `MAX_SAMPLE_COUNT` re-export from `services/inspector.py` `__all__` ; updated `tests/test_health.py` to import from the new canonical location (the only remaining caller — the "v0.1.x backcompat" comment was misleading). DDS adapter docstring + comment refresh (items 4-6): - `dds_cyclone/adapter.py` module docstring still listed a "v0.3.0 scope" with `peek_dds_samples` raising on user topics. Phase 1.5 shipped the XTypes pipeline ; refreshed to the v0.4.0+ scope listing full participant_events, topic_metrics, and the user-topic `_decode_status` annotation behavior. - `dds_cyclone/adapter.py:100` `_BUILTIN_DCPS_TOPICS` comment likewise refreshed. - `dds_fast/adapter.py` module docstring mirrored — Fast's user-topic path returns raw-annotated samples (not raises), `participant_events` is listener-callback-native, `topic_metrics` is opportunistic. Doc references retired (items 7-8): - `DDS_QUICKSTART.md §3`: extended-QoS coverage tagged "v0.3.x patches" → "v0.5.x patches" (consistent with §6 already updated). - `mcp-02-spec.md`: four "v0.3.x patch/candidate" callouts revisited. Two pointed at things already shipped (FastDdsAdapter v0.3.0, IDL/ XTypes Cyclone v0.4.0 Phase 1.5) — restated as past tense with pointers. One QoS callout retagged v0.5.x. Section heading "resolve before v0.3.x patches" → "v0.5.x patches". Out of scope : - The four remaining "v0.3.x roadmap" strings in `MIGRATION_v0.3_to_v0.4.md`, `audit-post-v0.4.0.md`, and `archive/launch-posts-v0.3.0/` are legitimate historical references (quoting the v0.3.0 error message, audit narrative, frozen marketing post) — kept as-is.
There was a problem hiding this comment.
Pull request overview
Housekeeping PR to align post-v0.5.0 code/docs with current layering and shipped DDS feature set, including consolidating MAX_SAMPLE_COUNT into a layer-neutral module and refreshing DDS/documentation wording.
Changes:
- Moved
MAX_SAMPLE_COUNTto a newtopicforge.constantsmodule and updated services/adapters/tests to import from the new canonical location. - Refreshed DDS adapter module docstrings and updated roadmap/past-tense references in docs.
- Updated
.env.exampleDDS backend value list andautopriority chain to the widened vendor matrix.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_health.py | Updates import path for MAX_SAMPLE_COUNT to the new canonical module. |
| src/topicforge/services/inspector.py | Switches MAX_SAMPLE_COUNT import source and removes it from __all__. |
| src/topicforge/services/health.py | Switches MAX_SAMPLE_COUNT import source for health reporting. |
| src/topicforge/services/constants.py | Removes the previous services-layer home of MAX_SAMPLE_COUNT. |
| src/topicforge/services/bag_service.py | Reuses canonical MAX_SAMPLE_COUNT rather than a local constant. |
| src/topicforge/constants.py | Introduces new root-level constants module containing MAX_SAMPLE_COUNT. |
| src/topicforge/adapters/ros2_mock/adapter.py | Reuses canonical MAX_SAMPLE_COUNT rather than a local constant. |
| src/topicforge/adapters/dds_fast/adapter.py | Updates module docstring to reflect current (v0.4.0+) behavior. |
| src/topicforge/adapters/dds_cyclone/adapter.py | Updates module docstring/comment to reflect current (v0.4.0+) behavior. |
| docs/projet-file/mcp-02-spec.md | Updates roadmap section wording to reflect shipped vs planned items. |
| docs/DDS_QUICKSTART.md | Updates QoS expansion reference from v0.3.x to v0.5.x patches. |
| .env.example | Updates DDS backend value list and auto-detect chain documentation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| `list_participants` poll calls (no native at-discovery callbacks). | ||
| * `topic_metrics` — opportunistic frequency / sequence-gap / latency | ||
| metrics buffered as `peek_dds_samples` surfaces samples (no native | ||
| at-sample-receive callback in cyclonedds 2.6.x Python). |
Comment on lines
12
to
+13
| from topicforge.adapters.base import AdapterError, AdapterName, MiddlewareAdapter | ||
| from topicforge.constants import MAX_SAMPLE_COUNT |
Comment on lines
343
to
346
| `dds-cyclone`, `dds-fast`, `dds` (union). Reframe DDS module | ||
| positioning around OMG-DDS-RTPS multi-vendor — see | ||
| `topicforge/docs/dds-interop-matrix.md` for the canonical | ||
| statement and the OMG May 2025 interop reference. Same 3 MCP |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Eight items found in a /effort max second-pass after v0.5.0 shipped. Zero behavior change, zero schema change, zero new MCP tool. House-keeping commit.
Summary
.env.example—TOPICFORGE_DDS_BACKENDlist andautochain refreshed from v0.3.0 (Fast > Cyclone > Mock) to v0.4.0 Phase 1.5 (8-vendor + Pro tier ordering).MAX_SAMPLE_COUNTconsolidation — two adapters had re-declared the constant locally (ros2_mock/adapter.py,bag_service.py), exactly the smell audit A4 prevented. Canonical home moved fromservices/constants.pytotopicforge/constants.py(root-level, layer-neutral —services/constants.pytriggered a circular load when imported fromadapters/). Dropped the v0.1.x re-export shim fromservices/inspector.py__all__.dds_cyclone/adapter.pyanddds_fast/adapter.pymodule docstrings refreshed from the v0.3.0 "raise on user topics" framing to the v0.4.0 reality (_decode_statusannotation,participant_events,topic_metrics).DDS_QUICKSTART.mdandmcp-02-spec.mdrevisited : shipped items rewritten as past-tense, still-pending QoS expansion re-tagged v0.5.x.The four remaining
v0.3.xstrings inMIGRATION_v0.3_to_v0.4.md,audit-post-v0.4.0.md, andarchive/launch-posts-v0.3.0/are legitimate historical references (quotes of old error messages, audit narrative, frozen marketing copy) — kept as-is.Test plan
Backward-compatibility checklist
Notes for the maintainer