Skip to content

Feat/orm search#280

Draft
kfsone wants to merge 1 commit intorelease/v1from
feat/orm-search
Draft

Feat/orm search#280
kfsone wants to merge 1 commit intorelease/v1from
feat/orm-search

Conversation

@kfsone
Copy link
Collaborator

@kfsone kfsone commented Jan 23, 2026

wip - I still need to round out the tests, but if either of you wants visibility before then.

  • I put it in db because that's how I think, what I was avoiding was blowing trade.py away with this specific sub-concern;
  • my first next alternative was having "tradeorm" be a directory rather than a single file, so it could be broken up accordingly, but in that case it felt like it should just be called "db" back to square one :)
  • why did I think "well this isn't tradeorm"? Just hits me as not actually an ORM specific concern per-se.

@kfsone kfsone force-pushed the feat/orm-search branch 2 times, most recently from fc561d1 to 13b214c Compare January 24, 2026 02:15
@Tromador
Copy link
Collaborator

If you think it's better in db, I am fine with that. Especially if there's likely to be any db specific code for either backend.

My original thinking was to keep the db mess out of the pure TD codebase, so that any incoming dev could work on algorithms and other improvements to the way we work without needing to involve the DB, or should the database get in the way, it's all in one place and one doesn't have to go a-grepping to find all the touchpoints. This was something I had to do at length when I did the refactor, and knowing how long it took just to find all the places where there was raw SQL and such, I'd like for future me (or you, or whomsoever) to be able to avoid that pain next time.

So don't worry about using that folder if you think it appropriate, I don't own it.

@kfsone
Copy link
Collaborator Author

kfsone commented Jan 27, 2026

If you think it's better in db, I am fine with that. Especially if there's likely to be any db specific code for either backend.

My original thinking was to keep the db mess out of the pure TD codebase, so that any incoming dev could work on algorithms and other improvements to the way we work without needing to involve the DB, or should the database get in the way, it's all in one place and one doesn't have to go a-grepping to find all the touchpoints. This was something I had to do at length when I did the refactor, and knowing how long it took just to find all the places where there was raw SQL and such, I'd like for future me (or you, or whomsoever) to be able to avoid that pain next time.

nod I'm calling it out to check with someone more knowledgeable/appropriate whether I picked the right cut-point for separation of concerns. db/search.py as it is mostly just builds queries using the orm; but I can totally see how that might be the wrong line.

Otoh - I wasn't sure if it might not end up being the portion of the feature that ends up with dialect specific quirks etc - I dunno - some magic wompawompaindex for ZoolookDB or something.

@kfsone kfsone marked this pull request as ready for review January 31, 2026 23:28
This provides backend orm-adjacent search methods for querying either
a table or a table + its grouping for fast exact/prefix matches or
for employing fuzzy logic.

TradeORM can then leverage these to implement it's alternatives to
things like lookupStation, lookupItem, etc.

At the same time, things like importers, journal code, etc, will have
a facade for such lookups without the overhead of fuzzy search.

chore: local notebooks are local

test: add some additional tests

feat: wire db.search into tradeorm
@kfsone kfsone marked this pull request as draft February 7, 2026 20:13
@kfsone
Copy link
Collaborator Author

kfsone commented Feb 7, 2026

Ok - I'm going to integrate the game journal stuff, too, so that ~ and ~@ work anywhere you use a system or station name. But I'm going to think about it a bit first because I want to avoid creating a dependency mess.

@eyeonus
Copy link
Owner

eyeonus commented Feb 7, 2026 via email

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.

3 participants