fix(BA-6887): reference the real from_status/to_status columns in kernel history conditions#12857
Merged
Merged
Conversation
github-actions
Bot
added
size:M
jopemachine
force-pushed
the
fix/kernel-scheduling-history-status-columns
branch
2 times, most recently
from
July 15, 2026 02:19
a10d727 to
7c1e0a9
Compare
jopemachine
changed the title
jopemachine
marked this pull request as ready for review
July 15, 2026 03:10
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes latent kernel scheduling-history filters to reference the actual status columns.
Changes:
- Renames kernel filter factories to status-based names.
- Removes casts masking invalid column references.
- Adds a fix changelog entry.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
conditions.py |
Corrects kernel status filter conditions. |
12857.fix.md |
Documents the fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| @staticmethod | ||
| def by_from_phase(phase: KernelSchedulingPhase) -> QueryCondition: | ||
| def by_from_status(phase: KernelSchedulingPhase) -> QueryCondition: |
…nel history conditions KernelSchedulingHistoryConditions.by_from_phase/by_to_phase referenced KernelSchedulingHistoryRow.from_phase and .to_phase. Those columns exist under different names: the table defines from_status and to_status, so either factory would have raised AttributeError once its condition was evaluated. The names drifted from an incomplete rename. BA-3061 first created kernel_scheduling_history with from_phase/to_phase columns. BA-3062 then renamed them to from_status/to_status by editing that not-yet-released migration in place, but the query conditions added in the same change kept the old names. The stale reference later survived the move from repositories/scheduling_history/options.py into this module (BA-5127). Two things kept it hidden: nothing calls either factory yet, so the closure was never evaluated; and the cast() wrapper kept the bad attribute access away from the type checker. Rename to by_from_status/by_to_status to match both the column names and the equivalent session factories, and drop the now-unneeded cast(). Renaming outright is safe as there are no callers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jopemachine
force-pushed
the
fix/kernel-scheduling-history-status-columns
branch
from
July 15, 2026 03:32
7c1e0a9 to
f6bd077
Compare
…conditions KernelSchedulingHistoryRow defines phase, from_status and to_status. Renaming by_from_phase/by_to_phase onto the real from_status/to_status columns left the phase column with no condition targeting it at all. The session, deployment and route condition classes each expose the full by_phase_contains/equals/starts_with/ends_with/in set; kernel was the only history class missing it. Add the same set against KernelSchedulingHistoryRow.phase, matching the existing implementations including case_insensitive and negated handling, and placing by_phase_in after the string filters as the other classes do. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jul 15, 2026
The kernel scheduling-history condition factories had no unit coverage, and the closest existing test (test_search_kernel_history_with_status_transition) filters only by kernel id and asserts the returned row's fields, so it never evaluates the status condition closures. That is why the stale from_phase / to_phase column references survived. Compile each factory to SQL and assert the exact predicate, covering the case_insensitive and negated combinations of every phase string filter since that branch logic is duplicated per factory. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
fregataa
approved these changes
Jul 15, 2026
jopemachine
added a commit
that referenced
this pull request
Jul 15, 2026
by_from_status, by_to_status and by_error_code have no production caller. Every filter reaches the conditions through the adapter, which uses the plural and MatchSpec factories instead: by_from_statuses / by_to_statuses for status, and convert_string_filter(equals_factory=by_error_code_equals, ...) for error_code. The only thing referencing the three was the test written for them. They are a strict subset of what replaces them — by_error_code_equals with a non-negated, case-sensitive spec compiles to the same `error_code = 'ERR'`, and also covers the case-insensitive and negated variants the plain one cannot. The from_status / to_status cases are dropped outright: by_from_statuses and by_to_statuses already pin the same columns. The error_code case moves onto by_error_code_equals rather than going away, so the column stays covered — a factory returns a lazy closure, so a wrong column only fails when compiled, which is the fault #12857 fixed and the reason these tests compile each condition. The identical factories on the session / deployment / route classes are equally dead but out of scope here.
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
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.
📚 Stacked PRs
BA-6887fix: kernel history conditions reference the real status columns ← you are hereBA-6889models: kernel query conditions and ordersBA-6890repository: kernel search scope and scoped searchBA-6891DTO + service: kernel v2 DTOs and search actionsBA-6892adapter + REST v2: kernel endpointsBA-6893SDK + CLI: kernel commandsMerge in order, bottom-up. Each PR is based on the one above it, so its diff shows only its own layer.
Problem
KernelSchedulingHistoryConditions.by_from_phase/by_to_phasereferencedKernelSchedulingHistoryRow.from_phaseand.to_phase. Those columns exist, but under different names — thekernel_scheduling_historytable definesfrom_statusandto_status. Both factories return a lazily-evaluated closure, so either would have raisedAttributeErrorthe moment its condition was actually evaluated.How the names drifted
An incomplete rename, spread over three changes:
1a2316bb2(BA-3061)kernel_scheduling_historywithfrom_phase/to_phasecolumns.dd85bdbe2(BA-3062)from_status/to_status, by editing that not-yet-released migration in place. The query conditions added in the same change kept the old names.34af67f18(BA-5127)repositories/scheduling_history/options.pyintomodels/scheduling_history/conditions.py— the stale reference came along.Two things kept it hidden:
cast()hid it from the type checker. The bad attribute access sat inside acast(...), so the lookups were never flagged.Fix
from_status/to_statuscolumns.by_from_status/by_to_status, matching both the column names and the equivalentSessionSchedulingHistoryConditions.by_from_status/by_to_status. Safe to rename outright: there are no callers.cast(), which was only masking the error.Scope of impact
No user-visible behavior changes. Nothing calls these factories, so the fault was unreachable — this clears a latent fault rather than a live one. Found while implementing the kernel scheduling-history read stack (BA-6882), which is the first code to call them; split out because it stands on its own and is not scoped to that story.
🤖 Generated with Claude Code