Fix parameter search caching and seeding filter #15
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Two critical fixes for the MCP workflow: 1. Remove caching from /parameters endpoint - The @cache decorator was ignoring the search parameter - All queries returned the same cached result - Search functionality was completely broken 2. Seed ALL parameters, not just labeled ones - Previous filter excluded core tax system parameters - UK basic_rate and similar parameters were missing - Users couldn't create policies for fundamental reforms These changes enable the complete UK basic rate analysis workflow. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
|
@PolicyEngine the filter on labelled params was deliberate, remove that bit of this PR |
Author
|
Done! Reverted the seeding filter to only include labeled parameters (commit 1121257). The PR now only contains the cache fix for the |
As requested by @nikhilwoodruff, revert the seeding filter change while keeping the cache fix for the /parameters endpoint. The filter was deliberately designed to only include user-facing parameters (those with labels), not all parameters. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
Testing the deployed API after PR #13 revealed two critical issues preventing the MCP workflow:
1. Search functionality broken
The
/parametersendpoint had@cache(expire=3600)which cached results without considering query parameters. Searching for?search=basic_ratereturned all parameters instead of filtered results.2. Missing core UK parameters
The seeding script filtered parameters to only include those with labels:
This excluded fundamental UK tax system parameters like
gov.hmrc.income_tax.rates.uk[0].rate. The database only had 82 UK parameters (mostly contrib/* policy levers), making the workflow incomplete.Solution
Cache fix: Removed the
@cachedecorator from the parameters endpoint. The search parameter is now properly respected.Seeding fix: Changed to seed ALL parameters:
This ensures core tax system parameters are available for policy creation.
Testing needed
After deployment:
/parameters/?search=basic_rateshould return filtered resultsmake db-reset-prod && make seedto re-seed with all parametersNote
The production database will need to be re-seeded after this PR merges to get the missing parameters.
🤖 Generated with Claude Code