Skip to content

Conversation

@lorchda
Copy link

@lorchda lorchda commented Nov 12, 2025

Issue #145

Description of changes:

For the configuration rvl-cdip-package-sample-with-few-shot-examples:

  • fix broken paths
  • fix classification prompt (add classes)
  • fix extraction prompt (add few-shot examples)

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

@rstrahan rstrahan changed the base branch from main to develop November 12, 2025 14:18
Copy link
Contributor

@rstrahan rstrahan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much.. I left a couple of comments..
In general it will be much easier to understand and review if you can first create Issues to describe bugs/defects that you encounter, and then create a clean PR per issue to resolve.. This significantly limits the 'blast radius' of each PR, make is easier/safer to review, and we'll get it tested and merged faster.
Is that possible?

"output_tokens": 0,
"invocation_count": 0,
"total_cost": 0.0,
f"Extraction/{self.config.extraction.model}": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please report a separate Issue to clarify the problem that this is intended to fix? It's not clear to me that we have a bug in metering (looks OK in the UI and metering Athena table), so need clarity here.

"metadata": {},
"source": [
"## 1. Install Dependencies\n",
"## 1. Setup AWS Access"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again I am uncertain what issue this resolves.. and why it would be specific to the notebook? Any notebook should work already based on aws credential provider chain (typically local aws configure profile).. Can you log a separate issue if there is a bug, that this is intended to fix. Tx.

@lorchda lorchda force-pushed the chore/few-shot-examples branch from 9b4b1e2 to d426f5f Compare November 27, 2025 15:15
@rstrahan
Copy link
Contributor

rstrahan commented Dec 3, 2025

@lorchda Any updates on this PR / path to resolve comments above? Also need to resolve new merge conflicts.
Many tx,

@lorchda lorchda force-pushed the chore/few-shot-examples branch from d426f5f to 5907159 Compare December 4, 2025 08:20
@lorchda
Copy link
Author

lorchda commented Dec 4, 2025

@rstrahan the PR should now apply cleanly.

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.

2 participants