Skip to content

docs: add message pinning section to conversation management page#2749

Merged
lizradway merged 1 commit into
strands-agents:mainfrom
lizradway:lizradway/message-pinning-docs
Jun 12, 2026
Merged

docs: add message pinning section to conversation management page#2749
lizradway merged 1 commit into
strands-agents:mainfrom
lizradway:lizradway/message-pinning-docs

Conversation

@lizradway

@lizradway lizradway commented Jun 12, 2026

Copy link
Copy Markdown
Member

Description

Add a "Message Pinning" section to the conversation management documentation page. Covers the new pin_first/pinFirst configuration parameter and runtime utility functions (pin_message/pinMessage, unpin_message/unpinMessage, is_pinned/isPinned) with examples for both Python and TypeScript. Adds cross-reference bullets in the SlidingWindow and Summarizing manager feature lists.

Related Issues

Documents the feature added in #2644

Documentation PR

Changes are under site/ in this PR.

Type of Change

Documentation update

Testing

  • Verified snippet inclusion tags match between .mdx and .ts files
  • I ran hatch run prepare

Checklist

  • I have read the CONTRIBUTING document
  • I have reviewed and understand every line of code in this PR, including any generated by AI tools, and I can explain why it works
  • My change is focused and reasonably small; I have split unrelated work into separate PRs
  • I have added any necessary tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly
  • I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@lizradway lizradway force-pushed the lizradway/message-pinning-docs branch from 1bf7d3f to b8f9e9b Compare June 12, 2026 15:11
@lizradway lizradway marked this pull request as ready for review June 12, 2026 15:11
@github-actions github-actions Bot added size/m and removed size/m labels Jun 12, 2026
@github-actions github-actions Bot added the enhancement New feature or request label Jun 12, 2026
@github-actions github-actions Bot added documentation Documentation changes, improvements, additions, content updates, site improvements, examples, guides area-context Session or context related size/m and removed size/m labels Jun 12, 2026
Comment thread site/src/content/docs/user-guide/concepts/agents/conversation-management.mdx Outdated
@github-actions

Copy link
Copy Markdown
Contributor

Assessment: Request Changes

The prose is clear and the conceptual description (metadata-based pinning, pin_first/pinFirst, tool-pair partner protection, one-time application on first reduction) accurately matches the implementation in both SDKs. The blocking concern is that the documented import paths for the runtime utility functions point to APIs that aren't publicly exported, so the examples won't work for readers as written.

Review themes
  • API accuracy (blocking): The pinMessage/unpinMessage/isPinned (TS) and pin_message/unpin_message/is_pinned (Python) functions are documented as importable public APIs, but they're not exported from any public entry point — TS imports the root package and a non-existent ./conversation-manager subpath, and Python deep-imports an internal module not in __all__. The two TS snippets also import from different paths. These need to become real, consistent public exports in the SDKs (or the runtime-utility section should be dropped) before the docs are correct.
  • Consistency: Whatever public surface is chosen should match across the Python and TypeScript tabs and between the snippet and imports files.
  • Scope/size: Focused, appropriately small docs-only change — good.

Nice, well-written addition overall — once the underlying functions are exported and the import paths are reconciled, this is in good shape.

@github-actions

github-actions Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Documentation Preview Ready

Your documentation preview has been successfully deployed!

Changed pages:

Updated at: 2026-06-12T19:11:46.408Z

Comment thread site/src/content/docs/user-guide/concepts/agents/conversation-management.mdx Outdated
@github-actions

Copy link
Copy Markdown
Contributor

Assessment: Request Changes

Thanks for documenting message pinning — the page is well-structured and the pinFirst/pin_first config sections are accurate and match the implementation. The blocker is that the runtime-utility examples reference an API that isn't publicly exported yet, so the snippets don't compile.

Review details
  • Broken TS imports (Critical): pinMessage / unpinMessage / isPinned aren't exported from @strands-agents/sdk. I built the SDK and ran typecheck:snippets (the CI gate) — it fails with TS2724/TS2305 on these three names. The _imports.ts variant also uses a @strands-agents/sdk/conversation-manager subpath that doesn't exist in package.json exports, and disagrees with the sibling snippet's import path.
  • Python deep import (Important): The Python example imports from the internal ...conversation_manager.pin_message module; those functions aren't in conversation_manager/__init__.py __all__, so it documents a private path as public.
  • API surface: Promoting these three functions to a documented public API is an API-review-worthy change (needs-api-review), not something to settle in a docs PR. Recommend deciding the public surface first, then documenting it.

Suggested path forward: either export the functions (TS barrel + Python __all__) and use one canonical import path each, or scope this PR to the verified pinFirst config and defer the runtime-pinning section. Re-running hatch run prepare / the snippet typecheck (currently unchecked in the PR template) will catch these before pushing.

The structure and the pinFirst coverage are solid — once the runtime API is actually exported, this is close to ready.

@lizradway lizradway force-pushed the lizradway/message-pinning-docs branch from c9cbb06 to 03228f5 Compare June 12, 2026 15:21
@github-actions github-actions Bot added size/m and removed size/m labels Jun 12, 2026
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Corrected Review — please read this instead of my earlier comments

My first review round was based on a stale local working tree that contained a version of the pinning feature; it did not reflect this PR. I re-checked out the actual PR branch (03228f5), built the SDK, and ran the real CI gate (typecheck:snippets). I also posted some comments twice — apologies for the noise. Here is the verified state.

Assessment: Request Changes (same verdict, corrected reasons)

Verified facts (PR branch + origin/main)

  • The pinning feature does not exist in the SDK this PR builds on:
    • No pinFirst in any TS conversation-manager config; no pin_first in Python.
    • No strands-py/.../conversation_manager/pin_message.py; no pinMessage/unpinMessage/isPinned / is_pinned anywhere in strands-py or strands-ts/src.
    • Confirmed the same on origin/main, so this isn't just a missing rebase of a merged feature.
  • npm run typecheck:snippets produces exactly 2 errors:
    1. conversation-management.ts:183TS2353: pinFirst does not exist in SlidingWindowConversationManagerConfig (real, caused by this PR).
    2. observability-evaluation/logs.ts:16 — TS2307 pino not found (pre-existing, unrelated).

What this means per file

  • conversation-management.ts: Uses local declare function stubs for the runtime helpers (so no import error there), but the pinFirst config snippet fails because the SDK config has no such field.
  • conversation-management_imports.ts: Has // @ts-nocheck, so its @strands-agents/sdk/conversation-manager import isn't typechecked — but that subpath still isn't a real package export and the symbols don't exist, so it's not a working example for users.
  • conversation-management.mdx (Python): Imports from strands.agent.conversation_manager.pin_message, a module that doesn't exist on this branch or main.

Path forward

This is fundamentally a sequencing problem: the docs are ahead of the SDK. Recommend one of:

  1. Land the SDK feature (feat(context): add message pinning to conversation managers #2644) first — pin_first/pinFirst config, the runtime functions, and a real public export path in both SDKs — then rebase this PR onto it and re-run typecheck:snippets; or
  2. Scope this PR down to only what the SDK currently supports (today that appears to be nothing pin-related), or mark it draft/blocked-on until feat(context): add message pinning to conversation managers #2644 merges.

Either way, the documented public API (import paths, config fields) must match the shipped SDK, and typecheck:snippets should pass before merge.

The prose and structure of the new section read well — once the SDK feature exists and is exported, this should come together quickly. Sorry again for the inaccurate first pass.

@github-actions

Copy link
Copy Markdown
Contributor

✅ Final Corrected Review — please disregard ALL my earlier review comments on this PR

I owe you a clear correction and an apology. Both of my earlier rounds were wrong, for two different environment reasons:

  1. Round 1 (~15:17–15:19) — reviewed a stale local working tree that didn't match this PR, and I accidentally posted some comments twice.
  2. Round 2 (~15:34) — my local checkout had gotten into a corrupted/mixed state (I'd swapped the SDK source dirs while investigating), which led me to incorrectly claim "the pinning feature doesn't exist in the SDK." That claim is false.

I re-verified against the authoritative GitHub PR diff at the current head (575a414). Here is the accurate state:

This PR is correct and self-consistent ✅

It is not docs-only — it correctly wires up the public exports alongside the docs:

Check Result
strands-ts/src/index.ts exports isPinned, pinMessage, unpinMessage ✅ added in this PR
strands-ts/src/conversation-manager/index.ts re-exports them ✅ added in this PR
strands-py/.../conversation_manager/__init__.py adds all three to __all__ ✅ added in this PR
Docs .ts + _imports.ts import from root @strands-agents/sdk ✅ matches the new exports
Docs .mdx Python imports from strands.agent.conversation_manager (package root) ✅ matches __all__
pinFirst / pin_first config field on both managers ✅ pre-exists (feature #2644)
Documented signatures match SDK (pinMessage(messages, index), etc.) ✅ verified
Snippet include-tags in .mdx ↔ defined in .ts/_imports.ts ✅ all resolve, none dangling

So the three "blocking" issues I originally raised (TS root import not exported, _imports.ts subpath, Python deep/private import) are all resolved in this PR — the import paths point at real public exports.

Note on local typecheck: I could not get a reliable typecheck:snippets run in my sandbox — even unmodified pre-existing files (e.g. agent-loop.ts) fail @strands-agents/sdk resolution there, which means it's a local file:-link/preserveSymlinks artifact, not a PR problem. The real CI gate on this PR is the authority.

Revised assessment: Approve (pending green CI)

Clean, well-scoped change; prose is clear and the public API is wired correctly across both SDKs. Sorry again for the noise and the incorrect earlier verdicts — please disregard the prior review comments entirely in favor of this one.

Comment thread site/src/content/docs/user-guide/concepts/agents/conversation-management.mdx Outdated
Cover the new pin_first/pinFirst configuration parameter and runtime
utility functions (pin_message, unpin_message, is_pinned) with examples
for both Python and TypeScript. Export pin functions from public API so
the documented imports resolve.
@lizradway lizradway enabled auto-merge (squash) June 12, 2026 19:08
@lizradway lizradway merged commit c749381 into strands-agents:main Jun 12, 2026
37 of 39 checks passed
@lizradway lizradway deleted the lizradway/message-pinning-docs branch June 12, 2026 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-context Session or context related documentation Documentation changes, improvements, additions, content updates, site improvements, examples, guides enhancement New feature or request size/m

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants