fix(render): label session endpoints PEER/LOCAL to match MRT naming - #341
Merged
Conversation
The `--format text` block header printed `FROM:`/`TO:` for the MRT record's peer and local endpoints. MRT (RFC 6396) calls these fields peer and local, and the underlying model already uses `peer_ip`/`peer_asn` and `local_ip`/`local_asn`, so the labels now match the vocabulary the values come from. Rendered values are unchanged. Legacy type-5 state-change records, which carry only the peer endpoint, print a single `PEER:` line as before (`FROM:`). Tests and the changelog are updated; the text fixture in `renders_layered_update_block` pins the new labels.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
State-change endpoint labels need explicit regression assertions.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Aligns text-rendered MRT session endpoints with the model’s peer/local terminology.
Changes:
- Renames
FROM/TOlabels toPEER/LOCAL. - Updates documentation, tests, and changelog.
File summaries
| File | Description |
|---|---|
src/render/text.rs |
Updates endpoint labels and related tests. |
CHANGELOG.md |
Documents the wording change. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #341 +/- ##
=======================================
Coverage 91.50% 91.50%
=======================================
Files 103 103
Lines 26483 26487 +4
=======================================
+ Hits 24232 24236 +4
Misses 2251 2251 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Copilot review on #341: the legacy and BGP4MP state-change branches renamed their endpoint labels without a regression assertion, so only message records protected the PEER/LOCAL contract. Assert both endpoints on the BGP4MP state change, and the single PEER line (with no LOCAL line) on the legacy one.
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.
Follow-up to the
--format textoutput shipped in v0.22.0.The block header labelled the two MRT endpoints
FROM:/TO:, which does not say which end is which. MRT (RFC 6396) names those fields peer and local, and that is also what the model carries (peer_ip/peer_asn,local_ip/local_asn), so the rendered labels now use the same vocabulary:Rendered values, indentation and every other line are unchanged. Legacy type-5 state-change records carry only the peer endpoint and keep printing a single line (now
PEER:).The format is one day old and explicitly human-oriented rather than a stable machine interface, so this is a wording fix rather than a compatibility break; it is listed under
## Unreleased→### Changedin the changelog.