Skip to content

refactor: sanitize Brokkr response by filtering invalid locations in parseOpportunity function #3387

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
idoshamun merged 1 commit into from
Jan 1, 2026
Merged

refactor: sanitize Brokkr response by filtering invalid locations in parseOpportunity function #3387

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions src/common/opportunity/parse.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { FastifyBaseLogger } from 'fastify';
import { DataSource, DeepPartial, IsNull, Not } from 'typeorm';
import { fileTypeFromBuffer } from 'file-type';
import {
Location,
LocationType,
OpportunityContent,
OpportunityState,
Expand Down Expand Up @@ -199,9 +200,19 @@ export async function parseOpportunityWithBrokkr(

log.info(result, 'brokkrParseOpportunityResponse');

const parsedOpportunity = await opportunityCreateParseSchema.parseAsync(
result.opportunity,
);
// Sanitize Brokkr response - filter out invalid locations
const sanitizedOpportunity = {
...result.opportunity,
location: Array.isArray(result.opportunity?.location)
? result.opportunity.location.filter((loc: Location) => {
// Only keep locations with valid country
return loc?.country && loc.country.trim().length > 0;
Copy link
Copy Markdown
Contributor

@capJavert capJavert Jan 2, 2026

Choose a reason for hiding this comment

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

think actually we should look at those with iso2, since that one is actually important for process down the line

Copy link
Copy Markdown
Member Author

@idoshamun idoshamun Jan 4, 2026

Choose a reason for hiding this comment

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

but i think country is the one we make strict, right?

Copy link
Copy Markdown
Contributor

@capJavert capJavert Jan 5, 2026

Choose a reason for hiding this comment

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

should be both, gondul looks only at ISO so it can't do anything with country name

Copy link
Copy Markdown
Member Author

@idoshamun idoshamun Jan 5, 2026

Choose a reason for hiding this comment

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

@claude fix it in a new branch

Copy link
Copy Markdown

@claude claude Bot Jan 5, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Claude finished @idoshamun's task —— View job • claude/pr-3387-20260105-0931 • Create PR ➔

✅ Fixed location filtering to check both country and iso2

Updated the location filtering logic in parseOpportunity function to validate both country and iso2 fields, ensuring gondul has valid ISO codes for downstream processing.

Todo List:

  • Read current implementation in parseOpportunity function
  • Update filtering logic to check both country and iso2 fields
  • Commit and push changes
  • Provide PR creation link

Changes Made:

  • Updated src/common/opportunity/parse.ts:209 to filter locations based on both country and iso2 validation
  • Added checks to ensure both fields are present and have non-empty trimmed values
  • This prevents invalid location data from reaching gondul which requires valid ISO codes

})
: [],
};

const parsedOpportunity =
await opportunityCreateParseSchema.parseAsync(sanitizedOpportunity);

const content = renderOpportunityMarkdownContent(parsedOpportunity.content);

Expand Down
Loading