feat: Historical Contractor Payments SDK (AI pilot experiment)#1476
feat: Historical Contractor Payments SDK (AI pilot experiment)#1476
Conversation
…eriment) Standalone HistoricalPaymentFlow component for recording past-dated contractor payments. Built as an AI-assisted development pilot to test whether existing SDK patterns + AI can accelerate component generation. Full flow: date selection → contractor amounts → preview → submit → success. Verified end-to-end with live API data via SDK Dev App. See src/components/Contractor/Payments/HistoricalPayments/README.md for full context, testing instructions, and what feedback we're looking for. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
I think overall this captured a lot of the existing conventions for component composition. I think the biggest miss was the combination of the preview and create historical payment into the same component which makes these tangled in a way that partners wouldn't be able to consume individually and also has the CreateHistoricalPayment component doing too much. This is one in which having a baseline eng consult to organize the state machine and component composition as a first pass would be beneficial.
We could also consider trying to distill some of that into a create flow skill as something to watch out for when spinning up flows.
In any case, my other largest rec would be to decompose for submission of the state machine and then each of the components individually for easier review and verification, likely merging the state machine first to get the baseline organization that will impact all of the other downstream components.
Other issues I flagged that introduce subtleties that may not be immediately obvious
- Manual form reset - it's not always apparent that this isn't working until you are doing some combo of triggering validation issues, clearing them, running submit but I have had trouble in the past with getting react hook form to properly reset and generally prefer to actually unmount via conditional rendering or react key
- Button with onSubmit and onClick will fire twice and could lead to erroneous behavior that isn't always predictable
- Virtualization has memoization that has a deep object that would not throw on referential equality so i would def want to stress test that and maybe just consider a more event driven approach to that composition instead
- I worry about the conditional field rendering and if it has correctly captured the business logic. The flat schema that corresponds also is worth digging into more
Overall if the purpose of this exercise is to determine where our gaps are with LLM generation for larger flows I think that goes into a bigger question of how we work as an eng team which is obviously going to evolve over time. But the biggest shortcoming is on the composition of the block components and how those would interact with the flow which would have implications for hooks build out as well as how partners would use these components. I think there's value in having more eng collaboration up front to make sure the base infrastructure and component organization is in place even as AI fleshes out the implementation details to help remedy that. I also think at the current snapshot in time there's still value in working smaller and submitting PRs that are more digestable.
| 'Contractor.Payments.HistoricalPayments.CreateHistoricalPayment', | ||
| dictionary, | ||
| ) | ||
| const { t } = useTranslation('Contractor.Payments.HistoricalPayments.CreateHistoricalPayment') |
There was a problem hiding this comment.
I think we would want to create a separate dedicated domain for this, Contractor.HistoricalPayments rather than nesting this many values on the translation key
| ) | ||
|
|
||
| const formMethods = useForm<EditHistoricalPaymentFormValues>({ | ||
| resolver: zodResolver(createEditHistoricalPaymentFormSchema()), |
There was a problem hiding this comment.
We likely don't need a create function on the schema for this one
| const initialContractorPayments: (ContractorPayments & { isTouched: boolean })[] = useMemo( | ||
| () => | ||
| contractors.map(contractor => ({ | ||
| contractorUuid: contractor.uuid, | ||
| paymentMethod: HISTORICAL_PAYMENT_METHOD, | ||
| wage: '0', | ||
| hours: '0', | ||
| bonus: '0', | ||
| reimbursement: '0', | ||
| isTouched: false, | ||
| })), | ||
| [contractors], | ||
| ) | ||
| const [virtualContractorPayments, setVirtualContractorPayments] = | ||
| useState(initialContractorPayments) | ||
| const totals = useMemo( | ||
| () => | ||
| virtualContractorPayments.reduce( | ||
| (acc, payment) => { | ||
| const contractor = contractors.find(c => c.uuid === payment.contractorUuid) | ||
| const isHourly = contractor?.wageType === 'Hourly' | ||
| const hours = Number(payment.hours || '0') | ||
| const wage = Number(payment.wage || '0') | ||
| const bonus = Number(payment.bonus || '0') | ||
| const reimbursement = Number(payment.reimbursement || '0') | ||
| const hourlyAmount = isHourly ? hours * Number(contractor.hourlyRate || '0') : 0 | ||
| const fixedWage = isHourly ? 0 : wage | ||
|
|
||
| return { | ||
| wage: acc.wage + fixedWage, | ||
| bonus: acc.bonus + bonus, | ||
| reimbursement: acc.reimbursement + reimbursement, | ||
| total: acc.total + hourlyAmount + fixedWage + bonus + reimbursement, | ||
| } | ||
| }, | ||
| { wage: 0, bonus: 0, reimbursement: 0, total: 0 }, | ||
| ), | ||
| [virtualContractorPayments, contractors], | ||
| ) |
There was a problem hiding this comment.
I think there's some reworking here that could take place from the mapping -> virtualization. i think we could update this to be driven by the modal submission rather than needing to memoize
| formMethods.reset( | ||
| { | ||
| wageType: contractor?.wageType || 'Hourly', | ||
| hours: Number(contractorPayment?.hours || '0'), | ||
| wage: Number(contractorPayment?.wage || '0'), | ||
| bonus: Number(contractorPayment?.bonus || '0'), | ||
| reimbursement: Number(contractorPayment?.reimbursement || '0'), | ||
| hourlyRate: Number(contractor?.hourlyRate || '0'), | ||
| contractorUuid: contractorUuid, | ||
| }, | ||
| { keepDirty: false, keepValues: false }, | ||
| ) |
There was a problem hiding this comment.
I think we can update this to conditionally render the modal so we get the form reset out of the box rather than trying to reset form state. There are some issues that can happen with forms that are tough to track down with the attempted manual reset
| const displayContractor = contractors.find( | ||
| contractor => contractor.uuid === data.contractorUuid, | ||
| ) | ||
| const displayName = DOMPurify.sanitize( |
There was a problem hiding this comment.
I believe we already have a first/last name utility that is using DOMPurify under the hood
| const totals = useMemo( | ||
| () => | ||
| contractorPaymentGroup.contractorPayments?.reduce( | ||
| (acc, contractor) => { | ||
| acc.wageAmount += Number(contractor.wage || '0') | ||
| acc.bonusAmount += Number(contractor.bonus || '0') | ||
| acc.reimbursementAmount += Number(contractor.reimbursement || '0') | ||
| acc.totalAmount += Number(contractor.wageTotal || '0') | ||
| return acc | ||
| }, | ||
| { wageAmount: 0, bonusAmount: 0, reimbursementAmount: 0, totalAmount: 0 }, | ||
| ), | ||
| [contractorPaymentGroup.contractorPayments], | ||
| ) |
There was a problem hiding this comment.
These totals calculations are getting repeated, would recommend breaking this into a shared util for contractor payments
| export type InternalAlert = { | ||
| type: 'error' | 'info' | 'success' | ||
| title: string | ||
| content?: ReactNode | ||
| onDismiss?: () => void | ||
| translationParams?: Record<string, unknown> | ||
| onAction?: () => void | ||
| actionLabel?: string | ||
| } |
There was a problem hiding this comment.
Not sure why this is duplicating the alert props?
| <Button | ||
| variant="primary" | ||
| type="submit" | ||
| form={formId} | ||
| onClick={() => formMethods.handleSubmit(onSubmit)} | ||
| > | ||
| {t('saveCta')} | ||
| </Button> |
There was a problem hiding this comment.
We should either remove type="submit" or update to associate the button action with the form id
Also doesn't this button need a loading/pending state?
| {previewData && ( | ||
| <HistoricalPreviewPresentation | ||
| contractorPaymentGroup={previewData} | ||
| contractors={contractors} | ||
| onBackToEdit={onBackToEdit} | ||
| onSubmit={onCreatePaymentGroup} | ||
| isLoading={isCreatingContractorPaymentGroup || isPreviewingContractorPaymentGroup} | ||
| /> | ||
| )} | ||
| {!previewData && ( | ||
| <CreateHistoricalPaymentPresentation | ||
| contractors={contractors} | ||
| contractorPayments={virtualContractorPayments} | ||
| paymentDate={paymentDate} | ||
| onPaymentDateChange={setPaymentDate} | ||
| onSaveAndContinue={onContinueToPreview} | ||
| onEditContractor={onEditContractor} | ||
| totals={totals} | ||
| alerts={alerts} | ||
| isLoading={isCreatingContractorPaymentGroup || isPreviewingContractorPaymentGroup} | ||
| /> | ||
| )} |
There was a problem hiding this comment.
I think this would need more thought from the eng side of things on how to compose these components. Having CreateHistoricalPayment fork on the preview or the create historical payment is coupling these things too tightly. I think we'd likely want the preview as a dedicated component living as a separate entry in the state machine and then we would navigate to that preview based on an event fired from this component. As is, this component is doing too much
| } as const | ||
|
|
||
| export const contractorHistoricalPaymentEvents = { | ||
| CONTRACTOR_HISTORICAL_PAYMENT_CREATE: 'contractor/historicalPayments/create', |
There was a problem hiding this comment.
This is never actually emitted and can be removed
2 participants
Context
This is the output of an AI-assisted SDK component generation pilot — not intended for production merge as-is. We're sharing it for eng commentary on pattern adherence, architectural decisions, and feasibility of this approach as a development accelerator.
Background: Historical Contractor Payments was descoped from the Contractor Payments SDK launch because mixing historical and future-dated payments in one flow caused UX confusion. This PR builds a standalone
HistoricalPaymentFlowcomponent for recording past-dated contractor payments.What's in this PR
A complete
HistoricalPaymentFlowwith a 3-step journey:15 new files under
src/components/Contractor/Payments/HistoricalPayments/following all existing SDK patterns: robot3 state machine, container/presentation split, BaseComponent, ComponentsContext, Zod forms, i18n, DataView, DOMPurify, React Query hooks.See the README for full details on how it was built, file inventory, and current status.
How to test
npm install && npm run sdk-appWhat's done
What's NOT done (would need before production)
What we're looking for
This is shared for commentary, not merge approval:
🤖 Generated with Claude Code