I did a bit of cleanup on the start cycle count API while I was reworking the product availability flow. Mainly I removed some outdated validation (the updateCycleCount method is not needed anymore) and moved other validation logic to the command object instead like we do for recount.

we now do the QoH refresh outside of the cycleCountItems loop below. We can't just loop the items of the count because there might be new items created that the count is not aware of.

suggestion: It would be cool to separate the "detect changes" from the "apply changes" in case we want show the user what's about to happen. Detect changes could return a list of changes that could be displayed to the user.

yeah I was actually considering doing something like this originally but it got kinda complex (I needed three lists, delete, create, update). I'll give it another think through again though

same here. We now refresh QoH outside of the loop on cycle count items because there may be new items that the count is not aware of

While the final QoH of the product for each [bin location + lot number] will always be the same value as was counted

I'm struggling to understand this comment. Is this just saying the final quantity counted (i.e. the quantity entered into the cycle count item) will be the same as the value counted in the physical world? Or is "final QoH" referring to the QoH computed and stored in product availability?

I think I get the concept you're trying to lay out, though. Correct me if I'm wrong.

• The user start the count, QoH (product availability) is copied to cycle count item (X)
• The user counts the cycle count item in the physical world (puts the value on paper) (Y)
• The user enters the quantity counted (Y) into the cycle count item
• The user submits the count for review (X and Y are saved to cycle count item)
• The system computes the quantity variance (Z = X - Y)
• The system displays the quantity variance (Z) to the user
• The user refreshes the cycle count which might update the quantity variance (Z₂ = X₂ - Y)
• The system displays this new quantity variance to the user but the user doesn't not necessarily see how the value changed (before/after refresh)

yeah essentially. The value of Y (aka quantity counted) remains unchanged from a refresh, so while quantity variance and the resulting adjustment amount may change, the actual quantity that we end up with will still be Y.

I'll update the comment to clarify

Have you made sure the items are still sorted properly? When I was helping Sebastian with that, initially I had this Set typed, but if I remember correctly the sorting was not working. In the docs they do not set type for SortedSet, I know this should be working with the type, but maybe it's Grails' bug that it doesn't.
Please make sure the sorting is still working when adding the type.

oh interesting. It looks like it's working fine for me (it's sorting by expiration date):

I would suggest creating a class for this amount of arguments being passed to a method. I think we once agreed that 3-4 arguments should be the maximum.

hmm yeah I can do that. I'll try to think a bit about it though to see if there's a way I can simplify things. The only reason I split this method out was to make the flow a bit cleaner so if it's still confusing maybe that's not helpful

Just create a command class something like UpdateOrCreateCycleCountItemCommand.

You could also potentially break this into multiple methods (find or create vs detect changes vs update).

The method that detect changes could also return data (or a command) to tell you what operation needs to be executed: UpdateCycleCountItemCommand, DeleteCycleCountItemCommand, etc.

I would add a comment explaining what is happening here. I had to think a longer time what it is doing.
I assume if the first item is changed, the flag is set to true, and then we don't even have to check further items in terms of itemHasChanged.

(a side thought): what if an available item was deleted for a particular inventoryItem + bin? Is it even possible? 🤔
if it is, then shouldn't we actually loop through cycleCountItems and find matching availableItem for an item?
I'm thinking about a situation where you have 5 cycleCountItems and suddenly only 4 matching availableItems - for this scenario, we wouldn't even "touch" one of the cycleCountItems for which the stock might not even exist.
Sounds complicated so let me know if you need a further clarification.

I thought we decided in a previous tech huddle that that case wasn't possible, or at least isn't realistic. I can double check with Manon about it though

I think I remember us confirming that it can definitely happen but I could be wrong. One example that comes to mind is that if you perform a Record Stock, this will remove inventory items from the product availability table if the items aren't included in the PRODUCT_INVENTORY transaction.

For v1, I think we need to implement a few simple (known) scenarios (quantities are changed) but be very defensive to protect against scenarios that we haven't thought of or are more complicated. I think the easiest way to do that would be to check whether the inventory items in product availability match exactly with the cycle count items (except for quantity). If they don't match, we should fail-fast and suggest cancelation of the cycle count until we have a chance to deal with the particular scenario that is missing or train users on how to get the transaction data right. We're going to have scenarios we haven't thought about so this will at least prevent us from causing more accuracy issues.

Note that the cycle count is an investigative / measurement process. The fact that we are also trying to correct inventory is, in my opinion, a secondary concern. So my preference would still be to create adjustments off what we knew at the beginning of the count rather than what we're doing here. In fact, this could be a location-based feature flag. I see two potential feature flags

1. whether to allow refresh at all (hide the button)
2. whether to fail or proceed when detecting changes

@kchelstowski Thanks for bringing that up.

if you perform a Record Stock, this will remove inventory items from the product availability table if the items aren't included in the PRODUCT_INVENTORY transaction.

wouldn't that just set QoH to 0, not delete the actual row? Anyways, I'll implement the case regardless. It's not too complex of a scenario to code up.

We could have added a check to see if the cycle count items and product availability align at the end of the count and fail-fast if they don't.

We do support that via the failOnOutdatedQuantity flag. We just also support refreshing it via the refreshQuantityOnHand flag. We let the client decide what behaviour they want. It's easy to change by simply specifying refreshQuantityOnHand=false.

The fact that transactions would occur during a cycle count is a symptom of bad practices

Most sites that are doing inventory counts now don't create transactions during the count (according to conversations Manon had). They pause everything. That said, I do think having the ability to recover from transactions happening isn't a bad thing. If we're following good practices, we won't ever need to refresh, but not being able to at all can result in scenarios where entire bins get zeroed out.

But again this is an edge case scenario. Normally we won't actually need to refresh at all. So I get your point. The thing we should fix is the practice, but assuming this flow works, I don't see any harm in having it

That said, I do think having the ability to recover from transactions happening isn't a bad thing.

I completely agree. I think this feature is going to be game-changing.

Apologies, this got longer than I was hoping. We can discuss on a call if you see any merit to continuing this conversation.

My main point is that while we might think we're covering our bases by implementing these three generic cases:

• same: there are the same number of cycle count items as product availability items
• more: there are more cycle count items than product availability items
• less: there are less cycle count items than product availability items

... it's possible that we might come to each of these three cases in more ways than we understand and it might also be possible that we might need to handle each case differently based on the specific use case that caused the state we're in.

And I'm not saying that we're not covering all of these use cases properly at the moment. I'm saying that I don't know for sure because it's complex. There are a lot of in-betweens that make this more complicated than "there are transactions happening while we're cycle counting".

For example, when a user is picking an order, the quantity picked might be anywhere in the warehouse (even if the system thinks it's in the bin location). It could be in the bin location, in-transit to the packing location, at the packing station, in a staging area, loaded onto a truck, etc. The system will always think the quantity being picked is still in the bin location so the inventory item will still be in product_availability (at the bin location) until it is shipped. If the quantity picked is anywhere else but the bin location then we might encounter a double debit (quantity picked, quantity adjusted) when the cycle count is completed.

The opposite is true when someone is putting an inventory item away. They could physically move the stock to the putaway location but they haven't walked back to the computer to enter the putaway into the system yet. So the counter would likely have double count stock.

I know we've thought a lot about these scenarios. So again, I'm not saying we're not handling these cases properly. I think the idea is to deal with these issues in the resolution process i.e. go off and investigate so that we understand where the stock might have gone. But that assumes that we can actually figure out where all of the inventory items went and explain discrepancies well enough to resolve them all without making a mistake.

But mistakes happen. Someone could see that a count variance can be explained by an outbound pick (because the quantity variance matches the quantity to pick) so they resolve the issue as "item was picked and moved to staging area" when it hadn't been picked yet. After the count is done another user goes and picks the order and we find ourselves in a double debit situation.

These are the easy cases so we'll probably handle them fine. But I'm concerned about scenarios that we haven't thought about or encountered yet.

TL;DR: Thinking that we can homogenously handle each case (e.g "there are the same/less/more cycle count items than product availability items") feels dangerous to me.

that's a fair point. The system only matches the inventory items as they are in the app, so any user error that causes OB to not reflect reality will still result in inaccuracies. But that's essentially outside of our control. We only know what OB knows, and so all we can do is stay in sync with that. Using your example, if a warehouse has a process that moves stock before it has been picked in OB (and don't document anywhere physically that this has happened), then someone does a count without knowledge of what the first person did, then the first person puts the pick in OB causing a double deduction, well that's probably a bad process that needs fixing and retraining.

The way I see it, any solution will always be stuck having to work in tandem with training users in communication and in best practices of using the app (and processing stock movements in general).

I get that we want to be flexible to however they want to use the app, but some practices, like the example I said above, fundamentally introduce confusion into the system. And that could be fixed in a number of ways. Maybe they require anything that is picked to always immediately get put in OB, maybe they have a physical ledger of pending picks that they can reference when counting to make sure they don't double deduct the stock if they find it missing.

Another example could be someone input a stock transfer in OB but didn't move the physical stock yet. Another user refreshes the count, sees a new bin has been added, goes to count it, finds nothing so puts 0, then we've wiped out the whole bin. But that's the same indication to me of a process failure.

I do get your point, but I see these types of inaccuracies as an indication of a bad practice and failure of communication. And that failure can be resolved via whatever way they see fit, whether its via keeping a more accurate stock in OB, or adding communication processes in the warehouse itself.

Either way, if someone misunderstands a situation and inputs inaccurate data, the hope is that they later realize what happened, and a new cycle count can be initialized to fix mistakes. Or if they don't realize, then at least it will get caught the next time a cycle count on the product is automatically scheduled.

Maybe they require anything that is picked to always immediately get put in OB,

That is exactly what needs to happen. We built OB with a very low volume warehouse use case in mind. But now that we're doing cycle counts and allowing transactions to occur during the cycle count, that means we need to be more fine-grained with the transactions. Whether stock is being picked into a "tote" and taken to an outbound packing location, being loaded onto a truck, or is on a forklift heading from the receiving bay to a putaway location ... each of these micro transactions should be tracked in OB.

This is why I've been advocating for handheld workflows. These activities (receipt, putaway, pick, pack, load, etc) are all internal transactions that should be recorded in the system to have precise details about where stock exists.

This may never happen with PIH, but for high volume warehouses this is going to be essential.

maybe they have a physical ledger of pending picks that they can reference when counting to make sure they don't double deduct the stock if they find it missing.

Yes, a lot of these locations have physical bin cards that can be used to understand the flow. It just gets more complicated to unravel and understand the edge cases as more actors (picker, counter) and systems (OB, paper bin card) get introduced into a scenario.

what happened with that method? I can't find where it has been moved or why it has been removed

this is part of the refactoring I did.

The first and second check got moved to the CycleCountStartCommand. There, if !cycleCount.status.isCounting() (which makes only REQUESTED and COUNTING valid), we error.

The third check isn't actually valid anymore since we split up start count and start recount flows and start count no longer accepts the countIndex (we just hard code countIndex == 0 for start count).

you don't need to check for valid cases, by default this would return true, so this check is unnecessary
Not a change request but if you were to remove this check, remember to add null safe operators or handle it somehow else below

it's so the next check doesn't fail if the cycleCount is null. I could have made the next check if (cycleCount && !cycleCount.status.isCounting()) but I figured it'd be cleaner this way and would make it easier to add more valiation in the future if we needed

future suggestion: Can we just make this an operation on the cycle count rather than the items subresource?

Also, this isn't a big deal (doesn't need to be done), but I have been wanting to adopt a convention around these RPC-like endpoints to distinguish them from the API resources.

I like the way google handles them.

• AIP https://google.aip.dev/234
• Real-world example https://developers.google.com/authorized-buyers/apis/reference/rest/v2beta1/accounts.proposals/accept

Any chance we could

"/api/facilities/$facility/cycle-counts/$cycleCountId:refresh" {
    controller = "cycleCountApi"
    action = [POST: "refreshCycleCountItems"]
}

We could even do it more generically

"/api/facilities/$facility/cycle-counts/$cycleCountId:$batchOperation" {
    controller = "cycleCountApi"
    action = [POST: batchOperation]
}

And the URL would look something like this.

/api/facilities/$facility/cycle-counts/$cycleCountId:refreshCycleCountItems

hmm an interesting idea. I don't know if I like having a generic operation handler API (how would we handle different request payloads? It seems like in the best case we'd end up with a huge switch statement) but I do think we should revisit our uri structure and the colon structure is intriguing.

I made it /items because it's an operation on the items, not the count (and we could have a /items/$cycleCountItemId/refresh API that refreshes just a single row/item if we really wanted) but I suppose we don't need to make that distinction when refreshing all the items of a count.

Sorry, I didn't make that more explicit.

The batchOperation was meant to show that we could use a variable for the action handler as opposed to defining multiple URL mappings that looks the same.

"/api/facilities/$facility/cycle-counts/$cycleCountId:$batchOperation" {
    controller = "cycleCountApi"
    action = [POST: batchOperation]
}

Therefore, we would implement the actions we want to support as separate methods.

def refreshCycleCount(RefreshCycleCountCommand) { }
def doThatThing(DoThatThingCommand command) { }
def doThatOtherThing(DoThatOtherThingCommand) { }  

The main design element would just be changing from a RPC endpoint (which looks like a REST resource but isn't).

/api/facilities/$facility/cycle-counts/$cycleCountId/refresh

to an action-oriented with a different naming convention

/api/facilities/$facility/cycle-counts/$cycleCountId:refresh

Functionally equivalent, but I really want to avoid RPC endpoints. So we either need to come up with a subresource where a refresh could be implemented using REST or we support these non-REST actions using a non-REST naming convention.

hm interesting. So essentially we have a /resource:action structure where we're making a distinction between the resource we're acting on and the action we're performing. Yeah I do like that.

I'm going to leave it as /$cycleCountId/refresh for now until we can think it through more thoroughly, but I do think a structure like $cycleCountId:refresh that is more explicit about what is the resource and what is the action could be quite useful.

I think I remember us confirming that it can definitely happen but I could be wrong. One example that comes to mind is that if you perform a Record Stock, this will remove inventory items from the product availability table if the items aren't included in the PRODUCT_INVENTORY transaction.

For v1, I think we need to implement a few simple (known) scenarios (quantities are changed) but be very defensive to protect against scenarios that we haven't thought of or are more complicated. I think the easiest way to do that would be to check whether the inventory items in product availability match exactly with the cycle count items (except for quantity). If they don't match, we should fail-fast and suggest cancelation of the cycle count until we have a chance to deal with the particular scenario that is missing or train users on how to get the transaction data right. We're going to have scenarios we haven't thought about so this will at least prevent us from causing more accuracy issues.

Note that the cycle count is an investigative / measurement process. The fact that we are also trying to correct inventory is, in my opinion, a secondary concern. So my preference would still be to create adjustments off what we knew at the beginning of the count rather than what we're doing here. In fact, this could be a location-based feature flag. I see two potential feature flags

1. whether to allow refresh at all (hide the button)
2. whether to fail or proceed when detecting changes

joke: But I loved this line, why did you get rid of it?

anxiety/perfectionist brain to the rescue!

@kchelstowski Thanks for bringing that up.

The method that detect changes could also return data (or a command) to tell you what operation needs to be executed: UpdateCycleCountItemCommand, DeleteCycleCountItemCommand, etc.

suggestion: It would be cool to separate the "detect changes" from the "apply changes" in case we want show the user what's about to happen. Detect changes could return a list of changes that could be displayed to the user.

Right now, the failOn is probably up to the frontend developer but it would be good to wire this into a feature flag that can be used to set this value on the frontend. i.e. instead of hard-coding this in React, we just pull from a the feature flag from the AppContext API.

agreed (though I'm leaving it as is for now). I have a lot of thoughts on how to handle frontend settings that I want to put in a discovery. Hopefully I can get that going soon.

Shouldn't we recompute whether this is true instead of relying on an argument? Or is that what's happening within the caller when this is invoked?

yeah it's happening in the caller. This will be true if command.refreshQuantityOnHand == true when submitting a count. We trust that the caller already did the refresh (cycleCountService.submitCount calls cycleCountProductAvailabilityService.refreshProductAvailability(cycleCount)) so that we can avoid doing it again here. It's a performance saving thing.

There isn't really another way of determining if this is true since to do so would require fetching product availability again (which is what we're trying to avoid).

While the final QoH of the product for each [bin location + lot number] will always be the same value as was counted

I'm struggling to understand this comment. Is this just saying the final quantity counted (i.e. the quantity entered into the cycle count item) will be the same as the value counted in the physical world? Or is "final QoH" referring to the QoH computed and stored in product availability?

I think I get the concept you're trying to lay out, though. Correct me if I'm wrong.

• The user start the count, QoH (product availability) is copied to cycle count item (X)
• The user counts the cycle count item in the physical world (puts the value on paper) (Y)
• The user enters the quantity counted (Y) into the cycle count item
• The user submits the count for review (X and Y are saved to cycle count item)
• The system computes the quantity variance (Z = X - Y)
• The system displays the quantity variance (Z) to the user
• The user refreshes the cycle count which might update the quantity variance (Z₂ = X₂ - Y)
• The system displays this new quantity variance to the user but the user doesn't not necessarily see how the value changed (before/after refresh)