Skip to content

Pattern 35: doc fixes from Slingshot review#264

Open
didhd wants to merge 1 commit into
aws-samples:mainfrom
didhd:slingshot-fixes-pattern-35
Open

Pattern 35: doc fixes from Slingshot review#264
didhd wants to merge 1 commit into
aws-samples:mainfrom
didhd:slingshot-fixes-pattern-35

Conversation

@didhd
Copy link
Copy Markdown
Contributor

@didhd didhd commented May 27, 2026

Summary

Follow-up to #263. Applies the documentation fixes that came out of the AWS Slingshot scan against multimodal-understanding/repeatable-patterns/35-hybrid-vision-spatial-reasoning/.

README.md

  • Split the How to run step that combined "run pipeline" and "write outputs" into separate numbered steps.
  • Expanded the absolute claim "any document" to a qualified list ("many documents — real estate listings, personnel directories, magazine spreads, product catalogs").
  • Added a Conclusion section that summarizes the pattern and lists next steps.
  • Expanded the first mention of S3 to Amazon Simple Storage Service (Amazon S3) and added an explicit permanent-deletion warning to the cleanup instructions.
  • Added a cleanup note for the local results/ directory.
  • Aligned the SageMaker reference in the cleanup section with the AWS naming registry (Amazon SageMaker AI instances). The Studio reference under setup stays as-is — Amazon SageMaker Studio is the current official feature name and is intentional.

01_yearbook_name_face_matching.ipynb

  • Split the Setup section into a dedicated Prerequisites section followed by Setup, listing IAM permission, model access, and Python prerequisites explicitly.
  • Replaced the bare display(IPyImage(...)) calls with display(Markdown('![alt-text](path)')) so the rendered images carry descriptive alt text for screen readers.
  • Added a Conclusion section.
  • Softened the 230 tokens per image pricing claim to a generic reference to the Amazon Bedrock pricing page so the doc does not pin a specific tier.
  • Expanded the first mentions of IAM and Bedrock Batch Inference to use the full service name.
  • Added a cleanup note for the local results/ directory.

Test plan

  • Notebook still runs end-to-end against us-west-2 with us.amazon.nova-2-lite-v1:0 and us.anthropic.claude-sonnet-4-6.
  • All three sample pages produce 100% name-to-face associations (20/20, 5/5 with face 0 correctly unmatched, 8/8) — verified after the prompt and notebook edits.
  • Notebook outputs cleared before commit.
  • No real student data; sample names remain synthetic.

- README: split how-to-run step 4 into separate steps, expand "any document" to "many documents (real estate, directories, etc.)", add Conclusion section, expand S3 first mention to "Amazon Simple Storage Service (Amazon S3)" with permanent-deletion warning, add results/ directory cleanup note, switch SageMaker reference to "Amazon SageMaker AI instances" in cleanup.
- Notebook: split Setup into separate Prerequisites + Setup sections, attach descriptive alt text to the sample-page and visualization image renders, add Conclusion section, soften the 230-tokens-per-image pricing claim and link to the Bedrock pricing page, expand IAM and Bedrock Batch Inference first mentions, add results/ directory cleanup note.
@review-notebook-app
Copy link
Copy Markdown

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

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