Skip to content

Python: add test for empty-message pruning in approval result replacement - #5617

Merged
Eduard van Valkenburg (eavanvalkenburg) merged 1 commit into
microsoft:feature/python-fidesfrom
eavanvalkenburg:fix/fides-empty-message-pruning-test
May 4, 2026
Merged

Python: add test for empty-message pruning in approval result replacement#5617
Eduard van Valkenburg (eavanvalkenburg) merged 1 commit into
microsoft:feature/python-fidesfrom
eavanvalkenburg:fix/fides-empty-message-pruning-test

Conversation

@eavanvalkenburg

Copy link
Copy Markdown
Member

Description

Adds the missing test coverage requested in this review comment on PR #5331.

The second pass in _replace_approval_contents_with_results removes messages whose contents list becomes empty after first-pass content removal. The new test test_replace_approval_contents_with_results_prunes_emptied_messages exercises that path: a user message containing only approval responses whose placeholders were already replaced is fully consumed in the first pass, leaving an empty contents list — the second pass must prune it from messages.

Contribution Checklist

  • The code builds clean without any errors or warnings
  • I have added tests that prove my fix is effective or that my feature works

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

…ment

Adds test coverage for the second-pass logic in
`_replace_approval_contents_with_results` that removes messages whose
`contents` list becomes empty after first-pass content removal.

Addresses review comment on PR microsoft#5331:
microsoft#5331 (comment)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@moonbox3 Evan Mattson (moonbox3) added the python Usage: [Issues, PRs], Target: Python label May 4, 2026
@eavanvalkenburg
Eduard van Valkenburg (eavanvalkenburg) merged commit 7d4c372 into microsoft:feature/python-fides May 4, 2026
5 checks passed

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Automated Code Review

Reviewers: 4 | Confidence: 94% | Result: All clear

Reviewed: Correctness, Security Reliability, Test Coverage, Design Approach


Automated review by eavanvalkenburg's agents

Giles Odigwe (giles17) pushed a commit to giles17/agent-framework that referenced this pull request May 5, 2026
…5331)

* Python: Information-flow control based prompt injection defense (microsoft#5024)

* fides integration

* documentation

* documentation

* documentation

* human-approval on policy violation

* numenous hyena 'works'

* IFC based implementation

* minor edits in documentation

* rebasing the branch and running the email example

* Add security tests for IFC middleware

* Fix Role.TOOL NameError in approval handling

* tiered labelling scheme

* 3 tier labelling scheme in middleware

* Adapt security middleware to list[Content] tool results

* Refactor SecureAgentConfig as context provider and address Copilot review comments

* Update FIDES docs to reflect context provider pattern and update code for ContextProvider rename

* Fix security examples: use OpenAIChatClient instead of non-existent AzureOpenAIChatClient

* Address PR review: consolidate security modules, remove ContentLineage, update docs

* remove unrelated files

* remove comment from _tools.py and rename decision file

* Fix CI failures: Bandit B110, broken md links, hosted approval passthrough

* apply template to decision doc 0024

* minor fixes to decision doc 0024

---------

Co-authored-by: Aashish <t-akolluri@microsoft.com>

* Python: follow up FIDES security flow (microsoft#5330)

* Python: follow up FIDES security flow

Refine the secure approval path, mark the security classes with the FIDES experimental feature label, and clean up the related docs/tests. Also fix workspace-level validation regressions uncovered while running the full Python check suite.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Python: remove FIDES GitHub MCP sample

Drop the GitHub MCP security sample from the FIDES follow-up branch while keeping the remaining security docs and samples intact.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Address PR review: fix paths and update FIDES implementation (microsoft#5352)

* Python: updated import naming and comment from review (microsoft#5421)

* updated import naming and comment from review

* Add approval replay None call-id test

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Python: Address PR 5331 comments and track sesssion while calling Agent in email_security_example (microsoft#5446)

* Address PR review: fix paths and update FIDES implementation

* Address PR comments and add session tracking in email example in samples

* Fix session creation and resolve merge conflict in docstring example

* Resolve merge conflict in docstring example

* Python: add test for empty-message pruning in approval result replacement (microsoft#5617)

Adds test coverage for the second-pass logic in
`_replace_approval_contents_with_results` that removes messages whose
`contents` list becomes empty after first-pass content removal.

Addresses review comment on PR microsoft#5331:
microsoft#5331 (comment)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: shrutitople <shruti.tople@gmail.com>
Co-authored-by: Aashish <t-akolluri@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@eavanvalkenburg
Eduard van Valkenburg (eavanvalkenburg) deleted the fix/fides-empty-message-pruning-test branch June 30, 2026 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants