Merge origin/main: keep the substring rule, read its tie
main had moved on with retrieval work validated against real queries — minTokenHits (the word match needs two thirds of the label, not all of it), separator folding so "Parle G"/"Parle-G"/"ParleG" all reach Parle-G, the floor at 0.50 after "Paracetamol" came back as "Paneer Makhni 500ml" at 0.304, and ties broken on cosine distance instead of name. All of that is kept exactly as it was. The conflict was in textScore: this branch replaced the substring rule with a coverage formula to stop a bare brand name resolving to one arbitrary product. That is the wrong half to change. The substring rule scores every product of a brand 0.95 IDENTICALLY, and that tie is not the bug — it is the signal. isAmbiguous reads it, so the branch's coverage rewrite is dropped and the ambiguity layer alone does the work: "britannia" → all 258 rows tie at 0.95 → ambiguous: true + candidates "Parle G" → folding and the single-character token still land it a real name → runner-up far behind → match, unchanged Dropped with it: scanSpecificEnough, the per-hit text score, and the proportional confirmation bonus — the flat +0.10 is back. Simpler, and it leaves main's tuning untouched. TestTextScoreRewardsSpecificityNotJustOverlap tested the removed formula and is replaced by TestABrandNameScoresItsProductsIdentically, which guards the tie itself: a formula that broke it on name length or word count would bring the bug back. Docs carry both rationales, and now say plainly that confidence stays high on the ambiguous path — gate on `ambiguous`, never on `confidence`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -208,3 +208,35 @@ func TestNormaliseBrandKeyRefusesToInventAKey(t *testing.T) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Every word of the label was once required, so one word the catalogue does
|
||||
// not use ("Parle G biscuit pack") kept the right product out of the result
|
||||
// altogether and left the vector search to answer alone.
|
||||
func TestMinTokenHitsAsksForMostWordsNotAllOfThem(t *testing.T) {
|
||||
for _, tc := range []struct{ tokens, want int }{
|
||||
{1, 1}, // one word: it has to be there
|
||||
{2, 2}, // "Parle G" — both, and both are in Parle-G
|
||||
{3, 2}, // "Milk Bikis pack" — the pack is allowed to be missing
|
||||
{4, 3}, // "Parle G biscuit pack"
|
||||
{5, 4},
|
||||
{6, 4},
|
||||
} {
|
||||
if got := minTokenHits(tc.tokens); got != tc.want {
|
||||
t.Errorf("%d tokens: need %d, want %d", tc.tokens, got, tc.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// A threshold that could fall to 1 would let any single common word drag in
|
||||
// whole brand tables; one that stayed at n would be the bug all over again.
|
||||
func TestMinTokenHitsStaysBetweenTwoAndAll(t *testing.T) {
|
||||
for n := 3; n <= 30; n++ {
|
||||
got := minTokenHits(n)
|
||||
if got < 2 {
|
||||
t.Fatalf("%d tokens: %d is too loose", n, got)
|
||||
}
|
||||
if got >= n {
|
||||
t.Fatalf("%d tokens: %d still demands every word", n, got)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -87,30 +87,67 @@ func (r *partnerRepository) GetPartners(aid, pid, uid int) ([]models.Partnerinfo
|
||||
var q1 string
|
||||
var args []interface{}
|
||||
|
||||
// Every variant joins partnerlocations, and that join is the whole point.
|
||||
//
|
||||
// ── It is what separates our partners from somebody else's ──────────────
|
||||
//
|
||||
// `partnerinfo` is shared. It has no column saying which product a row
|
||||
// belongs to — no configid, no appid — so a partner created by another app
|
||||
// on this database is indistinguishable from ours by its own fields, and
|
||||
// this read used to return every Active row on the platform. The console
|
||||
// made that worse rather than better: it asks `getapplocations` for EVERY
|
||||
// region and then fetches partners region by region, so the applocationid
|
||||
// filter below never narrowed anything.
|
||||
//
|
||||
// `partnerlocations` is the difference. Only `CreatePartner` writes it —
|
||||
// one row per region, in the same transaction as the partner — so a row in
|
||||
// that table means "registered through this console". The partners that
|
||||
// predate it were inserted by hand and have none, which is why two of them
|
||||
// are called "Test".
|
||||
//
|
||||
// ── The region filter reads the link table, not the home region ─────────
|
||||
//
|
||||
// `partnerinfo.applocationid` is the HOME region — CreatePartner writes
|
||||
// `regions[0]` there — while partnerlocations holds every region covered.
|
||||
// Those are not the same thing, and not only in theory: partner 44,
|
||||
// Xpress-Cbe-Main, has a home region of 1 and link rows for 1 AND 2, so
|
||||
// filtering on the partner row hid them from every Madurai query. That is
|
||||
// the case the link table exists for.
|
||||
//
|
||||
// DISTINCT because such a partner has one row per region in the join and is
|
||||
// still one partner. Only partnerinfo columns are selected, so there is
|
||||
// nothing per-region for it to fail to collapse.
|
||||
//
|
||||
// A caller fanning out over regions and concatenating the answers still has
|
||||
// to dedupe — the same partner is legitimately in two of them. The console's
|
||||
// `useAllPartners` does; it listed Xpress-Cbe-Main twice until it did.
|
||||
const columns = `select distinct p.partnerid,p.applocationid,p.partnertypeid,p.partnername,
|
||||
p.primarycontact,p.primaryemail,p.contactno,p.address,p.suburb,p.state,p.city,p.partnerimage
|
||||
from partnerinfo p
|
||||
inner join partnerlocations l on l.partnerid = p.partnerid
|
||||
where p.status='Active'`
|
||||
|
||||
if pid != 0 {
|
||||
q1 = `select partnerid,applocationid,partnertypeid,partnername,primarycontact,primaryemail,
|
||||
contactno,address,suburb,state,city,partnerimage
|
||||
from partnerinfo where status='Active' and partnerid=?`
|
||||
// Scoped the same way on purpose: asking for a partner by id must not
|
||||
// be a way round the separation above.
|
||||
q1 = columns + ` and p.partnerid=?`
|
||||
args = append(args, pid)
|
||||
|
||||
} else if aid != 0 {
|
||||
q1 = `select partnerid,applocationid,partnertypeid,partnername,primarycontact,primaryemail,
|
||||
contactno,address,suburb,state,city,partnerimage
|
||||
from partnerinfo where status='Active' and applocationid=?`
|
||||
q1 = columns + ` and l.applocationid=?`
|
||||
args = append(args, aid)
|
||||
|
||||
} else {
|
||||
q1 = `select partnerid,applocationid,partnertypeid,partnername,primarycontact,primaryemail,
|
||||
contactno,address,suburb,state,city,partnerimage
|
||||
from partnerinfo where status='Active'`
|
||||
q1 = columns
|
||||
}
|
||||
|
||||
q1 += ` order by p.partnername, p.partnerid`
|
||||
|
||||
err := r.db.Raw(q1, args...).Find(&data).Error
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
print(q1)
|
||||
return data, nil
|
||||
}
|
||||
|
||||
@@ -615,13 +652,17 @@ them are named "Test".
|
||||
|
||||
Where a partner works is recorded twice, on purpose and not by accident:
|
||||
|
||||
partnerinfo.applocationid their home region — `GetPartners` filters on it
|
||||
and the rider app reads it
|
||||
partnerinfo.applocationid their home region — the rider app reads it
|
||||
partnerlocations every region they cover
|
||||
|
||||
Both are kept in step here. Writing only the first would confine a partner to
|
||||
one city, and writing only the second would hide them from every existing
|
||||
query. */
|
||||
one city, and writing only the second would hide them from the rider app.
|
||||
|
||||
`GetPartners` reads the SECOND: it joins partnerlocations, which both scopes a
|
||||
region query to every city a partner actually covers and — because only this
|
||||
function writes that table — separates partners registered here from the ones
|
||||
another product put in the shared `partnerinfo`. So the link rows are not
|
||||
bookkeeping; they are what makes a partner ours. */
|
||||
|
||||
// CreatePartner onboards a delivery partner and records the regions they cover.
|
||||
func (r *partnerRepository) CreatePartner(input models.NewPartner) (int, error) {
|
||||
|
||||
@@ -26,7 +26,6 @@ type ProductRepository interface {
|
||||
UpdateProductStatus(productIDs []int, status string) error
|
||||
SyncProductLocationStatus(refs []models.ProductLocationRef) error
|
||||
EnsureProductLocation(refs []models.ProductLocationRef) error
|
||||
CreateProduct(product models.Products) error
|
||||
UpdateProduct(product models.Products) error
|
||||
DeleteProduct(productID int) error
|
||||
GetStockStatement(tenantID, locationID, subcategoryID, pageno, pagesize int, keyword string) ([]models.Productstockstatement, error)
|
||||
@@ -393,19 +392,32 @@ func (r *productRepository) UpdateProductStatus(productIDs []int, status string)
|
||||
Update("productstatus", status).Error
|
||||
}
|
||||
|
||||
func (r *productRepository) CreateProduct(product models.Products) error {
|
||||
tx := r.db.Begin()
|
||||
|
||||
if err := tx.Create(&product).Error; err != nil {
|
||||
tx.Rollback()
|
||||
return err
|
||||
// normaliseProductJSON makes a product safe to INSERT.
|
||||
//
|
||||
// `products.productimages` is jsonb and `models.Products.Productimages` is a
|
||||
// plain string, so a caller that never set it hands GORM the zero value — and
|
||||
// GORM puts that empty string in the INSERT rather than omitting the column.
|
||||
// Postgres answers "invalid input syntax for type json (SQLSTATE 22P02)" and
|
||||
// the whole row is rejected, over a field nobody asked for.
|
||||
//
|
||||
// That was not a corner case: the console's sheet importer sends no
|
||||
// productimages at all, so EVERY product it created failed with a 500, and
|
||||
// ImportCatalogueProduct leaves the field empty for any catalogue product that
|
||||
// has no photos. An empty ARRAY is the honest value — there are no extra
|
||||
// images — and it is what `catalogueUploadService` already does for its own
|
||||
// jsonb column, for the same reason.
|
||||
//
|
||||
// Applied at the one create path, which is the last point before the SQL, and
|
||||
// the constraint being satisfied is the database's.
|
||||
func normaliseProductJSON(product *models.Products) {
|
||||
if strings.TrimSpace(product.Productimages) == "" {
|
||||
product.Productimages = "[]"
|
||||
}
|
||||
|
||||
if err := tx.Commit().Error; err != nil {
|
||||
return err
|
||||
// An OBJECT, not an array: this one holds named catalogue fields, and `{}`
|
||||
// is what a reader parsing it expects to find when there are none.
|
||||
if strings.TrimSpace(product.Cataloguefacts) == "" {
|
||||
product.Cataloguefacts = "{}"
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
func (r *productRepository) UpdateProduct(product models.Products) error {
|
||||
@@ -1316,10 +1328,22 @@ func (r *productRepository) FindTenantProductByCatalogueRef(tenantid int, brand
|
||||
return &product, nil
|
||||
}
|
||||
|
||||
// CreateProductReturningID inserts a new product snapshot and returns its
|
||||
// generated productid. Kept separate from CreateProduct so existing callers
|
||||
// of CreateProduct are unaffected.
|
||||
// CreateProductReturningID inserts a product and returns its generated
|
||||
// productid.
|
||||
//
|
||||
// This is now the only way to create one. There used to be a second method,
|
||||
// `CreateProduct`, that did the same INSERT and threw the id away — it took
|
||||
// the struct by value, so GORM wrote the generated id onto a copy that went
|
||||
// out of scope, and `POST /products/create` answered `productid: 0` for every
|
||||
// product it had just created. The console worked around it by creating, then
|
||||
// re-reading the whole tenant catalogue, then matching back by SKU.
|
||||
//
|
||||
// The two were kept apart so that "existing callers are unaffected", but the
|
||||
// only caller of the id-less one was the endpoint that needed the id most.
|
||||
// One create path also means the jsonb guard above has one place to live.
|
||||
func (r *productRepository) CreateProductReturningID(product models.Products) (int, error) {
|
||||
normaliseProductJSON(&product)
|
||||
|
||||
if err := r.db.Create(&product).Error; err != nil {
|
||||
return 0, err
|
||||
}
|
||||
|
||||
@@ -470,9 +470,22 @@ func (r *scanRepository) VectorSearch(ctx context.Context, vector []float32, lim
|
||||
return hits, nil
|
||||
}
|
||||
|
||||
// minTokenHits is how many of the label's words a row must carry to be worth
|
||||
// looking at. Every word was once required, which meant a single word the
|
||||
// catalogue does not use — "Parle G biscuit pack", "Milk Bikis pack" — kept
|
||||
// the right product out of the result entirely, leaving the vector search to
|
||||
// answer alone and confidently wrong. Most of them is enough; scoring sorts
|
||||
// out the rest.
|
||||
func minTokenHits(n int) int {
|
||||
if n <= 2 {
|
||||
return n
|
||||
}
|
||||
return (n*2 + 2) / 3 // two thirds, rounded up; never below 2 for n >= 3
|
||||
}
|
||||
|
||||
// TextSearch is the fallback when there is no embedder, and the tie-breaker
|
||||
// beside it when there is: rows whose name or title contains the label, or
|
||||
// contains every word of it.
|
||||
// carry most of its words.
|
||||
func (r *scanRepository) TextSearch(ctx context.Context, label string, limit int) ([]CatalogueHit, error) {
|
||||
tables, err := r.brandTables(ctx)
|
||||
if err != nil {
|
||||
@@ -498,18 +511,32 @@ func (r *scanRepository) TextSearch(ctx context.Context, label string, limit int
|
||||
hay = "LOWER(COALESCE(product_name, '') || ' ' || COALESCE(title, '') || ' ' || COALESCE(search_query, ''))"
|
||||
}
|
||||
|
||||
conds := []string{hay + " LIKE ?"}
|
||||
args = append(args, "%"+label+"%")
|
||||
all := make([]string, 0, len(tokens))
|
||||
for _, tok := range tokens {
|
||||
all = append(all, hay+" LIKE ?")
|
||||
args = append(args, "%"+tok+"%")
|
||||
// How well a row matches, as a number: the whole label as a substring
|
||||
// outweighs any number of loose words, then one point per word found.
|
||||
hits := make([]string, 0, len(tokens)+1)
|
||||
hits = append(hits, "(CASE WHEN "+hay+" LIKE ? THEN 100 ELSE 0 END)")
|
||||
for range tokens {
|
||||
hits = append(hits, "(CASE WHEN "+hay+" LIKE ? THEN 1 ELSE 0 END)")
|
||||
}
|
||||
conds = append(conds, "("+strings.Join(all, " AND ")+")")
|
||||
rank := strings.Join(hits, " + ")
|
||||
|
||||
// The expression appears twice in the SQL — once to filter, once to
|
||||
// order — so its arguments are bound twice, in that order.
|
||||
bind := func() {
|
||||
args = append(args, "%"+label+"%")
|
||||
for _, tok := range tokens {
|
||||
args = append(args, "%"+tok+"%")
|
||||
}
|
||||
}
|
||||
bind()
|
||||
bind()
|
||||
|
||||
// Ordering matters as much as the threshold: a looser WHERE lets more
|
||||
// rows qualify, and an unordered LIMIT would then be free to return
|
||||
// the wrong ones. Best match per brand first, id to keep it stable.
|
||||
branches = append(branches, fmt.Sprintf(
|
||||
`(SELECT %s, -1::float8 AS distance FROM %s WHERE %s LIMIT %d)`,
|
||||
hitColumns(brand, cols), table, strings.Join(conds, " OR "), limit))
|
||||
`(SELECT %s, -1::float8 AS distance FROM %s WHERE (%s) >= %d ORDER BY (%s) DESC, id LIMIT %d)`,
|
||||
hitColumns(brand, cols), table, rank, minTokenHits(len(tokens)), rank, limit))
|
||||
}
|
||||
if len(branches) == 0 {
|
||||
return nil, nil
|
||||
|
||||
@@ -85,7 +85,22 @@ func (r *tenantRepository) GetAllTenants(pageno, pagesize, aid int, status, tena
|
||||
|
||||
var data []models.Tenantinfo
|
||||
|
||||
base := `SELECT * FROM tenants a WHERE 1 = 1`
|
||||
// `branchcount` is selected here because there is nowhere else to get it.
|
||||
//
|
||||
// This returns one row per TENANT — there is no join to tenantlocations at
|
||||
// all — but the console's store list read it as one row per
|
||||
// tenant-location pair and counted the duplicates, so every merchant on the
|
||||
// platform showed exactly one branch, and the "Branches" and "Avg branches"
|
||||
// tiles above the list were the tenant count wearing another name. The
|
||||
// tenant's own detail page, which reads gettenantlocations, disagreed with
|
||||
// the list it was opened from.
|
||||
//
|
||||
// A correlated subquery rather than a LEFT JOIN + GROUP BY: the row shape
|
||||
// stays exactly as it was, so nothing else that reads this endpoint has to
|
||||
// change, and every filter below still applies to `a` alone.
|
||||
base := `SELECT a.*,
|
||||
(SELECT COUNT(*) FROM tenantlocations tl WHERE tl.tenantid = a.tenantid) AS branchcount
|
||||
FROM tenants a WHERE 1 = 1`
|
||||
|
||||
var (
|
||||
conds []string
|
||||
@@ -337,7 +352,12 @@ func (r *tenantRepository) GetStaffs(tid int) ([]models.StaffInfo, error) {
|
||||
a.state,a.postcode,a.userfcmtoken,a.pin,a.applocationid,
|
||||
a.roleid,a.partnerid,a.tenantid,a.locationid,
|
||||
b.locationname,
|
||||
COALESCE(c.rolename,'') AS rolename
|
||||
COALESCE(c.rolename,'') AS rolename,
|
||||
-- Whether the account still works. Absent from this SELECT
|
||||
-- until now, so Users & access had nothing to read and showed
|
||||
-- every person on the platform as "Unknown" — an admin could not
|
||||
-- tell a working login from one that had been switched off.
|
||||
COALESCE(a.status,'') AS status
|
||||
FROM app_users a
|
||||
LEFT JOIN tenantlocations b ON a.locationid = b.locationid
|
||||
LEFT JOIN app_roles c ON c.roleid = a.roleid
|
||||
@@ -625,6 +645,51 @@ func (r *tenantRepository) CreateTenantUser(data models.Tenants) (bool, error) {
|
||||
var custloc models.Customerlocations
|
||||
var tcust models.Tenantcustomers
|
||||
|
||||
// A tenant with configid 0 is unreachable, and it takes its customer row
|
||||
// with it.
|
||||
//
|
||||
// Step 3 below already forces `user.Configid = 1`, with a comment
|
||||
// explaining that AppLogin only ever queries configid 1 and a zero makes
|
||||
// the account permanently unfindable. The same zero was left to flow into
|
||||
// `tenants` itself and into the `customers` row copied from it at step 4,
|
||||
// where nothing corrected it — so a caller that omits configid (the console
|
||||
// sends it; the mobile route and anything else need not) created a business
|
||||
// and a customer that no scoped read can see.
|
||||
//
|
||||
// Defaulted rather than rejected: 1 is the only value any caller has ever
|
||||
// meant here, and refusing the create would break callers that work today.
|
||||
if data.Configid == 0 {
|
||||
data.Configid = 1
|
||||
}
|
||||
|
||||
// Give the primary outlet the scaffolding the tenant already has.
|
||||
//
|
||||
// The outlet itself is created by GORM, as the `Tenantlocations`
|
||||
// association on the struct below — the console nests a full object in the
|
||||
// request and step 1 saves it with the tenant. What it does NOT do is fill
|
||||
// anything the caller left out, and two of those columns matter:
|
||||
//
|
||||
// applocationid — `orderRepository.go` calls it "authoritative" and has
|
||||
// no fallback anywhere for a 0.
|
||||
// moduleid — same file: "tenantlocations carries 0 for
|
||||
// moduleid/partnerid at outlets whose live orders
|
||||
// nonetheless use non-zero values", worked around there
|
||||
// by copying scaffolding off the most recent real order.
|
||||
// A shop commissioned a minute ago has no such order.
|
||||
//
|
||||
// Neither column has a database default, and no onboarding form asks for
|
||||
// them — they describe the platform, not the shop. The tenant's own values
|
||||
// are the right answer and are already right here.
|
||||
//
|
||||
// Filled before the insert rather than corrected after it, so there is one
|
||||
// write and no window where the row exists with a zero in it.
|
||||
if data.Tenantlocations.Applocationid == 0 {
|
||||
data.Tenantlocations.Applocationid = data.Applocationid
|
||||
}
|
||||
if data.Tenantlocations.Moduleid == 0 {
|
||||
data.Tenantlocations.Moduleid = data.Moduleid
|
||||
}
|
||||
|
||||
tx := r.db.Begin()
|
||||
|
||||
// Step 1: Insert into tenants
|
||||
|
||||
@@ -255,6 +255,30 @@ func (r *userRepository) GetTenantUserById(userid int) models.TenantUserInfo {
|
||||
}
|
||||
|
||||
func (r *userRepository) CreateUser(user models.User) (int, error) {
|
||||
// Inherit the delivery region from the tenant when the caller did not name
|
||||
// one.
|
||||
//
|
||||
// `app_users.applocationid` has no column default, and no console form
|
||||
// collects it — it is a platform region, not something a merchant picks
|
||||
// per person. So every back-office account created through this path landed
|
||||
// with 0, which is not a region: `orderRepository.go` calls the equivalent
|
||||
// column on tenantlocations "authoritative" and has no fallback for a zero,
|
||||
// and 43 of 75 live branches are already in that state.
|
||||
//
|
||||
// A lookup rather than a default value, because the right answer is
|
||||
// whichever region the business trades in. Failure is not fatal: the
|
||||
// account is still worth creating, and a 0 here is exactly what would have
|
||||
// been written anyway.
|
||||
if user.Applocationid == 0 && user.Tenantid > 0 {
|
||||
var inherited int
|
||||
if err := r.db.Raw(
|
||||
`SELECT COALESCE(applocationid, 0) FROM tenants WHERE tenantid = ?`,
|
||||
user.Tenantid,
|
||||
).Scan(&inherited).Error; err == nil && inherited > 0 {
|
||||
user.Applocationid = inherited
|
||||
}
|
||||
}
|
||||
|
||||
tx := r.db.Begin()
|
||||
|
||||
if err := tx.Table("app_users").Create(&user).Error; err != nil {
|
||||
|
||||
Reference in New Issue
Block a user