Skip to content

Conversation

@wagnerlmichael
Copy link
Member

@wagnerlmichael wagnerlmichael commented Jan 8, 2026

With the addition of manual analyst sale validation, we are updating the ingest here to preference outlier status based off of analyst determination.

I believe this PR updates all of the relevant functional fields. However, it is worth flagging the model report component of these refs. This ties into a larger question of how we want to handle the sv_outlier_reason$n in model reporting in the future, given that they no longer tell the whole story given the analyst review addition.

For example, if the is_outlier field is true based on a flag_override column, there will be corresponding sv_outlier_reason columns linked to that doc no, but they aren't the deterministic fields used in the calculation of the is_outlier in the flag_override case.

It probably also makes sense to consider changing the broader architecture of column organization. I'm thinking these questions probably makes sense for a separate issue, but I'm curious to hear thoughts.

@wagnerlmichael wagnerlmichael linked an issue Jan 8, 2026 that may be closed by this pull request
@wagnerlmichael wagnerlmichael marked this pull request as ready for review January 9, 2026 21:06
Copy link
Member

@jeancochrane jeancochrane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking great! I think our next step is to run the model and see if it works. That will be most interesting to do once we have completed ingesting 2026 data and are able to update the model params to predict the South tri for lien date Jan 1 2026, since most of the sales that Valuations reviewed are South tri 2025 sales. I expect we should be ready by early next week.

I believe this PR updates all of the relevant functional fields. However, it is worth flagging the model report component of these refs. This ties into a larger question of how we want to handle the sv_outlier_reason$n in model reporting in the future, given that they no longer tell the whole story given the analyst review addition.

Hmm, this is a great point. Building on the new is_outlier data model, I wonder if we could use a new outlier_reason field that can similarly coalesce model outlier reasons with analyst override outlier reasons? I'm imagining an array of strings that includes all sv_outlier_reason{N} values, plus any fieldnames from sale.flag_override that led to a sale being marked as an outlier. Does that seem reasonable to you? I'm happy to chat this out in person if it'd be easier, since there are some tricky follow-up design decisions (e.g. do we include sv_outlier_reason{N} values even when is_outlier is false). Regardless, I think you're right that we should tackle this in a separate issue -- if we pursue this direction, we'll likely want a data-architecture PR to define the field, and then a PR on each of model-res-avm and model-condo-avm to switch up the reporting to use this new combined fields for outlier reasons.

@jeancochrane
Copy link
Member

I stole this task (sorry!) and opened #423, so this branch is no longer necessary.

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.

Incorporate sales val analyst overrides into the model

2 participants